Skip to content

Commit 79d28b4

Browse files
JakobDegenmeta-codesync[bot]
authored andcommitted
artifacts: Rename uses_content_based_paths to path_resolution_may_require_artifact_value
Summary: In D92854486 there was some confusion caused by this naming, so lets update it to clarify what this actually means I've not changed the naming around things like promise artifacts and build artifacts where the name kind of means something different because it's tied to a specific user-facing API Differential Revision: D92929054 fbshipit-source-id: a6dc018469c85f45ca3c49d17e9aaf223f873948
1 parent 963d8d2 commit 79d28b4

File tree

19 files changed

+66
-54
lines changed

19 files changed

+66
-54
lines changed

app/buck2_action_impl/src/actions/impls/copy.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ impl Action for CopyAction {
164164
let artifact_fs = ctx.fs();
165165
let src = input.resolve_path(
166166
artifact_fs,
167-
if input.has_content_based_path() {
167+
if input.path_resolution_requires_artifact_value() {
168168
Some(src_value.content_based_path_hash())
169169
} else {
170170
None

app/buck2_action_impl/src/actions/impls/run.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1444,7 +1444,7 @@ impl Action for RunAction {
14441444
for x in self.starlark_values.outputs_for_error_handler.iter() {
14451445
let artifact = x.inner().artifact();
14461446

1447-
let content_based_path_hash = if artifact.has_content_based_path() {
1447+
let content_based_path_hash = if artifact.path_resolution_requires_artifact_value() {
14481448
let outputs = outputs.ok_or_else(|| {
14491449
buck2_error::buck2_error!(
14501450
buck2_error::ErrorTag::Input,

app/buck2_action_impl/src/actions/impls/run/dep_files.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,7 +1358,10 @@ impl DeclaredDepFiles {
13581358

13591359
for declared_dep_file in self.tagged.values() {
13601360
let dep_file = &declared_dep_file.output;
1361-
let content_hash = if declared_dep_file.output.has_content_based_path() {
1361+
let content_hash = if declared_dep_file
1362+
.output
1363+
.path_resolution_requires_artifact_value()
1364+
{
13621365
Some(
13631366
result
13641367
.get_from_artifact_path(&declared_dep_file.output.get_path())
@@ -1415,7 +1418,10 @@ impl DeclaredDepFiles {
14151418
let mut contents = HashMap::with_capacity(self.tagged.len());
14161419

14171420
for declared_dep_file in self.tagged.values() {
1418-
let content_hash = if declared_dep_file.output.has_content_based_path() {
1421+
let content_hash = if declared_dep_file
1422+
.output
1423+
.path_resolution_requires_artifact_value()
1424+
{
14191425
Some(
14201426
result
14211427
.get_from_artifact_path(&declared_dep_file.output.get_path())

app/buck2_action_impl/src/actions/impls/symlinked_dir.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ impl Action for SymlinkedDirAction {
243243

244244
let src = src_artifact.resolve_path(
245245
ctx.fs(),
246-
if src_artifact.has_content_based_path() {
246+
if src_artifact.path_resolution_requires_artifact_value() {
247247
Some(value.content_based_path_hash())
248248
} else {
249249
None

app/buck2_action_impl/src/actions/impls/write.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl CommandLineContentBasedInputVisitor {
7373

7474
impl<'v> CommandLineArtifactVisitor<'v> for CommandLineContentBasedInputVisitor {
7575
fn visit_input(&mut self, input: ArtifactGroup, _tags: Vec<&ArtifactTag>) {
76-
if input.uses_content_based_path() {
76+
if input.path_resolution_may_require_artifact_value() {
7777
self.content_based_inputs.insert(input);
7878
}
7979
}
@@ -213,7 +213,7 @@ impl Action for WriteAction {
213213
let mut content_based_inputs = visitor.content_based_inputs;
214214
if let Some(macro_files) = &self.inner.macro_files {
215215
for artifact in macro_files {
216-
if artifact.has_content_based_path() {
216+
if artifact.path_resolution_requires_artifact_value() {
217217
content_based_inputs.insert(ArtifactGroup::Artifact(artifact.dupe()));
218218
}
219219
}

app/buck2_action_impl/src/dynamic/deferred.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ async fn materialize_inputs(
466466
for (artifact, artifact_value) in ensured_artifacts {
467467
let path = artifact.resolve_path(
468468
&artifact_fs,
469-
if artifact.has_content_based_path() {
469+
if artifact.path_resolution_requires_artifact_value() {
470470
Some(artifact_value.content_based_path_hash())
471471
} else {
472472
None
@@ -542,7 +542,7 @@ fn artifact_values<'v>(
542542
let k = StarlarkArtifact::new(artifact.dupe());
543543
let path = artifact.get_path().resolve(
544544
artifact_fs,
545-
if artifact.has_content_based_path() {
545+
if artifact.path_resolution_requires_artifact_value() {
546546
Some(artifact_value.content_based_path_hash())
547547
} else {
548548
None
@@ -608,7 +608,7 @@ fn new_attr_value<'v>(
608608
DynamicAttrValue::ArtifactValue(artifact) => {
609609
let path = artifact.get_path().resolve(
610610
artifact_fs,
611-
if artifact.has_content_based_path() {
611+
if artifact.path_resolution_requires_artifact_value() {
612612
Some(
613613
ensured_artifacts
614614
.get(artifact)

app/buck2_anon_target/src/anon_target_attr_resolve.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ impl AnonTargetAttrResolution for AnonTargetAttr {
133133
.unwrap();
134134

135135
let promise_has_content_based_path = promise_artifact_attr.has_content_based_path;
136-
let artifact_has_content_based_path = artifact.has_content_based_path();
136+
let artifact_has_content_based_path =
137+
artifact.path_resolution_requires_artifact_value();
137138
if artifact_has_content_based_path && !promise_has_content_based_path {
138139
return Err(PromiseArtifactResolveError::UsesContentBasedPath(
139140
promise_id.clone(),

app/buck2_artifact/src/artifact/artifact_type.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ impl Artifact {
206206
)
207207
}
208208

209-
pub fn has_content_based_path(&self) -> bool {
209+
pub fn path_resolution_requires_artifact_value(&self) -> bool {
210210
match self.as_parts().0 {
211211
BaseArtifactKind::Source(_) => false,
212212
BaseArtifactKind::Build(b) => b.get_path().is_content_based_path(),
@@ -250,7 +250,7 @@ impl ArtifactDyn for Artifact {
250250
}
251251

252252
fn has_content_based_path(&self) -> bool {
253-
self.has_content_based_path()
253+
self.path_resolution_requires_artifact_value()
254254
}
255255

256256
fn is_projected(&self) -> bool {

app/buck2_build_api/src/actions/execute/action_executor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ impl ActionExecutionCtx for BuckActionExecutionContext<'_> {
407407
self.inputs
408408
.iter()
409409
.filter(|(ag, _)| {
410-
if !ag.uses_content_based_path() {
410+
if !ag.path_resolution_may_require_artifact_value() {
411411
return false;
412412
}
413413

app/buck2_build_api/src/artifact_groups.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,22 +49,22 @@ impl PromiseArtifactWrapper {
4949
}
5050

5151
#[derive(Clone, Debug, Display, Dupe, PartialEq, Eq, Hash, Allocative)]
52-
#[display("{} {}", key, has_content_based_path)]
52+
#[display("{} {}", key, path_resolution_may_require_artifact_value)]
5353
pub struct TransitiveSetProjectionWrapper {
5454
pub key: TransitiveSetProjectionKey,
55-
pub has_content_based_path: bool,
55+
pub path_resolution_may_require_artifact_value: bool,
5656
pub is_eligible_for_dedupe: bool,
5757
}
5858

5959
impl TransitiveSetProjectionWrapper {
6060
pub fn new(
6161
key: TransitiveSetProjectionKey,
62-
has_content_based_path: bool,
62+
path_resolution_may_require_artifact_value: bool,
6363
is_eligible_for_dedupe: bool,
6464
) -> Self {
6565
Self {
6666
key,
67-
has_content_based_path,
67+
path_resolution_may_require_artifact_value,
6868
is_eligible_for_dedupe,
6969
}
7070
}
@@ -117,10 +117,12 @@ impl ArtifactGroup {
117117
})
118118
}
119119

120-
pub fn uses_content_based_path(&self) -> bool {
120+
pub fn path_resolution_may_require_artifact_value(&self) -> bool {
121121
match self {
122-
ArtifactGroup::Artifact(a) => a.has_content_based_path(),
123-
ArtifactGroup::TransitiveSetProjection(a) => a.has_content_based_path,
122+
ArtifactGroup::Artifact(a) => a.path_resolution_requires_artifact_value(),
123+
ArtifactGroup::TransitiveSetProjection(a) => {
124+
a.path_resolution_may_require_artifact_value
125+
}
124126
ArtifactGroup::Promise(p) => p.has_content_based_path,
125127
}
126128
}

0 commit comments

Comments
 (0)