Skip to content

Commit

Permalink
Support promise artifacts in anon targets
Browse files Browse the repository at this point in the history
Summary: Allow anon targets to accept promise artifacts

Reviewed By: bobyangyf

Differential Revision: D46727402

fbshipit-source-id: b71db01145e2965f7b771d0514f7347e70e35656
  • Loading branch information
wendy728 authored and facebook-github-bot committed Jun 15, 2023
1 parent 1e5fb5b commit a00a673
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 6 deletions.
6 changes: 6 additions & 0 deletions app/buck2_anon_target/src/anon_target_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ 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_node::attrs::attr_type::arg::ConfiguredStringWithMacros;
Expand Down Expand Up @@ -62,6 +63,8 @@ pub enum AnonTargetAttr {
Dep(Box<DepAttr<ConfiguredProvidersLabel>>),
// Accepts any bound artifacts. Maps to `attr.source()`.
Artifact(Artifact),
// Accepts unresolved promise artifacts. Maps to `attr.source()`.
PromiseArtifact(StarlarkPromiseArtifact),
Arg(ConfiguredStringWithMacros),
}

Expand Down Expand Up @@ -95,6 +98,7 @@ impl AttrDisplayWithContext for AnonTargetAttr {
AnonTargetAttr::Dep(e) => write!(f, "\"{}\"", e),
AnonTargetAttr::Artifact(e) => write!(f, "\"{}\"", e),
AnonTargetAttr::Arg(e) => write!(f, "\"{}\"", e),
AnonTargetAttr::PromiseArtifact(e) => write!(f, "\"{}\"", e),
}
}
}
Expand All @@ -119,6 +123,7 @@ impl ToJsonWithContext for AnonTargetAttr {
AnonTargetAttr::Dep(e) => Ok(to_value(e.to_string())?),
AnonTargetAttr::Artifact(e) => Ok(to_value(e.to_string())?),
AnonTargetAttr::Arg(e) => Ok(to_value(e.to_string())?),
AnonTargetAttr::PromiseArtifact(e) => Ok(to_value(e.to_string())?),
}
}
}
Expand Down Expand Up @@ -165,6 +170,7 @@ impl AnonTargetAttr {
AnonTargetAttr::Dep(dep) => traversal.dep(&dep.label),
AnonTargetAttr::Artifact(_) => Ok(()),
AnonTargetAttr::Arg(e) => e.string_with_macros.traverse(traversal),
AnonTargetAttr::PromiseArtifact(..) => Ok(()),
}
}

Expand Down
14 changes: 10 additions & 4 deletions app/buck2_anon_target/src/anon_target_attr_coerce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::fmt::Debug;
use std::iter;

use buck2_build_api::interpreter::rule_defs::artifact::starlark_artifact_like::ValueAsArtifactLike;
use buck2_build_api::interpreter::rule_defs::artifact::StarlarkPromiseArtifact;
use buck2_build_api::interpreter::rule_defs::provider::dependency::Dependency;
use buck2_build_api::interpreter::rule_defs::resolved_macro::ResolvedStringWithMacros;
use buck2_core::soft_error;
Expand Down Expand Up @@ -117,13 +118,18 @@ impl AnonTargetAttrTypeCoerce for AttrType {
}
_ => Err(AnonTargetCoercionError::type_error("dependency", value).into()),
},
AttrTypeInner::Source(_) => match value.as_artifact() {
Some(artifact_like) => {
AttrTypeInner::Source(_) => {
// 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()))
} else if let Some(artifact_like) = value.as_artifact() {
let artifact = artifact_like.get_bound_artifact()?;
Ok(AnonTargetAttr::Artifact(artifact))
} else {
Err(AnonTargetCoercionError::type_error("artifact", value).into())
}
None => Err(AnonTargetCoercionError::type_error("artifact", value).into()),
},
}
AttrTypeInner::Arg(_) => match ResolvedStringWithMacros::from_value(value) {
Some(resolved_macro) => match resolved_macro.configured_macros() {
Some(configured_macros) => Ok(AnonTargetAttr::Arg(configured_macros.clone())),
Expand Down
1 change: 1 addition & 0 deletions app/buck2_anon_target/src/anon_target_attr_resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ impl AnonTargetAttrExt 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),
AnonTargetAttr::PromiseArtifact(artifact) => Ok(ctx.heap().alloc(artifact.clone())),
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,18 @@ enum PromiseArtifactError {
///
/// The `StarlarkPromiseArtifact` is created with `ctx.actions.promise_artifact()` and will
/// not have been bound until promises are resolved at the end of analysis.
#[derive(Debug, NoSerialize, ProvidesStaticType, Trace, Allocative, Clone)]
#[derive(
Debug,
NoSerialize,
ProvidesStaticType,
Trace,
Allocative,
Clone,
Hash,
Eq,
PartialEq
)]

pub struct StarlarkPromiseArtifact {
declaration_location: Option<FileSpan>,
artifact: PromiseArtifact,
Expand Down Expand Up @@ -93,7 +104,7 @@ impl StarlarkPromiseArtifact {
}
}

fn as_artifact(&self) -> ArtifactGroup {
pub fn as_artifact(&self) -> ArtifactGroup {
match self.artifact.get() {
Some(artifact) => ArtifactGroup::Artifact(artifact.dupe()),
None => ArtifactGroup::Promise(self.artifact.dupe()),
Expand Down

0 comments on commit a00a673

Please sign in to comment.