Enable build-dir layout v2 on nightly by default#17258
Conversation
3f297a4 to
d34b4b3
Compare
|
r? @epage rustbot has assigned @epage. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
Marking this as ready to review. I spent some time trying to reproduce sccache failures with argfiles on windows, and couldn't get a failure. (tested on zed again, with |
| /// gating unstable functionality to Cargo. These flags are only available on | ||
| /// the nightly channel of Cargo. | ||
| #[derive(Default, Debug, Deserialize)] | ||
| #[derive(Debug, Deserialize)] |
There was a problem hiding this comment.
Is there a way to do this without removing Default? This makes it more invasive to undo and redo.
There was a problem hiding this comment.
Ehh its get awkward to avoid, specially due to #[serde(default)].
I split out a refactor commit and manually impl'd Default in the macro to reduce the bloat and need to update the Default impl when adding new features.
It's still not ideal but maybe a bit better?
d34b4b3 to
4bb796b
Compare
This comment has been minimized.
This comment has been minimized.
4bb796b to
7b66084
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| // Defaults to enabled on nightly unless explicitly opted out. | ||
| let c = &channel(); | ||
| unstable.build_dir_new_layout = | ||
| (c == "nightly" || c == "dev") && !is_new_build_dir_layout_opt_out(); |
There was a problem hiding this comment.
Josh (or someone I don't remember) has mentioned there are people using stable toolchains but enable RUSTC_BOOTSTRAP=1 for enabling some unstable features. Should we avoid enable it if the toolchain is stable/beta?
There was a problem hiding this comment.
It was @Mark-Simulacrum that mentioned it here.
Yeah, I think that makes sense. When that comment was made it kind of went over my head (we have so many bespoke config options 😆)
If I understand it correctly, enabling RUSTC_BOOTSTRAP=1 will result in Cargo using the dev channel.
cargo/src/workspace/features.rs
Lines 1602 to 1605 in 5b6fca7
So the logic should be updated to
let is_rustc_bootstrap = env::var("RUSTC_BOOTSTRAP").map(|o| &o == "1");
unstable.build_dir_new_layout = (c == "nightly" || c == "dev") && !is_new_build_dir_layout_opt_out() && !is_rustc_bootstrap;Will this cause an issues in bootstrap? I guess not since bootstrap explicitly passes -Zbuild-dir-new-layout and this is just for the defaults.
Does that all sound correct?
There was a problem hiding this comment.
Either that or just something like crate::version().release_channel == Some("nightly" | "dev")?
There was a problem hiding this comment.
ah yeah that should work. I wish I had thought of this before updating all of the tests again. 🫠
Since this skips __CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS none of the tests need to be updated
7b66084 to
d035da6
Compare
Cargo enables build-dir layout v2 by default on nightly (rust-lang/cargo#17258). Intermediate build artifacts no longer live in `<target-dir>/<triple>/<profile>/deps/` but in per-unit `build/<pkg>/<hash>/out/` directories. Search the target directory recursively instead of hardcoding the artifact location, so these tests pass under both the old and the new layout.
Cargo enables build-dir layout v2 by default on nightly (rust-lang/cargo#17258). Intermediate build artifacts no longer live in `<target-dir>/<triple>/<profile>/deps/` but in per-unit `build/<pkg>/<hash>/out/` directories.
What does this PR try to resolve?
This PR enables the new
build-diron nightly by default.The majority of changes were taken/adapted from #16807
Tracked in #15010
How to test and review this PR?
Same as the original stabilization PR.
The notable changes are:
p.cargo()use a mixture of stable and nightlyp.cargo()as the tests are specifically testing for rebuilds.