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
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
index 0a736a3..30647ad 100644
--- 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 @@
* $(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 @@
}
}
- // 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
index 0b16cae..e578b03 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -905,6 +905,42 @@
}
@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");
scratch.file("test/BUILD", "java_library(name = 'xxx')");
