BaseDeferredKey -> DeferredHolderKey in ActionsRegistry

Summary:
Mostly for documentation: it is not obviously clear that `ActionsRegistry` is created per dynamic actions rather than one per target.

Can also be used in assertions.

Reviewed By: cjhopman

Differential Revision: D60861802

fbshipit-source-id: 94c569e462b16e1c0db3c994a102a5f3a31aca17

app/buck2_build_api/src/actions/registry.rs

@@ -18,7 +18,6 @@ use buck2_artifact::artifact::artifact_type::DeclaredArtifact;
 use buck2_artifact::artifact::artifact_type::OutputArtifact;
 use buck2_artifact::artifact::build_artifact::BuildArtifact;
 use buck2_artifact::deferred::key::DeferredHolderKey;
-use buck2_core::base_deferred_key::BaseDeferredKey;
 use buck2_core::category::Category;
 use buck2_core::execution_types::execution::ExecutionPlatformResolution;
 use buck2_core::fs::buck_out_path::BuckOutPath;
@@ -50,10 +49,10 @@ use crate::analysis::registry::AnalysisValueFetcher;
 use crate::artifact_groups::ArtifactGroup;
 use crate::deferred::calculation::ActionLookup;
 
-/// The actions registry for a particular analysis of a rule implementation
+/// The actions registry for a particular analysis of a rule, dynamic actions, anon target, BXL.
 #[derive(Allocative)]
 pub struct ActionsRegistry {
-    owner: BaseDeferredKey,
+    owner: DeferredHolderKey,
     action_key: Option<Arc<str>>,
     artifacts: IndexSet<DeclaredArtifact>,
 
@@ -66,7 +65,7 @@ pub struct ActionsRegistry {
 }
 
 impl ActionsRegistry {
-    pub fn new(owner: BaseDeferredKey, execution_platform: ExecutionPlatformResolution) -> Self {
+    pub fn new(owner: DeferredHolderKey, execution_platform: ExecutionPlatformResolution) -> Self {
         Self {
             owner,
             action_key: None,
@@ -188,7 +187,7 @@ impl ActionsRegistry {
         };
         self.claim_output_path(&path, declaration_location)?;
         let out_path =
-            BuckOutPath::with_action_key(self.owner.dupe(), path, self.action_key.dupe());
+            BuckOutPath::with_action_key(self.owner.owner().dupe(), path, self.action_key.dupe());
         let declared = DeclaredArtifact::new(out_path, output_type, hidden);
         if !self.artifacts.insert(declared.dupe()) {
             panic!("not expected duplicate artifact after output path was successfully claimed");

app/buck2_build_api/src/analysis/registry.rs

@@ -129,7 +129,7 @@ impl<'v> AnalysisRegistry<'v> {
         }
 
         Ok(AnalysisRegistry {
-            actions: ActionsRegistry::new(owner.dupe(), execution_platform.dupe()),
+            actions: ActionsRegistry::new(self_key.dupe(), execution_platform.dupe()),
             artifact_groups: ArtifactGroupRegistry::new(),
             dynamic: (DYNAMIC_REGISTRY_NEW.get()?)(owner.dupe(), execution_platform.dupe()),
             anon_targets: (ANON_TARGET_REGISTRY_NEW.get()?)(PhantomData, execution_platform),

app/buck2_build_api_tests/src/actions/registry.rs

@@ -41,7 +41,10 @@ fn declaring_artifacts() -> anyhow::Result<()> {
         "cell//pkg:foo",
         ConfigurationData::testing_new(),
     ));
-    let mut actions = ActionsRegistry::new(base.dupe(), ExecutionPlatformResolution::unspecified());
+    let mut actions = ActionsRegistry::new(
+        DeferredHolderKey::Base(base.dupe()),
+        ExecutionPlatformResolution::unspecified(),
+    );
     let out1 = ForwardRelativePathBuf::unchecked_new("bar.out".into());
     let buckout1 = BuckOutPath::new(base.dupe(), out1.clone());
     let declared1 = actions.declare_artifact(None, out1.clone(), OutputType::File, None)?;
@@ -71,12 +74,8 @@ fn declaring_artifacts() -> anyhow::Result<()> {
 
 #[test]
 fn claiming_conflicting_path() -> anyhow::Result<()> {
-    let target = ConfiguredTargetLabel::testing_parse(
-        "cell//pkg:my_target",
-        ConfigurationData::testing_new(),
-    );
     let mut actions = ActionsRegistry::new(
-        BaseDeferredKey::TargetLabel(target.dupe()),
+        DeferredHolderKey::testing_new("cell//pkg:my_target"),
         ExecutionPlatformResolution::unspecified(),
     );
 
@@ -140,7 +139,10 @@ fn register_actions() -> anyhow::Result<()> {
         "cell//pkg:foo",
         ConfigurationData::testing_new(),
     ));
-    let mut actions = ActionsRegistry::new(base.dupe(), ExecutionPlatformResolution::unspecified());
+    let mut actions = ActionsRegistry::new(
+        DeferredHolderKey::Base(base.dupe()),
+        ExecutionPlatformResolution::unspecified(),
+    );
     let out = ForwardRelativePathBuf::unchecked_new("bar.out".into());
     let declared = actions.declare_artifact(None, out, OutputType::File, None)?;
 
@@ -180,7 +182,7 @@ fn finalizing_actions() -> anyhow::Result<()> {
         ConfigurationData::testing_new(),
     ));
     let mut actions = ActionsRegistry::new(
-        base.dupe(),
+        DeferredHolderKey::Base(base.dupe()),
         ExecutionPlatformResolution::new(
             Some(ExecutionPlatform::legacy_execution_platform(
                 CommandExecutorConfig::testing_local(),
@@ -258,11 +260,7 @@ fn duplicate_category_identifier() {
 fn category_identifier_test(
     action_names: &[(&'static str, Option<&'static str>)],
 ) -> anyhow::Result<()> {
-    let base = BaseDeferredKey::TargetLabel(ConfiguredTargetLabel::testing_parse(
-        "cell//pkg:foo",
-        ConfigurationData::testing_new(),
-    ));
-    let deferred_holder_key = DeferredHolderKey::Base(base.dupe());
+    let base = DeferredHolderKey::testing_new("cell//pkg:foo");
     let mut actions = ActionsRegistry::new(
         base.dupe(),
         ExecutionPlatformResolution::new(
@@ -280,14 +278,9 @@ fn category_identifier_test(
             identifier.map(|i| i.to_owned()),
         );
 
-        actions.register(
-            &deferred_holder_key,
-            indexset![],
-            indexset![],
-            unregistered_action,
-        )?;
+        actions.register(&base, indexset![], indexset![], unregistered_action)?;
     }
 
-    actions.ensure_bound(&AnalysisValueFetcher::testing_new(deferred_holder_key))?;
+    actions.ensure_bound(&AnalysisValueFetcher::testing_new(base))?;
     Ok(())
 }

app/buck2_build_api_tests/src/interpreter/rule_defs/artifact/testing.rs

@@ -107,7 +107,7 @@ pub(crate) fn artifactory(builder: &mut GlobalsBuilder) {
     ) -> anyhow::Result<StarlarkDeclaredArtifact> {
         let target_label = get_label(eval, "//foo:bar")?;
         let mut registry = ActionsRegistry::new(
-            BaseDeferredKey::TargetLabel(target_label),
+            DeferredHolderKey::Base(BaseDeferredKey::TargetLabel(target_label)),
             ExecutionPlatformResolution::unspecified(),
         );
         let artifact = registry.declare_artifact(
@@ -130,7 +130,7 @@ pub(crate) fn artifactory(builder: &mut GlobalsBuilder) {
     ) -> anyhow::Result<StarlarkDeclaredArtifact> {
         let target_label = get_label(eval, target)?;
         let mut registry = ActionsRegistry::new(
-            BaseDeferredKey::TargetLabel(target_label.dupe()),
+            DeferredHolderKey::Base(BaseDeferredKey::TargetLabel(target_label.dupe())),
             ExecutionPlatformResolution::unspecified(),
         );
         let artifact = registry.declare_artifact(
@@ -195,7 +195,7 @@ pub(crate) fn artifactory(builder: &mut GlobalsBuilder) {
             ExecutionPlatformResolution::unspecified(),
         )?;
         let mut actions_registry = ActionsRegistry::new(
-            BaseDeferredKey::TargetLabel(target_label.dupe()),
+            DeferredHolderKey::Base(BaseDeferredKey::TargetLabel(target_label.dupe())),
             ExecutionPlatformResolution::unspecified(),
         );