Skip to content

Commit 036aed5

Browse files
Ian Childsfacebook-github-bot
Ian Childs
authored and
facebook-github-bot
committed
Add CONTENT_HASH option for writing paths to buck-out
Summary: Right now, we always write build artifacts to paths with a configuration hash. Now, a path can be specified as wanting to be written to a path with its content-based hash, and in that case we do not include the configuration hash. Instead, we require the caller to specify the content-based hash and we write it _after_ the package, before the short name. Reviewed By: JakobDegen Differential Revision: D72249286 fbshipit-source-id: cb882fa42caa9f0b7775e7925f5a5665800d11db
1 parent 45eb9b6 commit 036aed5

File tree

2 files changed

+123
-28
lines changed

2 files changed

+123
-28
lines changed

‎app/buck2_core/src/deferred/base_deferred_key.rs

Lines changed: 71 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ use dupe::Dupe;
2424
use static_assertions::assert_eq_size;
2525
use strong_hash::StrongHash;
2626

27+
use crate::content_hash::ContentBasedPathHash;
28+
use crate::fs::buck_out_path::BuckOutPathKind;
2729
use crate::fs::paths::forward_rel_path::ForwardRelativePath;
2830
use crate::fs::paths::forward_rel_path::ForwardRelativePathBuf;
2931
use crate::fs::project_rel_path::ProjectRelativePath;
@@ -60,6 +62,13 @@ impl PartialEq for BaseDeferredKeyBxl {
6062
}
6163
}
6264

