Skip to content

Commit ef12f10

Browse files
authored
Implemented build.build-dir config option (#15104)
## What does this PR try to resolve? This PR adds a new `build.build-dir` configuration option that was proposed in #14125 (comment) This new config option allows the user to specify a directory where intermediate build artifacts should be stored. I have shortened it to just `build-dir` from `target-build-dir`, although naming is still subject to change. ### What is a final artifact vs an intermediate build artifact #### Final artifacts These are the files that end users will typically want to access directly or indirectly via a third party tool. * binary executable for bin crates * binary executable for examples * `.crate` files output from `cargo package` * [depinfo files](https://doc.rust-lang.org/cargo/reference/build-cache.html#dep-info-files) (`.d` files) for third party build-system integrations (see https://github.com/rust-lang/cargo/blob/master/src/cargo/core/compiler/fingerprint/mod.rs#L194) * `cargo doc` output (html/css/js/etc) * Cargo's [`--timings`](https://doc.rust-lang.org/cargo/reference/timings.html) HTML report #### Intermediate build artifact, caches, and state These are files that are used internally by Cargo/Rustc during the build process * other depinfo files (generated by rustc, fingerprint, etc. See https://github.com/rust-lang/cargo/blob/master/src/cargo/core/compiler/fingerprint/mod.rs#L164) * rlibs and debug info from dependencies * build script `OUT_DIR` * output from proc macros (previously stored in `target/build`) * [incremental build](https://doc.rust-lang.org/rustc/codegen-options/index.html#incremental) output from rustc * fingerprint files used by Cargo for rebuild detection * scratchpad used for `cargo package` verify step * Cache of rustc invocations (`.rustc_info.json`) * "pre and non uplifted" binary executables. (ie. bins for `examples` that contain the hash in the name, bins for `benches`, proc macros, build scripts) * `CARGO_TARGET_TMPDIR` files (see rational for this [here](#14125 (comment))) * [future-incompat-report](https://doc.rust-lang.org/cargo/reference/future-incompat-report.html)'s `.future-incompat-report.json` file ## Feature Gating Strategy We are following the "Ignore the feature that is used without a gate" approach as described [here](https://doc.rust-lang.org/nightly/nightly-rustc/cargo/core/features/index.html). The rational for this is: The `build.build-dir` is likely going to be set by by users "globally" (ie. `$CARGO_HOME/config.toml`) to set a shared build directory to reduce rebuilding dependencies. For users that multiple cargo versions having having an error would be disrupted. The fallback behavior is to revert to the behavior of the current stable release (building in `$CARGO_TARGET_DIR`) ## Testing Strategy * We have the existing Cargo testsuite to be sure we do not introduce regressions. * I have also run the testsuite locally with the cli flag remove to verify all tests pass with the default build dir (which falls back to the target dir) * For testing thus far, I have been using small hello world project with a few dependencies like `rand` to verify files are being output to the correct directory. * When this PR is closer to merging, I plan to test with some larger projects with more dependencies, build scripts, ect. * Other testing recommendations are welcome 🙇 ## How should we test and review this PR? This is probably best reviewed commit by commit. I documented each commit. I tied to follow the atomic commits recommendation in the Cargo contributors guide, but I split out some commits for ease of review. (Otherwise I think this would have ended up being 1 or 2 large commits 😅) ## Questions - [x] What is the expected behavior of `cargo clean`? * At the time of writing it only cleans `target` and does not impact the build-dir but this is easily changable. * Answer: See #15104 (comment) - [x] When using `cargo package` are was expecting just the `.crate` file to be in `target` while all other output be stored in `build.build-dir`? Not sure if we consider things like `Cargo.toml`, ` Cargo.toml.orig`, `.cargo_vcs_info.json` part of the user facing interface. * Current consensus is that only `.crate` is considered a final artifact - [x] Where should `cargo doc` output go? HTML/JS for many crates can be pretty large. Moving to the build-dir would help reduce duplication if we find the that acceptable. For `cargo doc --open` this is not a problem but may be problematic for other use cases? * Answer: #15104 (comment) - [x] Are bins generated from `benches` considered final artifacts? * Since bins from `examples` are considered final artifacts, it seems natural that `benches` should also be considered final artifacts. However, unlike `examples` the `benches` bins are stored in `target/{profile}/deps` instead of a dedicated directory (like `target/{profile}/examples`). We could move them into a dedicated directory (`target/{profile}/benches`) but that mean would also be changing the structure of the `target` directory which feels out of scope for this change. If we decide that `benches` are final artifacts, it would probably be better to consider that changes as part of #6790 * Answer: #15104 (comment) - [ ] [Do we want to include a `CARGO_BUILD_DIR` shortcut env var?](#15104 (comment)) * The current commit (2af0c91) has included the `CARGO_BUILD_DIR` shortcut. This can be removed before merging if there a good reason to. ## TODO - Implementation - [x] Add support in `cargo clean` - [ ] ~~Implement templating for `build.build-dir`~~ * Will implement in a follow up PR [as suggested](#15104 (comment)) - [x] Fix issue with `target/examples` still containing "pre-uplifted" binaries - [x] Verify `build-dir` with non-bin crate types - Prepare for review - [x] Clean up/improve docs - [x] Review tests and add more as needed - [x] Fix tests in CI (Windows is currently failing) - [x] Clean up commits - [ ] Resolve remaining questions - [x] Request review
2 parents a1635e8 + 5874b6f commit ef12f10

File tree

19 files changed

+818
-97
lines changed

19 files changed

+818
-97
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
217217
} else if unit.target.is_custom_build() {
218218
self.build_script_dir(unit)
219219
} else if unit.target.is_example() {
220-
self.layout(unit.kind).examples().to_path_buf()
220+
self.layout(unit.kind).build_examples().to_path_buf()
221221
} else if unit.artifact.is_true() {
222222
self.artifact_dir(unit)
223223
} else {

src/cargo/core/compiler/custom_build.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1124,7 +1124,7 @@ fn prepare_metabuild(
11241124
let path = unit
11251125
.pkg
11261126
.manifest()
1127-
.metabuild_path(build_runner.bcx.ws.target_dir());
1127+
.metabuild_path(build_runner.bcx.ws.build_dir());
11281128
paths::create_dir_all(path.parent().unwrap())?;
11291129
paths::write_if_changed(path, &output)?;
11301130
Ok(())

src/cargo/core/compiler/fingerprint/dep_info.rs

+15-15
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ pub struct RustcDepInfo {
5151
pub enum DepInfoPathType {
5252
/// src/, e.g. src/lib.rs
5353
PackageRootRelative,
54-
/// target/debug/deps/lib...
54+
/// {build-dir}/debug/deps/lib...
5555
/// or an absolute path /.../sysroot/...
56-
TargetRootRelative,
56+
BuildRootRelative,
5757
}
5858

5959
/// Same as [`RustcDepInfo`] except avoids absolute paths as much as possible to
@@ -126,7 +126,7 @@ impl EncodedDepInfo {
126126
for _ in 0..nfiles {
127127
let ty = match read_u8(bytes)? {
128128
0 => DepInfoPathType::PackageRootRelative,
129-
1 => DepInfoPathType::TargetRootRelative,
129+
1 => DepInfoPathType::BuildRootRelative,
130130
_ => return None,
131131
};
132132
let path_bytes = read_bytes(bytes)?;
@@ -210,7 +210,7 @@ impl EncodedDepInfo {
210210
for (ty, file, checksum_info) in self.files.iter() {
211211
match ty {
212212
DepInfoPathType::PackageRootRelative => dst.push(0),
213-
DepInfoPathType::TargetRootRelative => dst.push(1),
213+
DepInfoPathType::BuildRootRelative => dst.push(1),
214214
}
215215
write_bytes(dst, paths::path2bytes(file)?);
216216
write_bool(dst, checksum_info.is_some());
@@ -292,14 +292,14 @@ pub fn translate_dep_info(
292292
cargo_dep_info: &Path,
293293
rustc_cwd: &Path,
294294
pkg_root: &Path,
295-
target_root: &Path,
295+
build_root: &Path,
296296
rustc_cmd: &ProcessBuilder,
297297
allow_package: bool,
298298
env_config: &Arc<HashMap<String, OsString>>,
299299
) -> CargoResult<()> {
300300
let depinfo = parse_rustc_dep_info(rustc_dep_info)?;
301301

302-
let target_root = crate::util::try_canonicalize(target_root)?;
302+
let build_root = crate::util::try_canonicalize(build_root)?;
303303
let pkg_root = crate::util::try_canonicalize(pkg_root)?;
304304
let mut on_disk_info = EncodedDepInfo::default();
305305
on_disk_info.env = depinfo.env;
@@ -351,8 +351,8 @@ pub fn translate_dep_info(
351351
let canon_file =
352352
crate::util::try_canonicalize(&abs_file).unwrap_or_else(|_| abs_file.clone());
353353

354-
let (ty, path) = if let Ok(stripped) = canon_file.strip_prefix(&target_root) {
355-
(DepInfoPathType::TargetRootRelative, stripped)
354+
let (ty, path) = if let Ok(stripped) = canon_file.strip_prefix(&build_root) {
355+
(DepInfoPathType::BuildRootRelative, stripped)
356356
} else if let Ok(stripped) = canon_file.strip_prefix(&pkg_root) {
357357
if !allow_package {
358358
return None;
@@ -362,7 +362,7 @@ pub fn translate_dep_info(
362362
// It's definitely not target root relative, but this is an absolute path (since it was
363363
// joined to rustc_cwd) and as such re-joining it later to the target root will have no
364364
// effect.
365-
(DepInfoPathType::TargetRootRelative, &*abs_file)
365+
(DepInfoPathType::BuildRootRelative, &*abs_file)
366366
};
367367
Some((ty, path.to_owned()))
368368
};
@@ -472,7 +472,7 @@ pub fn parse_rustc_dep_info(rustc_dep_info: &Path) -> CargoResult<RustcDepInfo>
472472
/// indicates that the crate should likely be rebuilt.
473473
pub fn parse_dep_info(
474474
pkg_root: &Path,
475-
target_root: &Path,
475+
build_root: &Path,
476476
dep_info: &Path,
477477
) -> CargoResult<Option<RustcDepInfo>> {
478478
let Ok(data) = paths::read_bytes(dep_info) else {
@@ -487,7 +487,7 @@ pub fn parse_dep_info(
487487
ret.files
488488
.extend(info.files.into_iter().map(|(ty, path, checksum_info)| {
489489
(
490-
make_absolute_path(ty, pkg_root, target_root, path),
490+
make_absolute_path(ty, pkg_root, build_root, path),
491491
checksum_info.and_then(|(file_len, checksum)| {
492492
Checksum::from_str(&checksum).ok().map(|c| (file_len, c))
493493
}),
@@ -499,13 +499,13 @@ pub fn parse_dep_info(
499499
fn make_absolute_path(
500500
ty: DepInfoPathType,
501501
pkg_root: &Path,
502-
target_root: &Path,
502+
build_root: &Path,
503503
path: PathBuf,
504504
) -> PathBuf {
505505
match ty {
506506
DepInfoPathType::PackageRootRelative => pkg_root.join(path),
507507
// N.B. path might be absolute here in which case the join will have no effect
508-
DepInfoPathType::TargetRootRelative => target_root.join(path),
508+
DepInfoPathType::BuildRootRelative => build_root.join(path),
509509
}
510510
}
511511

@@ -678,7 +678,7 @@ mod encoded_dep_info {
678678
fn gen_test(checksum: bool) {
679679
let checksum = checksum.then_some((768, "c01efc669f09508b55eced32d3c88702578a7c3e".into()));
680680
let lib_rs = (
681-
DepInfoPathType::TargetRootRelative,
681+
DepInfoPathType::BuildRootRelative,
682682
PathBuf::from("src/lib.rs"),
683683
checksum.clone(),
684684
);
@@ -691,7 +691,7 @@ mod encoded_dep_info {
691691
assert_eq!(EncodedDepInfo::parse(&data).unwrap(), depinfo);
692692

693693
let mod_rs = (
694-
DepInfoPathType::TargetRootRelative,
694+
DepInfoPathType::BuildRootRelative,
695695
PathBuf::from("src/mod.rs"),
696696
checksum.clone(),
697697
);

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

+17-17
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,7 @@ impl LocalFingerprint {
839839
mtime_cache: &mut HashMap<PathBuf, FileTime>,
840840
checksum_cache: &mut HashMap<PathBuf, Checksum>,
841841
pkg: &Package,
842-
target_root: &Path,
842+
build_root: &Path,
843843
cargo_exe: &Path,
844844
gctx: &GlobalContext,
845845
) -> CargoResult<Option<StaleItem>> {
@@ -852,8 +852,8 @@ impl LocalFingerprint {
852852
// the `dep_info` file itself whose mtime represents the start of
853853
// rustc.
854854
LocalFingerprint::CheckDepInfo { dep_info, checksum } => {
855-
let dep_info = target_root.join(dep_info);
856-
let Some(info) = parse_dep_info(pkg_root, target_root, &dep_info)? else {
855+
let dep_info = build_root.join(dep_info);
856+
let Some(info) = parse_dep_info(pkg_root, build_root, &dep_info)? else {
857857
return Ok(Some(StaleItem::MissingFile(dep_info)));
858858
};
859859
for (key, previous) in info.env.iter() {
@@ -910,7 +910,7 @@ impl LocalFingerprint {
910910
LocalFingerprint::RerunIfChanged { output, paths } => Ok(find_stale_file(
911911
mtime_cache,
912912
checksum_cache,
913-
&target_root.join(output),
913+
&build_root.join(output),
914914
paths.iter().map(|p| (pkg_root.join(p), None)),
915915
false,
916916
)),
@@ -1153,7 +1153,7 @@ impl Fingerprint {
11531153
mtime_cache: &mut HashMap<PathBuf, FileTime>,
11541154
checksum_cache: &mut HashMap<PathBuf, Checksum>,
11551155
pkg: &Package,
1156-
target_root: &Path,
1156+
build_root: &Path,
11571157
cargo_exe: &Path,
11581158
gctx: &GlobalContext,
11591159
) -> CargoResult<()> {
@@ -1261,7 +1261,7 @@ impl Fingerprint {
12611261
mtime_cache,
12621262
checksum_cache,
12631263
pkg,
1264-
target_root,
1264+
build_root,
12651265
cargo_exe,
12661266
gctx,
12671267
)? {
@@ -1449,13 +1449,13 @@ fn calculate(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult
14491449

14501450
// After we built the initial `Fingerprint` be sure to update the
14511451
// `fs_status` field of it.
1452-
let target_root = target_root(build_runner);
1452+
let build_root = build_root(build_runner);
14531453
let cargo_exe = build_runner.bcx.gctx.cargo_exe()?;
14541454
fingerprint.check_filesystem(
14551455
&mut build_runner.mtime_cache,
14561456
&mut build_runner.checksum_cache,
14571457
&unit.pkg,
1458-
&target_root,
1458+
&build_root,
14591459
cargo_exe,
14601460
build_runner.bcx.gctx,
14611461
)?;
@@ -1493,7 +1493,7 @@ fn calculate_normal(
14931493
};
14941494

14951495
// Afterwards calculate our own fingerprint information.
1496-
let target_root = target_root(build_runner);
1496+
let build_root = build_root(build_runner);
14971497
let local = if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
14981498
// rustdoc does not have dep-info files.
14991499
let fingerprint = pkg_fingerprint(build_runner.bcx, &unit.pkg).with_context(|| {
@@ -1505,7 +1505,7 @@ fn calculate_normal(
15051505
vec![LocalFingerprint::Precalculated(fingerprint)]
15061506
} else {
15071507
let dep_info = dep_info_loc(build_runner, unit);
1508-
let dep_info = dep_info.strip_prefix(&target_root).unwrap().to_path_buf();
1508+
let dep_info = dep_info.strip_prefix(&build_root).unwrap().to_path_buf();
15091509
vec![LocalFingerprint::CheckDepInfo {
15101510
dep_info,
15111511
checksum: build_runner.bcx.gctx.cli_unstable().checksum_freshness,
@@ -1714,7 +1714,7 @@ fn build_script_local_fingerprints(
17141714
// longstanding bug, in Cargo. Recent refactorings just made it painfully
17151715
// obvious.
17161716
let pkg_root = unit.pkg.root().to_path_buf();
1717-
let target_dir = target_root(build_runner);
1717+
let build_dir = build_root(build_runner);
17181718
let env_config = Arc::clone(build_runner.bcx.gctx.env_config()?);
17191719
let calculate =
17201720
move |deps: &BuildDeps, pkg_fingerprint: Option<&dyn Fn() -> CargoResult<String>>| {
@@ -1747,7 +1747,7 @@ fn build_script_local_fingerprints(
17471747
// them all here.
17481748
Ok(Some(local_fingerprints_deps(
17491749
deps,
1750-
&target_dir,
1750+
&build_dir,
17511751
&pkg_root,
17521752
&env_config,
17531753
)))
@@ -1783,7 +1783,7 @@ fn build_script_override_fingerprint(
17831783
/// [`RunCustomBuild`]: crate::core::compiler::CompileMode::RunCustomBuild
17841784
fn local_fingerprints_deps(
17851785
deps: &BuildDeps,
1786-
target_root: &Path,
1786+
build_root: &Path,
17871787
pkg_root: &Path,
17881788
env_config: &Arc<HashMap<String, OsString>>,
17891789
) -> Vec<LocalFingerprint> {
@@ -1796,7 +1796,7 @@ fn local_fingerprints_deps(
17961796
// absolute prefixes from them.
17971797
let output = deps
17981798
.build_script_output
1799-
.strip_prefix(target_root)
1799+
.strip_prefix(build_root)
18001800
.unwrap()
18011801
.to_path_buf();
18021802
let paths = deps
@@ -1854,10 +1854,10 @@ pub fn dep_info_loc(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> Path
18541854
build_runner.files().fingerprint_file_path(unit, "dep-")
18551855
}
18561856

1857-
/// Returns an absolute path that target directory.
1857+
/// Returns an absolute path that build directory.
18581858
/// All paths are rewritten to be relative to this.
1859-
fn target_root(build_runner: &BuildRunner<'_, '_>) -> PathBuf {
1860-
build_runner.bcx.ws.target_dir().into_path_unlocked()
1859+
fn build_root(build_runner: &BuildRunner<'_, '_>) -> PathBuf {
1860+
build_runner.bcx.ws.build_dir().into_path_unlocked()
18611861
}
18621862

18631863
/// Reads the value from the old fingerprint hash file and compare.

src/cargo/core/compiler/future_incompat.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ pub struct Diagnostic {
9191
pub level: String,
9292
}
9393

94-
/// The filename in the top-level `target` directory where we store
94+
/// The filename in the top-level `build-dir` directory where we store
9595
/// the report
9696
const FUTURE_INCOMPAT_FILE: &str = ".future-incompat-report.json";
9797
/// Max number of reports to save on disk.
@@ -166,7 +166,7 @@ impl OnDiskReports {
166166
}
167167
let on_disk = serde_json::to_vec(&self).unwrap();
168168
if let Err(e) = ws
169-
.target_dir()
169+
.build_dir()
170170
.open_rw_exclusive_create(
171171
FUTURE_INCOMPAT_FILE,
172172
ws.gctx(),
@@ -191,7 +191,7 @@ impl OnDiskReports {
191191

192192
/// Loads the on-disk reports.
193193
pub fn load(ws: &Workspace<'_>) -> CargoResult<OnDiskReports> {
194-
let report_file = match ws.target_dir().open_ro_shared(
194+
let report_file = match ws.build_dir().open_ro_shared(
195195
FUTURE_INCOMPAT_FILE,
196196
ws.gctx(),
197197
"Future incompatible report",

0 commit comments

Comments
 (0)