Make alias targets keep their own configuration.

This prevents the case of a rule acquiring the null configuration in case the alias points to an input file. Which, in turn, makes "bazel build" work for these targets. The reason it breaks is that TargetCompletionValue instances are created from the associated ConfiguredTarget in SkyframeExecutor#buildArtifacts(), which means that if the configurations do not match, TargetCompletionFunction requests a different ConfiguredTargetValue than it was created from.

Fixes #1297.

--
MOS_MIGRATED_REVID=122973526

5 files changed, 44 insertions, 14 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/Alias.java b/src/main/java/com/google/devtools/build/lib/rules/Alias.java
index a24c5378b5..11abd0e4ea 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/Alias.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/Alias.java
@@ -38,6 +38,7 @@ public class Alias implements RuleConfiguredTargetFactory {
   public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException {
     ConfiguredTarget actual = (ConfiguredTarget) ruleContext.getPrerequisite("actual", Mode.TARGET);
     return new AliasConfiguredTarget(
+        ruleContext.getConfiguration(),
         actual,
         ImmutableMap.of(
             AliasProvider.class, AliasProvider.fromAliasRule(ruleContext.getLabel(), actual),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
index c7d2387277..e3f7dd26a2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
@@ -29,6 +29,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.syntax.ClassObject;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
+import com.google.devtools.build.lib.util.Preconditions;
 
 /**
  * This configured target pretends to be whatever type of target "actual" is, returning its
@@ -38,14 +39,16 @@ import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
  */
 @Immutable
 public final class AliasConfiguredTarget implements ConfiguredTarget, ClassObject {
-  private final ConfiguredTarget configuredTarget;
+  private final BuildConfiguration configuration;
+  private final ConfiguredTarget actual;
   private final ImmutableMap<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>
       overrides;
 
-  public AliasConfiguredTarget(ConfiguredTarget actual,
+  public AliasConfiguredTarget(BuildConfiguration configuration, ConfiguredTarget actual,
       ImmutableMap<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> overrides) {
-    configuredTarget = actual;
-    this.overrides = overrides;
+    this.configuration = Preconditions.checkNotNull(configuration);
+    this.actual = Preconditions.checkNotNull(actual);
+    this.overrides = Preconditions.checkNotNull(overrides);
   }
 
   @Override
@@ -54,27 +57,30 @@ public final class AliasConfiguredTarget implements ConfiguredTarget, ClassObjec
       return provider.cast(overrides.get(provider));
     }
 
-    return configuredTarget == null ? null : configuredTarget.getProvider(provider);
+    return actual == null ? null : actual.getProvider(provider);
   }
 
   @Override
   public Label getLabel() {
-    return configuredTarget.getLabel();
+    return actual.getLabel();
   }
 
   @Override
   public Object get(String providerKey) {
-    return configuredTarget == null ? null : configuredTarget.get(providerKey);
+    return actual == null ? null : actual.get(providerKey);
   }
 
   @Override
   public Target getTarget() {
-    return configuredTarget == null ? null : configuredTarget.getTarget();
+    return actual == null ? null : actual.getTarget();
   }
 
   @Override
   public BuildConfiguration getConfiguration() {
-    return configuredTarget.getConfiguration();
+    // This does not return actual.getConfiguration() because actual might be an input file, in
+    // which case its configuration is null and we don't want to have rules that have a null
+    // configuration.
+    return configuration;
   }
 
   /* ClassObject methods */
@@ -87,18 +93,18 @@ public final class AliasConfiguredTarget implements ConfiguredTarget, ClassObjec
       // A shortcut for files to build in Skylark. FileConfiguredTarget and RunleConfiguredTarget
       // always has FileProvider and Error- and PackageGroupConfiguredTarget-s shouldn't be
       // accessible in Skylark.
-      return SkylarkNestedSet.of(Artifact.class, configuredTarget == null
+      return SkylarkNestedSet.of(Artifact.class, actual == null
           ? NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER)
           : getProvider(FileProvider.class).getFilesToBuild());
     }
-    return configuredTarget == null ? null : configuredTarget.get(name);
+    return actual == null ? null : actual.get(name);
   }
 
   @Override
   public ImmutableCollection<String> getKeys() {
     ImmutableList.Builder<String> result = ImmutableList.<String>builder().add("label", "files");
-    if (configuredTarget != null) {
-        result.addAll(configuredTarget.getProvider(SkylarkProviders.class).getKeys());
+    if (actual != null) {
+        result.addAll(actual.getProvider(SkylarkProviders.class).getKeys());
     }
     return result.build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/Bind.java b/src/main/java/com/google/devtools/build/lib/rules/repository/Bind.java
index a832903d32..f042902268 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/Bind.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/Bind.java
@@ -37,7 +37,9 @@ public class Bind implements RuleConfiguredTargetFactory {
     }
 
     ConfiguredTarget actual = (ConfiguredTarget) ruleContext.getPrerequisite("actual", Mode.TARGET);
-    return new AliasConfiguredTarget(actual,
+    return new AliasConfiguredTarget(
+        ruleContext.getConfiguration(),
+        actual,
         ImmutableMap.<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>of(
             AliasProvider.class, AliasProvider.fromAliasRule(ruleContext.getLabel(), actual)));
   }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/AliasTest.java b/src/test/java/com/google/devtools/build/lib/rules/AliasTest.java
index 34f7336a2a..17d3ea10e8 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/AliasTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/AliasTest.java
@@ -46,6 +46,16 @@ public class AliasTest extends BuildViewTestCase {
   }
 
   @Test
+  public void aliasToInputFile() throws Exception {
+    scratch.file("a/BUILD",
+        "exports_files(['a'])",
+        "alias(name='b', actual='a')");
+
+    ConfiguredTarget b = getConfiguredTarget("//a:b");
+    assertThat(ActionsTestUtil.baseArtifactNames(getFilesToBuild(b))).containsExactly("a");
+  }
+
+  @Test
   public void visibilityIsOverriddenAndIsOkay() throws Exception {
     scratch.file("a/BUILD",
         "filegroup(name='a', visibility=['//b:__pkg__'])");
diff --git a/src/test/shell/bazel/bazel_rules_test.sh b/src/test/shell/bazel/bazel_rules_test.sh
index c82b9cd996..bb3768685b 100755
--- a/src/test/shell/bazel/bazel_rules_test.sh
+++ b/src/test/shell/bazel/bazel_rules_test.sh
@@ -373,4 +373,15 @@ EOF
  expect_log "The number is 42"
 }
 
+function test_build_with_aliased_input_file() {
+  mkdir -p a
+  cat > a/BUILD <<EOF
+exports_files(['f'])
+alias(name='a', actual=':f')
+EOF
+
+  touch a/f
+  bazel build //a:a || fail "build failed"
+}
+
 run_suite "rules test"