Skip to content

Commit c169b66

Browse files
authored
feat(build-dir): Reorganize build-dir layout (#15947)
### What does this PR try to resolve? This PR re-organizes the `build-dir` file layout structure to a layout organized by "build unit" when `-Zbuild-dir-new-layout` is enabled. See #15010 for the motivations and design discussions. Below is file structure generated for a `foo` crate with a single dependency on `syn`. ``` $ tree -a target target ├── CACHEDIR.TAG ├── debug │   ├── build │   │   ├── foo │   │   │   └── 9f7d8db454c2e3a0 │   │   │   ├── deps │   │   │   │   ├── foo-9f7d8db454c2e3a0 │   │   │   │   └── foo-9f7d8db454c2e3a0.d │   │   │   └── fingerprint │   │   │   ├── bin-foo │   │   │   ├── bin-foo.json │   │   │   ├── dep-bin-foo │   │   │   └── invoked.timestamp │   │   ├── proc-macro2 │   │   │   ├── 39344080ec273066 │   │   │   │   ├── deps │   │   │   │   │   ├── libproc_macro2-39344080ec273066.rlib │   │   │   │   │   ├── libproc_macro2-39344080ec273066.rmeta │   │   │   │   │   └── proc_macro2-39344080ec273066.d │   │   │   │   └── fingerprint │   │   │   │   ├── dep-lib-proc_macro2 │   │   │   │   ├── invoked.timestamp │   │   │   │   ├── lib-proc_macro2 │   │   │   │   └── lib-proc_macro2.json │   │   │   ├── 8fc259c340d09182 │   │   │   │   ├── build-script │   │   │   │   │   ├── build-script-build │   │   │   │   │   ├── build_script_build-8fc259c340d09182 │   │   │   │   │   └── build_script_build-8fc259c340d09182.d │   │   │   │   ├── deps │   │   │   │   └── fingerprint │   │   │   │   ├── build-script-build-script-build │   │   │   │   ├── build-script-build-script-build.json │   │   │   │   ├── dep-build-script-build-script-build │   │   │   │   └── invoked.timestamp │   │   │   └── a6c299e456bc1663 │   │   │   ├── build-script-execution │   │   │   │   ├── invoked.timestamp │   │   │   │   ├── out │   │   │   │   ├── output │   │   │   │   ├── root-output │   │   │   │   └── stderr │   │   │   ├── deps │   │   │   └── fingerprint │   │   │   ├── run-build-script-build-script-build │   │   │   └── run-build-script-build-script-build.json │   │   ├── quote │   │   │   └── e95944fa29b3b7d5 │   │   │   ├── deps │   │   │   │   ├── libquote-e95944fa29b3b7d5.rlib │   │   │   │   ├── libquote-e95944fa29b3b7d5.rmeta │   │   │   │   └── quote-e95944fa29b3b7d5.d │   │   │   └── fingerprint │   │   │   ├── dep-lib-quote │   │   │   ├── invoked.timestamp │   │   │   ├── lib-quote │   │   │   └── lib-quote.json │   │   ├── syn │   │   │   └── a0dd360a8d0d85f9 │   │   │   ├── deps │   │   │   │   ├── libsyn-a0dd360a8d0d85f9.rlib │   │   │   │   ├── libsyn-a0dd360a8d0d85f9.rmeta │   │   │   │   └── syn-a0dd360a8d0d85f9.d │   │   │   └── fingerprint │   │   │   ├── dep-lib-syn │   │   │   ├── invoked.timestamp │   │   │   ├── lib-syn │   │   │   └── lib-syn.json │   │   └── unicode-ident │   │   └── 90d3c33eeb6b75ae │   │   ├── deps │   │   │   ├── libunicode_ident-90d3c33eeb6b75ae.rlib │   │   │   ├── libunicode_ident-90d3c33eeb6b75ae.rmeta │   │   │   └── unicode_ident-90d3c33eeb6b75ae.d │   │   └── fingerprint │   │   ├── dep-lib-unicode_ident │   │   ├── invoked.timestamp │   │   ├── lib-unicode_ident │   │   └── lib-unicode_ident.json │   ├── .cargo-lock │   ├── examples │   ├── foo │   ├── foo.d │   └── incremental │   └── foo-2476dro80nq0n │   ├── s-hbvjw5x5fa-1d58onh-7sam1xj7vrwy85uyr70f3vu2p │   │   ├── 1z96gpvj66dpsd06phiqkocsr.o │   │   ├── 8jaqpsn916x4kajeuk1b7rl12.o │   │   ├── 8sv45ek5gkoxjdcr3b6nbgu2o.o │   │   ├── 9yqg9fn9x870xvia9frlvvazg.o │   │   ├── a5v36uvuoyp00oyvscfno2pce.o │   │   ├── c2q7ff0mmsk4s1yg7s5ei1ryr.o │   │   ├── dep-graph.bin │   │   ├── query-cache.bin │   │   └── work-products.bin │   └── s-hbvjw5x5fa-1d58onh.lock └── .rustc_info.json 36 directories, 62 files ``` For why incremental is where it is, see #15947 (comment) As a side effect, we pass a lot more parameters to rustc, likely making `cargo -v` more annoying, similar to #13941. ### How to test and review this PR? This PR still needs to be more thoroughly tested. Thus far I have been testing on simple test crates. Also see #15874 for potential test harness improvements that could be used by this PR. ### Follow up actions - [ ] Remove redundant `-Cextra-filename` in files where possible. - [ ] See if we can drop `<platform>/<profile>` after #4282 - [ ] Re-evaluate if we want platform to be unconditionally included in the build-dir layout - [ ] Is `<pkgname>/<hash>` good enough or do we need to go with prefixes to reduce the number of items within a directory, see #15947 (comment) - [ ] Can we simplify how fingerpritns are stored, reducing pressure on path lenghts - [ ] Under the new layout, should `cargo clean -p` also cleans old layout paths? See #15947 (comment)
2 parents 13918cc + c89baf2 commit c169b66

File tree

9 files changed

+1537
-182
lines changed

9 files changed

+1537
-182
lines changed

crates/cargo-test-support/src/compare.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,9 @@ fn add_regex_redactions(subs: &mut snapbox::Redactions) {
235235
// Match file name hashes like `foo-06b451d0d6f88b1d`
236236
subs.insert("[HASH]", regex!(r"[a-z0-9]+-(?<redacted>[a-f0-9]{16})"))
237237
.unwrap();
238+
// Match path hashes like `../06b451d0d6f88b1d/..` used in directory paths
239+
subs.insert("[HASH]", regex!(r"\/(?<redacted>[0-9a-f]{16})\/"))
240+
.unwrap();
238241
subs.insert(
239242
"[AVG_ELAPSED]",
240243
regex!(r"(?<redacted>[0-9]+(\.[0-9]+)?) ns/iter"),

src/cargo/core/compiler/build_runner/compilation_files.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -230,17 +230,21 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
230230
self.export_dir.clone()
231231
}
232232

233-
/// Directory name to use for a package in the form `NAME-HASH`.
233+
/// Directory name to use for a package in the form `{NAME}/{HASH}`.
234234
///
235235
/// Note that some units may share the same directory, so care should be
236236
/// taken in those cases!
237237
fn pkg_dir(&self, unit: &Unit) -> String {
238+
let seperator = match self.ws.gctx().cli_unstable().build_dir_new_layout {
239+
true => "/",
240+
false => "-",
241+
};
238242
let name = unit.pkg.package_id().name();
239243
let meta = self.metas[unit];
240244
if let Some(c_extra_filename) = meta.c_extra_filename() {
241-
format!("{}-{}", name, c_extra_filename)
245+
format!("{}{}{}", name, seperator, c_extra_filename)
242246
} else {
243-
format!("{}-{}", name, self.target_short_hash(unit))
247+
format!("{}{}{}", name, seperator, self.target_short_hash(unit))
244248
}
245249
}
246250

@@ -255,20 +259,27 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
255259
}
256260

257261
/// Returns the host `deps` directory path.
258-
pub fn host_deps(&self) -> &Path {
259-
self.host.deps()
262+
pub fn host_deps(&self, unit: &Unit) -> PathBuf {
263+
let dir = self.pkg_dir(unit);
264+
self.host.deps(&dir)
260265
}
261266

262267
/// Returns the directories where Rust crate dependencies are found for the
263268
/// specified unit.
264-
pub fn deps_dir(&self, unit: &Unit) -> &Path {
265-
self.layout(unit.kind).deps()
269+
pub fn deps_dir(&self, unit: &Unit) -> PathBuf {
270+
let dir = self.pkg_dir(unit);
271+
self.layout(unit.kind).deps(&dir)
266272
}
267273

268274
/// Directory where the fingerprint for the given unit should go.
269275
pub fn fingerprint_dir(&self, unit: &Unit) -> PathBuf {
270276
let dir = self.pkg_dir(unit);
271-
self.layout(unit.kind).fingerprint().join(dir)
277+
self.layout(unit.kind).fingerprint(&dir)
278+
}
279+
280+
/// Directory where incremental output for the given unit should go.
281+
pub fn incremental_dir(&self, unit: &Unit) -> &Path {
282+
self.layout(unit.kind).incremental()
272283
}
273284

274285
/// Returns the path for a file in the fingerprint directory.
@@ -303,7 +314,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
303314
assert!(!unit.mode.is_run_custom_build());
304315
assert!(self.metas.contains_key(unit));
305316
let dir = self.pkg_dir(unit);
306-
self.layout(CompileKind::Host).build().join(dir)
317+
self.layout(CompileKind::Host).build_script(&dir)
307318
}
308319

309320
/// Returns the directory for compiled artifacts files.
@@ -337,7 +348,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
337348
assert!(unit.target.is_custom_build());
338349
assert!(unit.mode.is_run_custom_build());
339350
let dir = self.pkg_dir(unit);
340-
self.layout(unit.kind).build().join(dir)
351+
self.layout(unit.kind).build_script_execution(&dir)
341352
}
342353

343354
/// Returns the "`OUT_DIR`" directory for running a build script.

src/cargo/core/compiler/build_runner/mod.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::core::compiler::{self, Unit, artifact};
1010
use crate::util::cache_lock::CacheLockMode;
1111
use crate::util::errors::CargoResult;
1212
use anyhow::{Context as _, bail};
13+
use cargo_util::paths;
1314
use filetime::FileTime;
1415
use itertools::Itertools;
1516
use jobserver::Client;
@@ -401,9 +402,17 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
401402
self.compilation
402403
.root_output
403404
.insert(kind, layout.dest().to_path_buf());
404-
self.compilation
405-
.deps_output
406-
.insert(kind, layout.deps().to_path_buf());
405+
if self.bcx.gctx.cli_unstable().build_dir_new_layout {
406+
for (unit, _) in self.bcx.unit_graph.iter() {
407+
let dep_dir = self.files().deps_dir(unit);
408+
paths::create_dir_all(&dep_dir)?;
409+
self.compilation.deps_output.insert(kind, dep_dir);
410+
}
411+
} else {
412+
self.compilation
413+
.deps_output
414+
.insert(kind, layout.legacy_deps().to_path_buf());
415+
}
407416
}
408417
Ok(())
409418
}

