When running aspects, don't track target's artifacts.

Proposed fix for #7083 With this I can make the test case in that Issue pass. I'm open to suggestions of better ways to tell if we are in an aspect or not. I'll also add a test case if the approach I took seems acceptable. Closes #7282. PiperOrigin-RevId: 238611044
bazelbuild · Mar 15, 2019 · 5adde8e · 5adde8e
1 parent 7988d78
commit 5adde8e
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 4 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java b/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java
@@ -122,7 +122,6 @@ private LocationExpander(
    * $(execpath)/$(execpaths) using Artifact.getExecPath().
    *
    * @param ruleContext BUILD rule
-   * @param labelMap A mapping of labels to build artifacts.
    */
   public static LocationExpander withRunfilesPaths(RuleContext ruleContext) {
     return new LocationExpander(ruleContext, null, false, false);
@@ -362,9 +361,15 @@ static Map<Label, Collection<Artifact>> buildLocationMap(
       }
     }
 
-    // Add all destination locations.
-    for (OutputFile out : ruleContext.getRule().getOutputFiles()) {
-      mapGet(locationMap, out.getLabel()).add(ruleContext.createOutputArtifact(out));
+    // We don't want to do this if we're processing aspect rules. It will
+    // create output artifacts and unbalance the input/output state, leading
+    // to an error (output artifact with no action to create its inputs).
+    if (ruleContext.getMainAspect() == null) {
+      // Add all destination locations.
+      for (OutputFile out : ruleContext.getRule().getOutputFiles()) {
+        // Not in aspect processing, so explicitly build an artifact & let it verify.
+        mapGet(locationMap, out.getLabel()).add(ruleContext.createOutputArtifact(out));
+      }
     }
 
     if (ruleContext.getRule().isAttrDefined("srcs", BuildType.LABEL_LIST)) {

diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -904,6 +904,42 @@ public void aspectFailingOrphanArtifacts() throws Exception {
             + "test/missing_in_action.txt\n");
   }
 
+  @Test
+  public void aspectSkippingOrphanArtifactsWithLocation() throws Exception {
+    scratch.file(
+        "simple/print.bzl",
+        "def _print_expanded_location_impl(target, ctx):",
+        "    return struct(result=ctx.expand_location(ctx.rule.attr.cmd, []))",
+        "",
+        "print_expanded_location = aspect(",
+        "    implementation = _print_expanded_location_impl,",
+        ")");
+    scratch.file(
+        "simple/BUILD",
+        "filegroup(",
+        "    name = \"files\",",
+        "    srcs = [\"afile\"],",
+        ")",
+        "",
+        "genrule(",
+        "    name = \"concat_all_files\",",
+        "    srcs = [\":files\"],",
+        "    outs = [\"concatenated.txt\"],",
+        "    cmd = \"$(location :files)\"",
+        ")");
+
+    reporter.removeHandler(failFastHandler);
+    AnalysisResult analysisResult =
+        update(
+            ImmutableList.of("//simple:print.bzl%print_expanded_location"),
+            "//simple:concat_all_files");
+    assertThat(analysisResult.hasError()).isFalse();
+    AspectValue value = Iterables.getOnlyElement(analysisResult.getAspects());
+    String result = (String) value.getConfiguredAspect().get("result");
+
+    assertThat(result).isEqualTo("simple/afile");
+  }
+
   @Test
   public void topLevelAspectIsNotAnAspect() throws Exception {
     scratch.file("test/aspect.bzl", "MyAspect = 4");