65+
#[derive(Debug, buck2_error::Error)]
66+
#[buck2(tag = Input)]
67+
pub enum PathResolutionError {
68+
#[error("Tried to resolve a content-based path {0} without providing the content hash!")]
69+
ContentBasedPathWithNoContentHash(ForwardRelativePathBuf),
70+
}
71+
6372
#[derive(Debug, derive_more::Display, Dupe, Clone, Allocative)]
6473
pub enum BaseDeferredKey {
6574
TargetLabel(ConfiguredTargetLabel),
@@ -132,6 +141,8 @@ impl BaseDeferredKey {
132141
action_key: Option<&str>,
133142
path: &ForwardRelativePath,
134143
fully_hash_path: bool,
144+
path_resolution_method: BuckOutPathKind,
145+
content_hash: Option<&ContentBasedPathHash>,
135146
) -> buck2_error::Result<ProjectRelativePathBuf> {
136147
match self {
137148
BaseDeferredKey::TargetLabel(target) => {
@@ -141,33 +152,66 @@ impl BaseDeferredKey {
141152
// It is performance critical that we use slices and allocate via `join` instead of
142153
// repeated calls to `join` on the path object because `join` allocates on each call,
143154
// which has a significant impact.
144-
let path_identifier = [
145-
target.cfg().output_hash().as_str(),
146-
if target.exec_cfg().is_some() { "-" } else { "" },
147-
target
148-
.exec_cfg()
149-
.as_ref()
150-
.map_or("", |x| x.output_hash().as_str()),
151-
"/",
152-
cell_relative_path,
153-
if cell_relative_path.is_empty() {
154-
""
155-
} else {
156-
"/"
157-
},
158-
"__",
159-
escaped_target_name.as_ref(),
160-
"__",
161-
"/",
162-
if action_key.is_none() {
163-
""
164-
} else {
165-
"__action__"
166-
},
167-
action_key.unwrap_or_default(),
168-
if action_key.is_none() { "" } else { "__/" },
169-
];
170-
155+
let path_identifier = match path_resolution_method {
156+
BuckOutPathKind::Configuration => [
157+
target.cfg().output_hash().as_str(),
158+
if target.exec_cfg().is_some() { "-" } else { "" },
159+
target
160+
.exec_cfg()
161+
.as_ref()
162+
.map_or("", |x| x.output_hash().as_str()),
163+
"/",
164+
cell_relative_path,
165+
if cell_relative_path.is_empty() {
166+
""
167+
} else {
168+
"/"
169+
},
170+
"__",
171+
escaped_target_name.as_ref(),
172+
"__",
173+
"/",
174+
if action_key.is_none() {
175+
""
176+
} else {
177+
"__action__"
178+
},
179+
action_key.unwrap_or_default(),
180+
if action_key.is_none() { "" } else { "__/" },
181+
],
182+
BuckOutPathKind::ContentHash => {
183+
let content_hash = content_hash.as_ref().map(|x| x.as_str());
184+
if let Some(content_hash) = content_hash {
185+
[
186+
cell_relative_path,
187+
if cell_relative_path.is_empty() {
188+
""
189+
} else {
190+
"/"
191+
},
192+
"__",
193+
escaped_target_name.as_ref(),
194+
"__",
195+
"/",
196+
if action_key.is_none() {
197+
""
198+
} else {
199+
"__action__"
200+
},
201+
action_key.unwrap_or_default(),
202+
if action_key.is_none() { "" } else { "__/" },
203+
content_hash,
204+
"/",
205+
"",
206+
"",
207+
]
208+
} else {
209+
return Err(PathResolutionError::ContentBasedPathWithNoContentHash(
210+
path.to_buf(),
211+
))?;
212+
}
213+
}
214+
};
171215
let path_or_hash = if fully_hash_path {
172216
let mut hasher = DefaultHasher::new();
173217
path_identifier.hash(&mut hasher);

‎app/buck2_core/src/fs/buck_out_path.rs

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use itertools::Itertools;
2020
use crate::category::CategoryRef;
2121
use crate::cells::external::ExternalCellOrigin;
2222
use crate::cells::paths::CellRelativePath;
23+
use crate::content_hash::ContentBasedPathHash;
2324
use crate::deferred::base_deferred_key::BaseDeferredKey;
2425
use crate::deferred::key::DeferredHolderKey;
2526
use crate::fs::dynamic_actions_action_key::DynamicActionsActionKey;
@@ -31,6 +32,31 @@ use crate::provider::label::ConfiguredProvidersLabel;
3132
use crate::provider::label::NonDefaultProvidersName;
3233
use crate::provider::label::ProvidersName;
3334

35+
#[derive(
36+
Copy,
37+
Clone,
38+
Debug,
39+
Display,
40+
Allocative,
41+
Hash,
42+
Eq,
43+
PartialEq,
44+
strong_hash::StrongHash
45+
)]
46+
pub enum BuckOutPathKind {
47+
/// A path that contains the configuration of the owning target.
48+
Configuration,
49+
50+
/// A path that contains the content hash of the artifact stored at the path.
51+
ContentHash,
52+
}
53+
54+
impl Default for BuckOutPathKind {
55+
fn default() -> Self {
56+
Self::Configuration
57+
}
58+
}
59+
3460
#[derive(
3561
Clone,
3662
Debug,
@@ -47,6 +73,8 @@ struct BuildArtifactPathData {
4773
owner: DeferredHolderKey,
4874
/// The path relative to that target.
4975
path: Box<ForwardRelativePath>,
76+
/// How the path is resolved
77+
path_resolution_method: BuckOutPathKind,
5078
}
5179

5280
/// Represents a resolvable path corresponding to outputs of rules that are part
@@ -81,6 +109,7 @@ impl BuildArtifactPath {
81109
BuildArtifactPath(Arc::new(BuildArtifactPathData {
82110
owner,
83111
path: path.into_box(),
112+
path_resolution_method: BuckOutPathKind::Configuration,
84113
}))
85114
}
86115

@@ -95,6 +124,10 @@ impl BuildArtifactPath {
95124
pub fn path(&self) -> &ForwardRelativePath {
96125
&self.0.path
97126
}
127+
128+
pub fn path_resolution_method(&self) -> BuckOutPathKind {
129+
self.0.path_resolution_method
130+
}
98131
}
99132

100133
#[derive(Clone, Debug, Display, Eq, PartialEq)]
@@ -221,6 +254,8 @@ impl BuckOutPathResolver {
221254
.map(|x| x.as_str()),
222255
path.path(),
223256
false,
257+
path.path_resolution_method(),
258+
None,
224259
)
225260
}
226261

@@ -236,6 +271,8 @@ impl BuckOutPathResolver {
236271
.map(|x| x.as_str()),
237272
path.path(),
238273
false,
274+
path.path_resolution_method(),
275+
None,
239276
)
240277
}
241278

@@ -274,6 +311,8 @@ impl BuckOutPathResolver {
274311
&path.path,
275312
// Fully hash scratch path as it can be very long and cause path too long issue on Windows.
276313
true,
314+
BuckOutPathKind::Configuration,
315+
None,
277316
)
278317
}
279318

@@ -311,6 +350,8 @@ impl BuckOutPathResolver {
311350
None,
312351
&path,
313352
true,
353+
BuckOutPathKind::Configuration,
354+
None,
314355
)
315356
}
316357

@@ -321,8 +362,18 @@ impl BuckOutPathResolver {
321362
action_key: Option<&str>,
322363
path: &ForwardRelativePath,
323364
fully_hash_path: bool,
365+
path_resolution_method: BuckOutPathKind,
366+
content_hash: Option<&ContentBasedPathHash>,
324367
) -> buck2_error::Result<ProjectRelativePathBuf> {
325-
owner.make_hashed_path(&self.buck_out_v2, prefix, action_key, path, fully_hash_path)
368+
owner.make_hashed_path(
369+
&self.buck_out_v2,
370+
prefix,
371+
action_key,
372+
path,
373+
fully_hash_path,
374+
path_resolution_method,
375+
content_hash,
376+
)
326377
}
327378

328379
/// This function returns the exact location of the symlink of a given target.

0 commit comments

Comments
 (0)