Expose aspect-related information in the extra-action proto that Bazel hands to action_listener() rules.

RELNOTES: Extra actions now contain aspect-related information.

--
MOS_MIGRATED_REVID=138832922

9 files changed, 129 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index a20888b8f2..69bcac0355 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -41,6 +41,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Symlinks;
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Map;
 import javax.annotation.Nullable;
 
 /**
@@ -440,10 +441,24 @@ public abstract class AbstractAction implements Action, SkylarkValue {
 
   @Override
   public ExtraActionInfo.Builder getExtraActionInfo() {
-    return ExtraActionInfo.newBuilder()
-        .setOwner(getOwner().getLabel().toString())
-        .setId(getKey())
-        .setMnemonic(getMnemonic());
+    ActionOwner owner = getOwner();
+    ExtraActionInfo.Builder result =
+        ExtraActionInfo.newBuilder()
+            .setOwner(owner.getLabel().toString())
+            .setId(getKey())
+            .setMnemonic(getMnemonic());
+    if (owner.getAspectName() != null) {
+      result.setAspectName(owner.getAspectName());
+    }
+    if (owner.getAspectParameters() != null) {
+      for (Map.Entry<String, Collection<String>> entry :
+          owner.getAspectParameters().getAttributes().asMap().entrySet()) {
+        result.putAspectParameters(
+            entry.getKey(),
+            ExtraActionInfo.StringList.newBuilder().addAllValue(entry.getValue()).build());
+      }
+    }
+    return result;
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
index a7a4aa7bb3..b641a4d31a 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
@@ -17,6 +17,7 @@ import com.google.auto.value.AutoValue;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.util.Preconditions;
 import javax.annotation.Nullable;
 
@@ -32,10 +33,12 @@ import javax.annotation.Nullable;
 public abstract class ActionOwner {
   /** An action owner for special cases. Usage is strongly discouraged. */
   public static final ActionOwner SYSTEM_ACTION_OWNER =
