Skip to content

Commit c21d1fa

Browse files
Ian Childsfacebook-github-bot
Ian Childs
authored and
facebook-github-bot
committed
Wire up all callsites that need to account for content-based hashes and leave TODOs
Summary: Content-based path hashing requires that you pass in the content-based hash when you want to resolve the path. So, wire up all the callsites that need to do that and add a TODO for them to actually pass in a content-based hash correctly. Reviewed By: JakobDegen Differential Revision: D72457374 fbshipit-source-id: fd005eabae4b7831558eee7bc6d9751480cde57b
1 parent 283f87b commit c21d1fa

File tree

37 files changed

+238
-84
lines changed

37 files changed

+238
-84
lines changed

‎app/buck2_action_impl/src/actions/impls/cas_artifact.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,8 @@ impl Action for CasArtifactAction {
291291
}
292292
};
293293

294-
let path = ctx.fs().resolve_build(self.output.get_path())?;
294+
// TODO(T219919866) Add support for experimental content-based path hashing
295+
let path = ctx.fs().resolve_build(self.output.get_path(), None)?;
295296
ctx.materializer()
296297
.declare_cas_many(
297298
Arc::new(CasDownloadInfo::new_declared(self.inner.re_use_case)),

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,9 @@ impl Action for CopyAction {
157157
.buck_error_context("Input did not dereference to exactly one artifact")?;
158158

159159
let artifact_fs = ctx.fs();
160-
let src = input.resolve_path(artifact_fs)?;
161-
let dest = artifact_fs.resolve_build(self.output().get_path())?;
160+
// TODO(T219919866) Add support for experimental content-based path hashing
161+
let src = input.resolve_path(artifact_fs, None)?;
162+
let dest = artifact_fs.resolve_build(self.output().get_path(), None)?;
162163

163164
let value = {
164165
let fs = artifact_fs.fs();

‎app/buck2_action_impl/src/actions/impls/download_file.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,8 @@ impl Action for DownloadFileAction {
279279
match self.declared_metadata(&client, ctx.digest_config()).await? {
280280
Some(metadata) => {
281281
let artifact_fs = ctx.fs();
282-
let rel_path = artifact_fs.resolve_build(self.output().get_path())?;
282+
// TODO(T219919866) Add support for experimental content-based path hashing
283+
let rel_path = artifact_fs.resolve_build(self.output().get_path(), None)?;
283284

284285
// Fast path: download later via the materializer.
285286
ctx.materializer()
@@ -302,7 +303,8 @@ impl Action for DownloadFileAction {
302303

303304
let artifact_fs = ctx.fs();
304305
let project_fs = artifact_fs.fs();
305-
let rel_path = artifact_fs.resolve_build(self.output().get_path())?;
306+
// TODO(T219919866) Add support for experimental content-based path hashing
307+
let rel_path = artifact_fs.resolve_build(self.output().get_path(), None)?;
306308

307309
// Slow path: download now.
308310
let digest = http_download(

‎app/buck2_action_impl/src/actions/impls/offline.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@ pub(crate) async fn declare_copy_to_offline_output_cache(
2626
output: &BuildArtifact,
2727
value: ArtifactValue,
2828
) -> buck2_error::Result<ProjectRelativePathBuf> {
29-
let build_path = ctx.fs().resolve_build(output.get_path())?;
29+
// TODO(T219919866) Add support for experimental content-based path hashing
30+
let build_path = ctx.fs().resolve_build(output.get_path(), None)?;
3031
let offline_cache_path = ctx
3132
.fs()
32-
.resolve_offline_output_cache_path(output.get_path())?;
33+
.resolve_offline_output_cache_path(output.get_path(), None)?;
3334
declare_copy_materialization(ctx, build_path, offline_cache_path.clone(), value).await?;
3435

3536
Ok(offline_cache_path)
@@ -43,9 +44,10 @@ pub(crate) async fn declare_copy_from_offline_cache(
4344
ctx: &mut dyn ActionExecutionCtx,
4445
output: &BuildArtifact,
4546
) -> buck2_error::Result<ActionOutputs> {
47+
// TODO(T219919866) Add support for experimental content-based path hashing
4648
let offline_cache_path = ctx
4749
.fs()
48-
.resolve_offline_output_cache_path(output.get_path())?;
50+
.resolve_offline_output_cache_path(output.get_path(), None)?;
4951

5052
let (value, _hashing_time) = build_entry_from_disk(
5153
ctx.fs().fs().resolve(&offline_cache_path),
@@ -69,7 +71,8 @@ pub(crate) async fn declare_copy_from_offline_cache(
6971
});
7072
let value = ArtifactValue::from(entry);
7173

72-
let build_path = ctx.fs().resolve_build(output.get_path())?;
74+
// TODO(T219919866) Add support for experimental content-based path hashing
75+
let build_path = ctx.fs().resolve_build(output.get_path(), None)?;
7376
declare_copy_materialization(ctx, offline_cache_path, build_path, value.dupe()).await?;
7477

7578
Ok(ActionOutputs::from_single(output.get_path().dupe(), value))

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ use buck2_core::execution_types::executor_config::MetaInternalExtraParams;
4343
use buck2_core::execution_types::executor_config::RemoteExecutorCustomImage;
4444
use buck2_core::execution_types::executor_config::RemoteExecutorDependency;
4545
use buck2_core::fs::artifact_path_resolver::ArtifactFs;
46+
use buck2_core::fs::buck_out_path::BuckOutPathKind;
4647
use buck2_core::fs::buck_out_path::BuildArtifactPath;
4748
use buck2_core::fs::fs_util;
4849
use buck2_core::fs::paths::forward_rel_path::ForwardRelativePathBuf;
@@ -533,10 +534,14 @@ impl RunAction {
533534
extra_env: &mut Vec<(String, String)>,
534535
) -> buck2_error::Result<()> {
535536
if let Some(metadata_param) = &self.inner.metadata_param {
536-
let path =
537-
BuildArtifactPath::new(ctx.target().owner().dupe(), metadata_param.path.clone());
537+
let path = BuildArtifactPath::new(
538+
ctx.target().owner().dupe(),
539+
metadata_param.path.clone(),
540+
BuckOutPathKind::default(),
541+
);
542+
// TODO(T219919866) Add support for experimental content-based path hashing
538543
let env = cli_ctx
539-
.resolve_project_path(fs.buck_out_path_resolver().resolve_gen(&path)?)?
544+
.resolve_project_path(fs.buck_out_path_resolver().resolve_gen(&path, None)?)?
540545
.into_string();
541546
let (data, digest) = metadata_content(fs, artifact_inputs, ctx.digest_config())?;
542547
inputs.push(CommandExecutionInput::ActionMetadata(ActionMetadataBlob {
@@ -628,7 +633,8 @@ impl RunAction {
628633
.map(|artifact| {
629634
artifact
630635
.artifact()
631-
.and_then(|a| a.get_path().resolve(ctx.fs()))
636+
// TODO(T219919866) Add support for experimental content-based path hashing
637+
.and_then(|a| a.get_path().resolve(ctx.fs(), None))
632638
})
633639
.collect::<buck2_error::Result<Vec<_>>>()?;
634640

@@ -868,7 +874,8 @@ impl Action for RunAction {
868874

869875
for x in self.starlark_values.outputs_for_error_handler.iter() {
870876
let artifact = (*x.artifact()?).dupe().ensure_bound()?.into_artifact();
871-
let path = artifact.get_path().resolve(artifact_fs)?;
877+
// TODO(T219919866) Add support for experimental content-based path hashing
878+
let path = artifact.get_path().resolve(artifact_fs, None)?;
872879

873880
let abs = artifact_fs.fs().resolve(&path);
874881
// Check if the output file specified exists. We will return an error if it doesn't

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -772,7 +772,13 @@ async fn outputs_match(
772772
let output_matches = previous_state
773773
.result
774774
.iter()
775-
.map(|(path, value)| Ok((fs.buck_out_path_resolver().resolve_gen(path)?, value.dupe())))
775+
// TODO(T219919866) Add support for experimental content-based path hashing
776+
.map(|(path, value)| {
777+
Ok((
778+
fs.buck_out_path_resolver().resolve_gen(path, None)?,
779+
value.dupe(),
780+
))
781+
})
776782
.collect::<buck2_error::Result<Vec<(ProjectRelativePathBuf, ArtifactValue)>>>()?;
777783

778784
let materializer_accepts = ctx
@@ -1231,7 +1237,8 @@ impl DeclaredDepFiles {
12311237

12321238
for declared_dep_file in self.tagged.values() {
12331239
let dep_file = &declared_dep_file.output;
1234-
let path = dep_file.resolve_path(fs).map_err(|e| {
1240+
// TODO(T219919866) Add support for experimental content-based path hashing
1241+
let path = dep_file.resolve_path(fs, None).map_err(|e| {
12351242
MaterializeDepFilesError::MaterializationFailed { source: e.into() }
12361243
})?;
12371244
paths.push(path);
@@ -1276,7 +1283,8 @@ impl DeclaredDepFiles {
12761283
for declared_dep_file in self.tagged.values() {
12771284
let mut selector = DirectorySelector::empty();
12781285

1279-
let dep_file = declared_dep_file.output.resolve_path(fs)?;
1286+
// TODO(T219919866) Add support for experimental content-based path hashing
1287+
let dep_file = declared_dep_file.output.resolve_path(fs, None)?;
12801288

12811289
let read_dep_file: buck2_error::Result<()> = try {
12821290
let dep_file_path = fs.fs().resolve(&dep_file);

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,8 @@ impl Action for SymlinkedDirAction {
224224
ctx: &mut dyn ActionExecutionCtx,
225225
) -> Result<(ActionOutputs, ActionExecutionMetadata), ExecuteError> {
226226
let fs = ctx.fs().fs();
227-
let output = ctx.fs().resolve_build(self.output().get_path())?;
227+
// TODO(T219919866) Add support for experimental content-based path hashing
228+
let output = ctx.fs().resolve_build(self.output().get_path(), None)?;
228229
let mut builder = ArtifactValueBuilder::new(fs, ctx.digest_config());
229230
let mut srcs = Vec::new();
230231

@@ -235,7 +236,8 @@ impl Action for SymlinkedDirAction {
235236
.into_singleton()
236237
.buck_error_context("Input did not dereference to exactly one artifact")?;
237238

238-
let src = src_artifact.resolve_path(ctx.fs())?;
239+
// TODO(T219919866) Add support for experimental content-based path hashing
240+
let src = src_artifact.resolve_path(ctx.fs(), None)?;
239241
let dest = output.join(dest);
240242

241243
if self.copy {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,8 @@ impl Action for WriteAction {
191191
execution_start = Some(Instant::now());
192192
let content = self.get_contents(&ctx.executor_fs())?.into_bytes();
193193
Ok(vec![WriteRequest {
194-
path: fs.resolve_build(self.output.get_path())?,
194+
// TODO(T219919866) Add support for experimental content-based path hashing
195+
path: fs.resolve_build(self.output.get_path(), None)?,
195196
content,
196197
is_executable: self.inner.is_executable,
197198
}])

‎app/buck2_action_impl/src/actions/impls/write_json.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,8 @@ impl Action for WriteJsonAction {
208208
execution_start = Some(Instant::now());
209209
let content = self.get_contents(&ctx.executor_fs())?;
210210
Ok(vec![WriteRequest {
211-
path: fs.resolve_build(self.output.get_path())?,
211+
// TODO(T219919866) Add support for experimental content-based path hashing
212+
path: fs.resolve_build(self.output.get_path(), None)?,
212213
content,
213214
is_executable: false,
214215
}])

‎app/buck2_action_impl/src/actions/impls/write_macros.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@ impl Action for WriteMacrosToFileAction {
176176
std::iter::zip(self.outputs.iter(), output_contents.into_iter())
177177
.map(|(output, content)| {
178178
Ok(WriteRequest {
179-
path: fs.fs().resolve_build(output.get_path())?,
179+
// TODO(T219919866) Add support for experimental content-based path hashing
180+
path: fs.fs().resolve_build(output.get_path(), None)?,
180181
content: content.into_bytes(),
181182
is_executable: false,
182183
})

‎app/buck2_action_impl/src/dynamic/deferred.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,8 @@ async fn materialize_inputs(
388388
let mut paths = Vec::with_capacity(materialized_artifacts.len());
389389

390390
for artifact in materialized_artifacts {
391-
let path = artifact.resolve_path(&artifact_fs)?;
391+
// TODO(T219919866) Add support for experimental content-based path hashing
392+
let path = artifact.resolve_path(&artifact_fs, None)?;
392393
paths.push(path.clone());
393394
}
394395

@@ -453,7 +454,8 @@ fn artifact_values<'v>(
453454
let mut artifact_values_dict = Vec::with_capacity(artifact_values.len());
454455
for x in artifact_values {
455456
let k = StarlarkArtifact::new(x.dupe());
456-
let path = x.get_path().resolve(artifact_fs)?;
457+
// TODO(T219919866) Add support for experimental content-based path hashing
458+
let path = x.get_path().resolve(artifact_fs, None)?;
457459
// `InputArtifactsMaterialized` marker indicates that the artifact is materialized.
458460
let v = StarlarkArtifactValue::new(x.dupe(), path.to_owned(), artifact_fs.fs().dupe());
459461
artifact_values_dict.push((k, v));
@@ -500,7 +502,8 @@ fn new_attr_value<'v>(
500502
Ok(env.heap().alloc(StarlarkOutputArtifact::new(artifact)))
501503
}
502504
DynamicAttrValue::ArtifactValue(artifact) => {
503-
let path = artifact.get_path().resolve(&artifact_fs)?;
505+
// TODO(T219919866) Add support for experimental content-based path hashing
506+
let path = artifact.get_path().resolve(&artifact_fs, None)?;
504507
// `InputArtifactsMaterialized` marker indicates that the artifact is materialized.
505508
Ok(env.heap().alloc(StarlarkArtifactValue::new(
506509
Artifact::from(artifact.dupe()),

‎app/buck2_artifact/src/artifact/artifact_type.rs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use std::rc::Rc;
1818
use std::sync::Arc;
1919

2020
use allocative::Allocative;
21+
use buck2_core::content_hash::ContentBasedPathHash;
2122
use buck2_core::deferred::base_deferred_key::BaseDeferredKey;
2223
use buck2_core::fs::artifact_path_resolver::ArtifactFs;
2324
use buck2_core::fs::buck_out_path::BuildArtifactPath;
@@ -188,8 +189,12 @@ impl Artifact {
188189
}
189190

190191
impl ArtifactDyn for Artifact {
191-
fn resolve_path(&self, fs: &ArtifactFs) -> buck2_error::Result<ProjectRelativePathBuf> {
192-
self.get_path().resolve(fs)
192+
fn resolve_path(
193+
&self,
194+
fs: &ArtifactFs,
195+
content_hash: Option<&ContentBasedPathHash>,
196+
) -> buck2_error::Result<ProjectRelativePathBuf> {
197+
self.get_path().resolve(fs, content_hash)
193198
}
194199

195200
fn requires_materialization(&self, fs: &ArtifactFs) -> bool {
@@ -536,6 +541,7 @@ impl UnboundArtifact {
536541
pub mod testing {
537542
use buck2_core::deferred::base_deferred_key::BaseDeferredKey;
538543
use buck2_core::deferred::key::DeferredHolderKey;
544+
use buck2_core::fs::buck_out_path::BuckOutPathKind;
539545
use buck2_core::fs::buck_out_path::BuildArtifactPath;
540546
use buck2_core::fs::paths::forward_rel_path::ForwardRelativePath;
541547
use buck2_core::target::configured_target_label::ConfiguredTargetLabel;
@@ -595,6 +601,7 @@ pub mod testing {
595601
BuildArtifactPath::new(
596602
BaseDeferredKey::TargetLabel(target.dupe()),
597603
ForwardRelativePath::new(path).unwrap().to_buf(),
604+
BuckOutPathKind::default(),
598605
),
599606
ActionKey::new(
600607
DeferredHolderKey::Base(BaseDeferredKey::TargetLabel(target)),
@@ -617,6 +624,7 @@ mod tests {
617624
use buck2_core::deferred::base_deferred_key::BaseDeferredKey;
618625
use buck2_core::deferred::key::DeferredHolderKey;
619626
use buck2_core::fs::artifact_path_resolver::ArtifactFs;
627+
use buck2_core::fs::buck_out_path::BuckOutPathKind;
620628
use buck2_core::fs::buck_out_path::BuckOutPathResolver;
621629
use buck2_core::fs::buck_out_path::BuildArtifactPath;
622630
use buck2_core::fs::fs_util;
@@ -650,6 +658,7 @@ mod tests {
650658
BuildArtifactPath::new(
651659
BaseDeferredKey::TargetLabel(target.dupe()),
652660
ForwardRelativePathBuf::unchecked_new("bar.out".into()),
661+
BuckOutPathKind::default(),
653662
),
654663
OutputType::File,
655664
0,
@@ -703,7 +712,7 @@ mod tests {
703712
);
704713

705714
assert_eq!(
706-
Artifact::from(source).get_path().resolve(&fs)?,
715+
Artifact::from(source).get_path().resolve(&fs, None)?,
707716
ProjectRelativePath::unchecked_new("cell_path/pkg/src.cpp")
708717
);
709718

@@ -732,20 +741,20 @@ mod tests {
732741
BuckOutPathResolver::new(ProjectRelativePathBuf::unchecked_new("buck_out".into())),
733742
project_fs.dupe(),
734743
);
735-
let expected_path1 = project_fs.resolve(fs.resolve_build(artifact1.get_path())?);
736-
let expected_path2 = project_fs.resolve(fs.resolve_build(artifact2.get_path())?);
744+
let expected_path1 = project_fs.resolve(fs.resolve_build(artifact1.get_path(), None)?);
745+
let expected_path2 = project_fs.resolve(fs.resolve_build(artifact2.get_path(), None)?);
737746

738-
let dest_path = fs.resolve_build(artifact1.get_path())?;
747+
let dest_path = fs.resolve_build(artifact1.get_path(), None)?;
739748
fs.fs().write_file(&dest_path, "artifact1", false)?;
740749

741750
assert_eq!("artifact1", fs_util::read_to_string(&expected_path1)?);
742751

743-
let dest_path = fs.resolve_build(artifact2.get_path())?;
752+
let dest_path = fs.resolve_build(artifact2.get_path(), None)?;
744753
fs.fs().write_file(&dest_path, "artifact2", true)?;
745754

746755
assert_eq!("artifact2", fs_util::read_to_string(&expected_path2)?);
747756

748-
let dest_path = fs.resolve_build(artifact3.get_path())?;
757+
let dest_path = fs.resolve_build(artifact3.get_path(), None)?;
749758
fs.fs()
750759
.write_file(&dest_path, "artifact3", false)
751760
.expect_err("should fail because bar.cpp is a file");

‎app/buck2_audit_server/src/classpath.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ impl ServerAuditSubcommand for AuditClasspathCommand {
8484
let classpaths: buck2_error::Result<Vec<_>> = label_to_artifact
8585
.into_values()
8686
.map(|artifact| {
87-
let path = artifact.get_path().resolve(&artifact_fs)?;
87+
// TODO(T219919866) Add support for experimental content-based path hashing
88+
let path =
89+
artifact.get_path().resolve(&artifact_fs, None)?;
8890
buck2_error::Ok(artifact_fs.fs().resolve(&path))
8991
})
9092
.collect();
@@ -107,7 +109,8 @@ impl ServerAuditSubcommand for AuditClasspathCommand {
107109
)
108110
.await?;
109111
for (_label, artifact) in label_to_artifact {
110-
let path = artifact.get_path().resolve(&artifact_fs)?;
112+
// TODO(T219919866) Add support for experimental content-based path hashing
113+
let path = artifact.get_path().resolve(&artifact_fs, None)?;
111114
let abs_path = artifact_fs.fs().resolve(&path);
112115
writeln!(stdout, "{}", &abs_path)?;
113116
}

0 commit comments

Comments
 (0)