src/cargo/core/compiler/layout.rs

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ pub struct Layout {
142142
/// Will be `None` when the build-dir and target-dir are the same path as we cannot
143143
/// lock the same path twice.
144144
_build_lock: Option<FileLock>,
145+
is_new_layout: bool,
145146
}
146147

147148
impl Layout {
@@ -156,6 +157,7 @@ impl Layout {
156157
target: Option<CompileTarget>,
157158
dest: &str,
158159
) -> CargoResult<Layout> {
160+
let is_new_layout = ws.gctx().cli_unstable().build_dir_new_layout;
159161
let mut root = ws.target_dir();
160162
let mut build_root = ws.build_dir();
161163
if let Some(target) = target {
@@ -212,14 +214,17 @@ impl Layout {
212214
dest,
213215
_lock: lock,
214216
_build_lock: build_lock,
217+
is_new_layout,
215218
})
216219
}
217220

218221
/// Makes sure all directories stored in the Layout exist on the filesystem.
219222
pub fn prepare(&mut self) -> CargoResult<()> {
220-
paths::create_dir_all(&self.deps)?;
223+
if !self.is_new_layout {
224+
paths::create_dir_all(&self.deps)?;
225+
paths::create_dir_all(&self.fingerprint)?;
226+
}
221227
paths::create_dir_all(&self.incremental)?;
222-
paths::create_dir_all(&self.fingerprint)?;
223228
paths::create_dir_all(&self.examples)?;
224229
paths::create_dir_all(&self.build_examples)?;
225230
paths::create_dir_all(&self.build)?;
@@ -232,7 +237,15 @@ impl Layout {
232237
&self.dest
233238
}
234239
/// Fetch the deps path.
235-
pub fn deps(&self) -> &Path {
240+
pub fn deps(&self, pkg_dir: &str) -> PathBuf {
241+
if self.is_new_layout {
242+
self.build_unit(pkg_dir).join("deps")
243+
} else {
244+
self.legacy_deps().to_path_buf()
245+
}
246+
}
247+
/// Fetch the deps path. (old layout)
248+
pub fn legacy_deps(&self) -> &Path {
236249
&self.deps
237250
}
238251
/// Fetch the examples path.
@@ -256,17 +269,45 @@ impl Layout {
256269
&self.incremental
257270
}
258271
/// Fetch the fingerprint path.
259-
pub fn fingerprint(&self) -> &Path {
272+
pub fn fingerprint(&self, pkg_dir: &str) -> PathBuf {
273+
if self.is_new_layout {
274+
self.build_unit(pkg_dir).join("fingerprint")
275+
} else {
276+
self.legacy_fingerprint().to_path_buf().join(pkg_dir)
277+
}
278+
}
279+
/// Fetch the fingerprint path. (old layout)
280+
pub fn legacy_fingerprint(&self) -> &Path {
260281
&self.fingerprint
261282
}
262-
/// Fetch the build script path.
283+
/// Fetch the build path.
263284
pub fn build(&self) -> &Path {
264285
&self.build
265286
}
287+
/// Fetch the build script path.
288+
pub fn build_script(&self, pkg_dir: &str) -> PathBuf {
289+
if self.is_new_layout {
290+
self.build_unit(pkg_dir).join("build-script")
291+
} else {
292+
self.build().join(pkg_dir)
293+
}
294+
}
295+
/// Fetch the build script execution path.
296+
pub fn build_script_execution(&self, pkg_dir: &str) -> PathBuf {
297+
if self.is_new_layout {
298+
self.build_unit(pkg_dir).join("build-script-execution")
299+
} else {
300+
self.build().join(pkg_dir)
301+
}
302+
}
266303
/// Fetch the artifact path.
267304
pub fn artifact(&self) -> &Path {
268305
&self.artifact
269306
}
307+
/// Fetch the build unit path
308+
pub fn build_unit(&self, pkg_dir: &str) -> PathBuf {
309+
self.build().join(pkg_dir)
310+
}
270311
/// Create and return the tmp path.
271312
pub fn prepare_tmp(&self) -> CargoResult<&Path> {
272313
paths::create_dir_all(&self.tmp)?;

src/cargo/core/compiler/mod.rs

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ pub mod unit_dependencies;
5656
pub mod unit_graph;
5757

5858
use std::borrow::Cow;
59-
use std::collections::{HashMap, HashSet};
59+
use std::collections::{BTreeMap, HashMap, HashSet};
6060
use std::env;
6161
use std::ffi::{OsStr, OsString};
6262
use std::fmt::Display;
@@ -69,6 +69,7 @@ use std::sync::{Arc, LazyLock};
6969
use annotate_snippets::{AnnotationKind, Group, Level, Renderer, Snippet};
7070
use anyhow::{Context as _, Error};
7171
use cargo_platform::{Cfg, Platform};
72+
use itertools::Itertools;
7273
use lazycell::LazyCell;
7374
use regex::Regex;
7475
use tracing::{debug, instrument, trace};
@@ -1356,12 +1357,8 @@ fn build_base_args(
13561357
.map(|s| s.as_ref()),
13571358
);
13581359
if incremental {
1359-
let dir = build_runner
1360-
.files()
1361-
.layout(unit.kind)
1362-
.incremental()
1363-
.as_os_str();
1364-
opt(cmd, "-C", "incremental=", Some(dir));
1360+
let dir = build_runner.files().incremental_dir(&unit);
1361+
opt(cmd, "-C", "incremental=", Some(dir.as_os_str()));
13651362
}
13661363

13671364
let pkg_hint_mostly_unused = match hints.mostly_unused {
@@ -1671,18 +1668,35 @@ fn build_deps_args(
16711668
unit: &Unit,
16721669
) -> CargoResult<()> {
16731670
let bcx = build_runner.bcx;
1674-
cmd.arg("-L").arg(&{
1675-
let mut deps = OsString::from("dependency=");
1676-
deps.push(build_runner.files().deps_dir(unit));
1677-
deps
1678-
});
1671+
if build_runner.bcx.gctx.cli_unstable().build_dir_new_layout {
1672+
let mut map = BTreeMap::new();
1673+
1674+
// Recursively add all depenendency args to rustc process
1675+
add_dep_arg(&mut map, build_runner, unit);
1676+
1677+
let paths = map.into_iter().map(|(_, path)| path).sorted_unstable();
1678+
1679+
for path in paths {
1680+
cmd.arg("-L").arg(&{
1681+
let mut deps = OsString::from("dependency=");
1682+
deps.push(path);
1683+
deps
1684+
});
1685+
}
1686+
} else {
1687+
cmd.arg("-L").arg(&{
1688+
let mut deps = OsString::from("dependency=");
1689+
deps.push(build_runner.files().deps_dir(unit));
1690+
deps
1691+
});
1692+
}
16791693

16801694
// Be sure that the host path is also listed. This'll ensure that proc macro
16811695
// dependencies are correctly found (for reexported macros).
16821696
if !unit.kind.is_host() {
16831697
cmd.arg("-L").arg(&{
16841698
let mut deps = OsString::from("dependency=");
1685-
deps.push(build_runner.files().host_deps());
1699+
deps.push(build_runner.files().host_deps(unit));
16861700
deps
16871701
});
16881702
}
@@ -1758,6 +1772,21 @@ fn build_deps_args(
17581772
Ok(())
17591773
}
17601774

1775+
fn add_dep_arg<'a, 'b: 'a>(
1776+
map: &mut BTreeMap<&'a Unit, PathBuf>,
1777+
build_runner: &'b BuildRunner<'b, '_>,
1778+
unit: &'a Unit,
1779+
) {
1780+
if map.contains_key(&unit) {
1781+
return;
1782+
}
1783+
map.insert(&unit, build_runner.files().deps_dir(&unit));
1784+
1785+
for dep in build_runner.unit_deps(unit) {
1786+
add_dep_arg(map, build_runner, &dep.unit);
1787+
}
1788+
}
1789+
17611790
/// Adds extra rustc flags and environment variables collected from the output
17621791
/// of a build-script to the command to execute, include custom environment
17631792
/// variables and `cfg`.

src/cargo/ops/cargo_clean.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ fn clean_specs(
200200

201201
// Clean fingerprints.
202202
for (_, layout) in &layouts_with_host {
203-
let dir = escape_glob_path(layout.fingerprint())?;
203+
let dir = escape_glob_path(layout.legacy_fingerprint())?;
204204
clean_ctx
205205
.rm_rf_package_glob_containing_hash(&pkg.name(), &Path::new(&dir).join(&pkg_dir))?;
206206
}
@@ -226,8 +226,13 @@ fn clean_specs(
226226
CompileMode::Check { test: false },
227227
] {
228228
for (compile_kind, layout) in &layouts {
229-
let triple = target_data.short_name(compile_kind);
229+
if clean_ctx.gctx.cli_unstable().build_dir_new_layout {
230+
let dir = layout.build_unit(&pkg.name());
231+
clean_ctx.rm_rf_glob(&dir)?;
232+
continue;
233+
}
230234

235+
let triple = target_data.short_name(compile_kind);
231236
let (file_types, _unsupported) = target_data
232237
.info(*compile_kind)
233238
.rustc_outputs(mode, target.kind(), triple, clean_ctx.gctx)?;
@@ -236,8 +241,8 @@ fn clean_specs(
236241
(layout.build_examples(), Some(layout.examples()))
237242
}
238243
// Tests/benchmarks are never uplifted.
239-
TargetKind::Test | TargetKind::Bench => (layout.deps(), None),
240-
_ => (layout.deps(), Some(layout.dest())),
244+
TargetKind::Test | TargetKind::Bench => (layout.legacy_deps(), None),
245+
_ => (layout.legacy_deps(), Some(layout.dest())),
241246
};
242247
let mut dir_glob_str = escape_glob_path(dir)?;
243248
let dir_glob = Path::new(&dir_glob_str);

0 commit comments

Comments
 (0)