Names of extra-action protos now take into account aspect names.
That is, if an Aspect registered an action that an extra-action is shadowing, its name will be used when creating the extra-action's ID and name.
Without this change, it's impossible to analyze extra-actions when there's more than one aspected rule that acts on the same rule (e.g., java_proto_library and java_lite_proto_library on the same proto_library).
--
PiperOrigin-RevId: 141587608
MOS_MIGRATED_REVID=141587608
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
index 9efea2d..745cea0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionSpec.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Action;
+import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.CommandHelper;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -189,7 +190,7 @@
*/
private Artifact getExtraActionOutputArtifact(
RuleContext ruleContext, Action action, String template) {
- String actionId = getActionId(ruleContext.getLabel(), action);
+ String actionId = getActionId(ruleContext.getActionOwner(), action);
template = template.replace("$(ACTION_ID)", actionId);
template = template.replace("$(OWNER_LABEL_DIGEST)", getOwnerDigest(ruleContext));
@@ -232,9 +233,14 @@
* of a uniqueness guarantee.
*/
@VisibleForTesting
- public static String getActionId(Label label, Action action) {
+ public static String getActionId(ActionOwner owner, Action action) {
Fingerprint f = new Fingerprint();
- f.addString(label.toString());
+ f.addString(owner.getLabel().toString());
+ f.addNullableString(owner.getAspectName());
+ f.addBoolean(owner.getAspectParameters() != null);
+ if (owner.getAspectParameters() != null) {
+ f.addString(owner.getAspectParameters().toString());
+ }
f.addString(action.getKey());
return f.hexDigestAndReset();
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index ae4f533..2ad6952 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -541,6 +541,47 @@
}
@Test
+ public void extraActionsFromDifferentAspectsDontConflict() throws Exception {
+ useConfiguration(
+ "--experimental_action_listener=//pkg1:listener",
+ "--experimental_extra_action_top_level_only");
+
+ scratch.file(
+ "x/BUILD",
+ "load(':extension.bzl', 'injector1', 'injector2', 'null_rule')",
+ "injector1(name='i1_a', deps=[':n'], param = 'a')",
+ "injector1(name='i1_b', deps=[':n'], param = 'b')",
+ "injector2(name='i2', deps=[':n'])",
+ "null_rule(name = 'n')");
+
+ scratch.file(
+ "x/extension.bzl",
+ "def _aspect_impl(target, ctx):",
+ " ctx.empty_action(mnemonic='Mnemonic')",
+ " return struct()",
+ "aspect1 = aspect(_aspect_impl, attr_aspects=['deps'], attrs =",
+ " {'param': attr.string(values = ['a', 'b'])})",
+ "aspect2 = aspect(_aspect_impl, attr_aspects=['deps'])",
+ "def _rule_impl(ctx):",
+ " return struct()",
+ "injector1 = rule(_rule_impl, attrs =",
+ " { 'deps' : attr.label_list(aspects = [aspect1]), 'param' : attr.string() })",
+ "injector2 = rule(_rule_impl, attrs = { 'deps' : attr.label_list(aspects = [aspect2]) })",
+ "null_rule = rule(_rule_impl, attrs = { 'deps' : attr.label_list() })"
+ );
+
+ scratch.file(
+ "pkg1/BUILD",
+ "extra_action(name='xa', cmd='echo dont-care')",
+ "action_listener(name='listener', mnemonics=['Mnemonic'], extra_actions=[':xa'])");
+
+ update("//x:i1_a", "//x:i1_b", "//x:i2");
+
+ // Implicitly check that update() didn't throw an exception because of two actions producing
+ // the same outputs.
+ }
+
+ @Test
public void aspectPropagatesToAllAttributesImplicit() throws Exception {
setRulesAvailableInTests(new TestAspects.BaseRule(),
new TestAspects.SimpleRule(),