-      ActionOwner.create(null, null, "system", "empty target kind", "system", null);
+      ActionOwner.create(null, null, null, null, "system", "empty target kind", "system", null);
 
   public static ActionOwner create(
       @Nullable Label label,
+      @Nullable String aspectName,
+      @Nullable AspectParameters aspectParameters,
       @Nullable Location location,
       @Nullable String mnemonic,
       @Nullable String targetKind,
@@ -44,6 +47,8 @@ public abstract class ActionOwner {
     return new AutoValue_ActionOwner(
         location,
         label,
+        aspectName,
+        aspectParameters,
         mnemonic,
         Preconditions.checkNotNull(configurationChecksum),
         targetKind,
@@ -58,6 +63,12 @@ public abstract class ActionOwner {
   @Nullable
   public abstract Label getLabel();
 
+  @Nullable
+  public abstract String getAspectName();
+
+  @Nullable
+  public abstract AspectParameters getAspectParameters();
+
   /** Returns the configuration's mnemonic. */
   @Nullable
   public abstract String getConfigurationMnemonic();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 239fd936ef..f90123dbc4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -1055,6 +1055,7 @@ public class BuildView {
             env,
             (Rule) target.getTarget(),
             null,
+            null,
             targetConfig,
             configurations.getHostConfiguration(),
             ruleClassProvider.getPrerequisiteValidator(),
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index fdd38db6de..7a375cedad 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -228,6 +228,7 @@ public final class ConfiguredTargetFactory {
                 env,
                 rule,
                 null,
+                null,
                 configuration,
                 hostConfiguration,
                 ruleClassProvider.getPrerequisiteValidator(),
@@ -317,13 +318,16 @@ public final class ConfiguredTargetFactory {
       BuildConfiguration aspectConfiguration,
       BuildConfiguration hostConfiguration)
       throws InterruptedException {
-    RuleContext.Builder builder = new RuleContext.Builder(env,
-        associatedTarget.getTarget(),
-        aspect.getAspectClass().getName(),
-        aspectConfiguration,
-        hostConfiguration,
-        ruleClassProvider.getPrerequisiteValidator(),
-        aspect.getDefinition().getConfigurationFragmentPolicy());
+    RuleContext.Builder builder =
+        new RuleContext.Builder(
+            env,
+            associatedTarget.getTarget(),
+            aspect.getAspectClass().getName(),
+            aspect.getParameters(),
+            aspectConfiguration,
+            hostConfiguration,
+            ruleClassProvider.getPrerequisiteValidator(),
+            aspect.getDefinition().getConfigurationFragmentPolicy());
     RuleContext ruleContext =
         builder
             .setVisibility(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 8f186d4cfa..8bee13b64e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -53,6 +53,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
@@ -106,6 +107,7 @@ import javax.annotation.Nullable;
  */
 public final class RuleContext extends TargetContext
     implements ActionConstructionContext, ActionRegistry, RuleErrorConsumer {
+
   /**
    * The configured version of FilesetEntry.
    */
@@ -147,6 +149,8 @@ public final class RuleContext extends TargetContext
   private static final String HOST_CONFIGURATION_PROGRESS_TAG = "for host";
 
   private final Rule rule;
+  @Nullable private final String aspectName;
+  @Nullable private final AspectParameters aspectParameters;
   private final ListMultimap<String, ConfiguredTarget> targetMap;
   private final ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap;
   private final ImmutableMap<Label, ConfigMatchingProvider> configConditions;
@@ -178,6 +182,8 @@ public final class RuleContext extends TargetContext
     super(builder.env, builder.rule, builder.configuration, builder.prerequisiteMap.get(null),
         builder.visibility);
     this.rule = builder.rule;
+    this.aspectName = builder.getAspectName();
+    this.aspectParameters = builder.getAspectParameters();
     this.configurationFragmentPolicy = builder.configurationFragmentPolicy;
     this.universalFragment = universalFragment;
     this.targetMap = targetMap;
@@ -322,7 +328,7 @@ public final class RuleContext extends TargetContext
   @Override
   public ActionOwner getActionOwner() {
     if (actionOwner == null) {
-      actionOwner = createActionOwner(rule, getConfiguration());
+      actionOwner = createActionOwner(rule, aspectName, aspectParameters, getConfiguration());
     }
     return actionOwner;
   }
@@ -398,9 +404,15 @@ public final class RuleContext extends TargetContext
   }
 
   @VisibleForTesting
-  public static ActionOwner createActionOwner(Rule rule, BuildConfiguration configuration) {
+  public static ActionOwner createActionOwner(
+      Rule rule,
+      @Nullable String aspectName,
+      @Nullable AspectParameters aspectParameters,
+      BuildConfiguration configuration) {
     return ActionOwner.create(
         rule.getLabel(),
+        aspectName,
+        aspectParameters,
         rule.getLocation(),
         configuration.getMnemonic(),
         rule.getTargetKind(),
@@ -1402,6 +1414,7 @@ public final class RuleContext extends TargetContext
     private final BuildConfiguration hostConfiguration;
     private final PrerequisiteValidator prerequisiteValidator;
     @Nullable private final String aspectName;
+    @Nullable private final AspectParameters aspectParameters;
     private final ErrorReporter reporter;
     private OrderedSetMultimap<Attribute, ConfiguredTarget> prerequisiteMap;
     private ImmutableMap<Label, ConfigMatchingProvider> configConditions;
@@ -1413,6 +1426,7 @@ public final class RuleContext extends TargetContext
         AnalysisEnvironment env,
         Rule rule,
         @Nullable String aspectName,
+        @Nullable AspectParameters aspectParameters,
         BuildConfiguration configuration,
         BuildConfiguration hostConfiguration,
         PrerequisiteValidator prerequisiteValidator,
@@ -1420,6 +1434,7 @@ public final class RuleContext extends TargetContext
       this.env = Preconditions.checkNotNull(env);
       this.rule = Preconditions.checkNotNull(rule);
       this.aspectName = aspectName;
+      this.aspectParameters = aspectParameters;
       this.configurationFragmentPolicy = Preconditions.checkNotNull(configurationFragmentPolicy);
       this.configuration = Preconditions.checkNotNull(configuration);
       this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration);
@@ -1924,6 +1939,16 @@ public final class RuleContext extends TargetContext
         prerequisiteValidator.validate(this, prerequisite, attribute);
       }
     }
+
+    @Nullable
+    public AspectParameters getAspectParameters() {
+      return aspectParameters;
+    }
+
+    @Nullable
+    public String getAspectName() {
+      return aspectName;
+    }
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java b/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java
index 6862f106ce..15ef2bb372 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java
@@ -19,7 +19,6 @@ import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.Multimap;
-
 import java.util.Objects;
 
 /**
@@ -64,6 +63,10 @@ public final class AspectParameters {
     return attributes.get(key);
   }
 
+  public ImmutableMultimap<String, String> getAttributes() {
+    return attributes;
+  }
+
   /**
    * Similar to {@link #getAttribute}}, but asserts that there's only one value for the provided
    * key.
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index 53b83c682e..81fbd6c0dc 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -190,7 +190,14 @@ public final class ActionsTestUtil {
 
   public static final ActionOwner NULL_ACTION_OWNER =
       ActionOwner.create(
-          NULL_LABEL, null, "dummy-configuration-mnemonic", null, "dummy-configuration", null);
+          NULL_LABEL,
+          null,
+          null,
+          null,
+          "dummy-configuration-mnemonic",
+          null,
+          "dummy-configuration",
+          null);
 
   public static final ArtifactOwner NULL_ARTIFACT_OWNER =
       new ArtifactOwner() {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
index 80b8ab9448..3c87b27e9f 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis.actions;
 
+import static com.google.common.collect.Iterables.getOnlyElement;
 import static com.google.common.truth.Truth.assertThat;
 import static java.nio.charset.StandardCharsets.ISO_8859_1;
 import static java.util.Arrays.asList;
@@ -24,6 +25,7 @@ import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Sets;
+import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.AbstractAction;
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -420,4 +422,44 @@ public class SpawnActionTest extends BuildViewTestCase {
       fail("Expected exception");
     } catch (IllegalArgumentException expected) {}
   }
+
+  /**
+   * Tests that the ExtraActionInfo proto that's generated from an action, contains Aspect-related
+   * information.
+   */
+  @Test
+  public void testGetExtraActionInfoOnAspects() throws Exception {
+    scratch.file(
+        "a/BUILD",
+        "load('//a:def.bzl', 'testrule')",
+        "testrule(name='a', deps=[':b'])",
+        "testrule(name='b')");
+    scratch.file(
+        "a/def.bzl",
+        "def _aspect_impl(target, ctx):",
+        "  f = ctx.new_file('foo.txt')",
+        "  ctx.action(outputs = [f], command = 'echo foo > \"$1\"')",
+        "  return struct(output=f)",
+        "def _rule_impl(ctx):",
+        "  return struct(files=set([artifact.output for artifact in ctx.attr.deps]))",
+        "aspect1 = aspect(_aspect_impl, attr_aspects=['deps'], ",
+        "    attrs = {'parameter': attr.string(values = ['param_value'])})",
+        "testrule = rule(_rule_impl, attrs = { ",
+        "    'deps' : attr.label_list(aspects = [aspect1]), ",
+        "    'parameter': attr.string(default='param_value') })");
+
+    update(
+        ImmutableList.of("//a:a"),
+        false /* keepGoing */,
+        1 /* loadingPhaseThreads */,
+        true /* doAnalysis */,
+        new EventBus());
+
+    Artifact artifact = getOnlyElement(getFilesToBuild(getConfiguredTarget("//a:a")));
+    ExtraActionInfo.Builder extraActionInfo = getGeneratingAction(artifact).getExtraActionInfo();
+    assertThat(extraActionInfo.getAspectName()).isEqualTo("//a:def.bzl%aspect1");
+    assertThat(extraActionInfo.getAspectParametersMap())
+        .containsExactly(
+            "parameter", ExtraActionInfo.StringList.newBuilder().addValue("param_value").build());
+  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java
index 5594a32734..e841310da1 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/ExperimentalStateTrackerTest.java
@@ -390,7 +390,7 @@ public class ExperimentalStateTrackerTest extends FoundationTestCase {
         "/home/user/bazel/out/abcdef/some/very/very/long/path/for/some/library/directory/foo.jar");
     Label label =
         Label.parseAbsolute("//some/very/very/long/path/for/some/library/directory:libfoo");
-    ActionOwner owner = ActionOwner.create(label, null, null, null, "fedcba", null);
+    ActionOwner owner = ActionOwner.create(label, null, null, null, null, null, "fedcba", null);
     when(action.getOwner()).thenReturn(owner);
 
     clock.advanceMillis(TimeUnit.SECONDS.toMillis(3));
@@ -517,7 +517,8 @@ public class ExperimentalStateTrackerTest extends FoundationTestCase {
     Label labelFooTest = Label.parseAbsolute("//foo/bar:footest");
     ConfiguredTarget targetFooTest = Mockito.mock(ConfiguredTarget.class);
     when(targetFooTest.getLabel()).thenReturn(labelFooTest);
-    ActionOwner fooOwner = ActionOwner.create(labelFooTest, null, null, null, "abcdef", null);
+    ActionOwner fooOwner =
+        ActionOwner.create(labelFooTest, null, null, null, null, null, "abcdef", null);
 
     Label labelBarTest = Label.parseAbsolute("//baz:bartest");
     ConfiguredTarget targetBarTest = Mockito.mock(ConfiguredTarget.class);
@@ -525,7 +526,8 @@ public class ExperimentalStateTrackerTest extends FoundationTestCase {
     TestFilteringCompleteEvent filteringComplete = Mockito.mock(TestFilteringCompleteEvent.class);
     when(filteringComplete.getTestTargets())
         .thenReturn(ImmutableSet.of(targetFooTest, targetBarTest));
-    ActionOwner barOwner = ActionOwner.create(labelBarTest, null, null, null, "fedcba", null);
+    ActionOwner barOwner =
+        ActionOwner.create(labelBarTest, null, null, null, null, null, "fedcba", null);
 
     stateTracker.testFilteringComplete(filteringComplete);