diff --git a/app/buck2_anon_target/src/anon_target_attr.rs b/app/buck2_anon_target/src/anon_target_attr.rs index 8e88ade90053..b9e9b5499767 100644 --- a/app/buck2_anon_target/src/anon_target_attr.rs +++ b/app/buck2_anon_target/src/anon_target_attr.rs @@ -12,7 +12,6 @@ use std::fmt::Display; use allocative::Allocative; use buck2_artifact::artifact::artifact_type::Artifact; -use buck2_build_api::interpreter::rule_defs::artifact::StarlarkPromiseArtifact; use buck2_core::package::PackageLabel; use buck2_core::provider::label::ConfiguredProvidersLabel; use buck2_core::provider::label::ProvidersLabel; @@ -38,6 +37,7 @@ use serde::Serializer; use serde_json::to_value; use crate::anon_target_attr_resolve::AnonTargetAttrTraversal; +use crate::promise_artifacts::PromiseArtifactAttr; #[derive(Debug, Clone, PartialEq, Eq, Hash, Allocative)] pub enum AnonTargetAttr { @@ -66,7 +66,7 @@ pub enum AnonTargetAttr { // Accepts any bound artifacts. Maps to `attr.source()`. Artifact(Artifact), // Accepts unresolved promise artifacts. Maps to `attr.source()`. - PromiseArtifact(StarlarkPromiseArtifact), + PromiseArtifact(PromiseArtifactAttr), Arg(ConfiguredStringWithMacros), Label(ProvidersLabel), } @@ -183,7 +183,7 @@ impl AnonTargetAttr { #[allow(unused)] pub fn traverse_anon_attr<'a>( &'a self, - _traversal: &mut dyn AnonTargetAttrTraversal, + traversal: &mut dyn AnonTargetAttrTraversal, ) -> anyhow::Result<()> { match self { AnonTargetAttr::Bool(_) => Ok(()), @@ -192,31 +192,30 @@ impl AnonTargetAttr { AnonTargetAttr::EnumVariant(_) => Ok(()), AnonTargetAttr::List(list) => { for v in list.iter() { - v.traverse_anon_attr(_traversal)?; + v.traverse_anon_attr(traversal)?; } Ok(()) } AnonTargetAttr::Tuple(list) => { for v in list.iter() { - v.traverse_anon_attr(_traversal)?; + v.traverse_anon_attr(traversal)?; } Ok(()) } AnonTargetAttr::Dict(dict) => { for (k, v) in dict.iter() { - k.traverse_anon_attr(_traversal)?; - v.traverse_anon_attr(_traversal)?; + k.traverse_anon_attr(traversal)?; + v.traverse_anon_attr(traversal)?; } Ok(()) } AnonTargetAttr::None => Ok(()), - AnonTargetAttr::OneOf(l, _) => l.traverse_anon_attr(_traversal), + AnonTargetAttr::OneOf(l, _) => l.traverse_anon_attr(traversal), AnonTargetAttr::Dep(_) => Ok(()), AnonTargetAttr::Artifact(_) => Ok(()), AnonTargetAttr::Arg(_) => Ok(()), - AnonTargetAttr::PromiseArtifact(_) => { - // TODO(@wendyy) - use traversal here after updating the attr type - Ok(()) + AnonTargetAttr::PromiseArtifact(promise_artifact) => { + traversal.promise_artifact(promise_artifact) } AnonTargetAttr::Label(_) => Ok(()), } diff --git a/app/buck2_anon_target/src/anon_target_attr_coerce.rs b/app/buck2_anon_target/src/anon_target_attr_coerce.rs index de86d561e851..c752a3952612 100644 --- a/app/buck2_anon_target/src/anon_target_attr_coerce.rs +++ b/app/buck2_anon_target/src/anon_target_attr_coerce.rs @@ -46,6 +46,7 @@ use starlark::values::Value; use crate::anon_target_attr::AnonTargetAttr; use crate::anon_targets::AnonAttrCtx; +use crate::promise_artifacts::PromiseArtifactAttr; pub trait AnonTargetAttrTypeCoerce { fn coerce_item(&self, ctx: &AnonAttrCtx, value: Value) -> anyhow::Result; @@ -134,7 +135,10 @@ impl AnonTargetAttrTypeCoerce for AttrType { // Check if this is a StarlarkPromiseArtifact first before checking other artifact types to // allow anon targets to accept unresolved promise artifacts. if let Some(promise_artifact) = StarlarkPromiseArtifact::from_value(value) { - Ok(AnonTargetAttr::PromiseArtifact(promise_artifact.clone())) + Ok(AnonTargetAttr::PromiseArtifact(PromiseArtifactAttr { + id: promise_artifact.artifact.id.as_ref().clone(), + short_path: promise_artifact.short_path.clone(), + })) } else if let Some(artifact_like) = ValueAsArtifactLike::unpack_value(value) { let artifact = artifact_like.0.get_bound_artifact()?; Ok(AnonTargetAttr::Artifact(artifact)) diff --git a/app/buck2_anon_target/src/anon_target_attr_resolve.rs b/app/buck2_anon_target/src/anon_target_attr_resolve.rs index feed424b0db6..2030f0638cf4 100644 --- a/app/buck2_anon_target/src/anon_target_attr_resolve.rs +++ b/app/buck2_anon_target/src/anon_target_attr_resolve.rs @@ -8,6 +8,8 @@ */ use std::collections::HashMap; +use std::sync::Arc; +use std::sync::OnceLock; use buck2_analysis::analysis::env::RuleAnalysisAttrResolutionContext; use buck2_analysis::attrs::resolve::attr_type::arg::ConfiguredStringWithMacrosExt; @@ -15,7 +17,10 @@ use buck2_analysis::attrs::resolve::attr_type::dep::DepAttrTypeExt; use buck2_analysis::attrs::resolve::ctx::AttrResolutionContext; use buck2_artifact::artifact::artifact_type::Artifact; use buck2_build_api::analysis::calculation::RuleAnalysisCalculation; +use buck2_build_api::artifact_groups::promise::PromiseArtifact; +use buck2_build_api::artifact_groups::promise::PromiseArtifactResolveError; use buck2_build_api::interpreter::rule_defs::artifact::StarlarkArtifact; +use buck2_build_api::interpreter::rule_defs::artifact::StarlarkPromiseArtifact; use buck2_build_api::interpreter::rule_defs::provider::collection::FrozenProviderCollectionValue; use buck2_build_api::keep_going; use buck2_core::package::PackageLabel; @@ -43,7 +48,6 @@ use crate::promise_artifacts::PromiseArtifactAttr; // No macros in anon targets, so query results are empty. Execution platform resolution should // always be inherited from the anon target. pub(crate) struct AnonTargetAttrResolutionContext<'v> { - #[allow(unused)] // TODO(@wendyy) pub(crate) promised_artifacts_map: HashMap<&'v PromiseArtifactAttr, Artifact>, pub(crate) rule_analysis_attr_resolution_ctx: RuleAnalysisAttrResolutionContext<'v>, } @@ -122,9 +126,45 @@ impl AnonTargetAttrResolution for AnonTargetAttr { AnonTargetAttr::Dep(d) => DepAttrType::resolve_single(ctx, d), AnonTargetAttr::Artifact(d) => Ok(ctx.heap().alloc(StarlarkArtifact::new(d.clone()))), AnonTargetAttr::Arg(a) => a.resolve(ctx, &pkg), - AnonTargetAttr::PromiseArtifact(artifact) => { - // TODO(@wendyy) - use promised artifact map here to construct fulfilled `StarlarkPromiseArtifact` - Ok(ctx.heap().alloc(artifact.clone())) + AnonTargetAttr::PromiseArtifact(promise_artifact_attr) => { + let promise_id = promise_artifact_attr.id.clone(); + // We validated that the analysis contains the promise artifact id earlier + let artifact = anon_resolution_ctx + .promised_artifacts_map + .get(&promise_artifact_attr) + .unwrap(); + + // Assert the short path, since we have the real artifact now + if let Some(expected_short_path) = &promise_artifact_attr.short_path { + artifact.get_path().with_short_path(|artifact_short_path| { + if artifact_short_path != expected_short_path { + Err(anyhow::Error::from( + PromiseArtifactResolveError::ShortPathMismatch( + expected_short_path.clone(), + artifact_short_path.to_string(), + ), + )) + } else { + Ok(()) + } + })?; + } + + let fulfilled = OnceLock::new(); + fulfilled.set(artifact.clone()).unwrap(); + + let fulfilled_promise_inner = + PromiseArtifact::new(Arc::new(fulfilled), Arc::new(promise_id)); + + let fulfilled_promise_artifact = StarlarkPromiseArtifact::new( + None, + fulfilled_promise_inner, + promise_artifact_attr.short_path.clone(), + ); + + // To resolve the promise artifact attr, we end up creating a new `StarlarkPromiseArtifact` with the `OnceLock` set + // with the artifact that was found from the upstream analysis. + Ok(ctx.heap().alloc(fulfilled_promise_artifact)) } AnonTargetAttr::Label(label) => { Ok(ctx.heap().alloc(StarlarkProvidersLabel::new(label.clone()))) @@ -155,6 +195,7 @@ impl AnonTargetDependents { anon_target: &AnonTargetKey, ) -> anyhow::Result { struct DepTraversal(Vec); + struct PromiseArtifactTraversal(Vec); impl ConfiguredAttrTraversal for DepTraversal { fn dep(&mut self, dep: &ConfiguredProvidersLabel) -> anyhow::Result<()> { @@ -171,15 +212,25 @@ impl AnonTargetDependents { } } + impl AnonTargetAttrTraversal for PromiseArtifactTraversal { + fn promise_artifact( + &mut self, + promise_artifact: &PromiseArtifactAttr, + ) -> anyhow::Result<()> { + self.0.push(promise_artifact.clone()); + Ok(()) + } + } + let mut dep_traversal = DepTraversal(Vec::new()); - // @TODO(@wendyy) - populate after switching over to PromiseArtifactAttrs - let promise_artifacts = Vec::new(); + let mut promise_artifact_traversal = PromiseArtifactTraversal(Vec::new()); for x in anon_target.0.attrs().values() { x.traverse(anon_target.0.name().pkg(), &mut dep_traversal)?; + x.traverse_anon_attr(&mut promise_artifact_traversal)?; } Ok(AnonTargetDependents { deps: dep_traversal.0, - promise_artifacts, + promise_artifacts: promise_artifact_traversal.0, }) } diff --git a/app/buck2_anon_target/src/promise_artifacts.rs b/app/buck2_anon_target/src/promise_artifacts.rs index df361c73536d..43f71e04a719 100644 --- a/app/buck2_anon_target/src/promise_artifacts.rs +++ b/app/buck2_anon_target/src/promise_artifacts.rs @@ -75,7 +75,7 @@ impl PromiseArtifactRegistry { #[derive(Debug, Clone, PartialEq, Eq, Hash, Allocative)] pub(crate) struct PromiseArtifactAttr { pub(crate) id: PromiseArtifactId, - short_path: Option, + pub(crate) short_path: Option, } impl fmt::Display for PromiseArtifactAttr { diff --git a/app/buck2_build_api/src/interpreter/rule_defs/artifact/starlark_promise_artifact.rs b/app/buck2_build_api/src/interpreter/rule_defs/artifact/starlark_promise_artifact.rs index de8a7a82baa8..10e9620bbd05 100644 --- a/app/buck2_build_api/src/interpreter/rule_defs/artifact/starlark_promise_artifact.rs +++ b/app/buck2_build_api/src/interpreter/rule_defs/artifact/starlark_promise_artifact.rs @@ -91,7 +91,7 @@ enum PromiseArtifactError { pub struct StarlarkPromiseArtifact { pub declaration_location: Option, pub artifact: PromiseArtifact, - short_path: Option, + pub short_path: Option, } starlark_simple_value!(StarlarkPromiseArtifact);