Skip to content

Stabilize build-dir layout v2 #16807

Merged
weihanglo merged 9 commits into
rust-lang:masterfrom
ranger-ross:stablize-build-dir-new-layout
Jul 6, 2026
Merged

Stabilize build-dir layout v2 #16807
weihanglo merged 9 commits into
rust-lang:masterfrom
ranger-ross:stablize-build-dir-new-layout

Conversation

@ranger-ross

@ranger-ross ranger-ross commented Mar 30, 2026

Copy link
Copy Markdown
Member

View all comments

What does this PR try to resolve?

FCP

This PR stabilizes the build-dir reorg tracked in #15010

We have completed a crater runs (rust-lang/rust#149852) as well a call for testing (https://blog.rust-lang.org/2026/03/13/call-for-testing-build-dir-layout-v2/) with no issues reported.

At a recent Cargo meeting, we discussed the path towards stabilization:

  • Stabilize build-dir layout v2, and merge into nightly shortly after the 1.96 beta branch
  • Allow a full nightly cycle for nightly users to report issues.
  • Pending no major blockers, allow the stabilization to be included in the Rust 1.97 beta (otherwise revert before beta branching)

This stabilization does NOT remove the code for the legacy build-dir layout and enables -Zbuild-dir-new-layout by default. We also provide a temporary __CARGO_TEMPORARY_BUILD_DIR_NEW_LAYOUT_OPT_OUT env var that users can use to opt out. Note that this flag is temporary and only meant to mitigate the transition. It will be removed in the next release along with the legacy layout code.

The rational for this approach is that we may need to rollback the changes before the next stabilization and removing the feature flag would be a lot of changes that could result in a conflict. Keeping the feature flag minimizes the code changes needed to stabilize and makes it less risky if we need to revert.
The opt out flag is to provide a short term way for users opt if. This is primarily intended for users that absolutely must use nightly and rely on tools that expect the old layout.

closes #15010

Flag/opt out priority: (highest to lowest)

  • -Zfine-grain-locking (will always force enable the new layout)
  • __CARGO_TEMPORARY_BUILD_DIR_NEW_LAYOUT_OPT_OUT=1
  • -Zbuild-dir-new-layout (this will exist on nightly but be enabled by default)

Note on the locking design:

We will defer changing .cargo-artifact-lock to .cargo-lock to reduce the scope of this PR.
See #16807 (comment) for the rational for why this is safe to do and forward compatible

Pending tasks

How to test and review this PR?

There are a LOT of changes and I did my best to separate them into isolated commits.
However the stabilization commit has a lot of path updates in many tests.

@rustbot rustbot added the A-unstable Area: nightly unstable support label Mar 30, 2026
@epage

epage commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

This stabilization does NOT remove the code for the legacy build-dir layout

Please update the PR description to include motivations for decisions.

Comment thread tests/testsuite/build_dir_fine_grain_locking.rs
Comment thread tests/testsuite/bench.rs
[COMPILING] foo v0.5.0 ([ROOT]/foo)
[FINISHED] `bench` profile [optimized] target(s) in [ELAPSED]s
[RUNNING] unittests src/main.rs (target/release/deps/foo-[HASH][EXE])
[RUNNING] unittests src/main.rs (target/release/build/foo/[HASH]/out/foo-[HASH][EXE])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we kept the hash in test filenames?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh, do think so. Maybe it's worth revisiting?

That also reminds me that build scripts have an unnesscary hardlink within build-unit/out dir.

out
   build_script_build
   build_script_build-[HASH]
   build_script_build.d

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I raised #16812 to remove the hash from build script bins.
Looking at the PR (#16351) the removed the hash from regular bins there is no mention of tests.
Perhaps this was just missed? I can't think of any reason for them to be different.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you mentioned, maybe we just leave the test bins with the hash to avoid breaking projects that have already updated to use the new build-dir layout.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any documentation we want to update when stabilizing this? I guess at least unstable.md?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also https://doc.rust-lang.org/cargo/reference/build-cache.html has some details about the build-dir internals.

github-merge-queue Bot pushed a commit that referenced this pull request Mar 31, 2026
### What does this PR try to resolve?

This PR spawned out of a
[comment](#16807 (comment))
from @epage raising that we didn't remove the hashes for some bins.
However we did remove the hashes from "regular" bins in
#16351

This PR
* Removes the use of `-Cextra-filename` for build scripts when
`-Zbuild-dir-new-layout` is enabled.
* Does not uplift build-scripts in the `build-dir` internally. (its not
really uplifting, its more of a rename via hardlink)

Previously, we had a directory like
```
build/<pkg>/<hash>/out/
    build_script_build-[HASH].d
    build_script_build-[HASH][EXE]
    build-script-build[EXE]
```

After this change
```
build/<pkg>/<hash>/out/
    build_script_build.d
    build_script_build[EXE]
```

Part of: #15010

### How to test and review this PR?

See the test updates in each commit.
I could split into 2 PRs if needed but I felt the changes were related
and small enough to go together.
@ranger-ross ranger-ross force-pushed the stablize-build-dir-new-layout branch from 7b5186b to 0a0c24e Compare April 1, 2026 15:44
@ranger-ross

Copy link
Copy Markdown
Member Author

addressed the comments thus far and fixed more tests and down to about ~180 failing tests remaining.
Ran out of time today but will continue migrating the rest tomorrow

@ranger-ross ranger-ross force-pushed the stablize-build-dir-new-layout branch 5 times, most recently from 0cff515 to 925630d Compare April 4, 2026 15:34
@rustbot rustbot added the A-testing-cargo-itself Area: cargo's tests label Apr 4, 2026
@ranger-ross

Copy link
Copy Markdown
Member Author

Made some pretty good progress on the tests today.
I am down to about 15 that are still failing.

The remaining tests either:

  • Require more complex test changes than a simple string replace
  • Had unexpected behavior like rebuilding when previously a unit was fresh (or vise versa)

I'll look into these tomorrow and hopefully resolve the remaining tests.

@ranger-ross ranger-ross force-pushed the stablize-build-dir-new-layout branch 3 times, most recently from 0c39f75 to 4e04b01 Compare April 6, 2026 12:54
@ranger-ross

ranger-ross commented Apr 6, 2026

Copy link
Copy Markdown
Member Author

Okay, we are down to 1 test left failing: lto::dylib.

This test is verifying that rebuild behavior works correctly for LTO mode changes for a given dylib build unit.
The rebuild behavior is correct, however the dirty reason has changed from DirtyReason::UnitDependencyInfoChanged to DirtyReason::FreshBuild and I am not exactly sure why.

Still debugging what is causing this difference in the new layout, but if any fingerprint experts are around I'd appreciate if you have any insights

@epage

epage commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

@ranger-ross it might be #16345, see the commit message in d72e446

@ranger-ross

Copy link
Copy Markdown
Member Author

Ahhh okay, I think I understand what is happening.

Given the crate structure

foo v0.0.0
├── bar v0.0.0 <---- dylib crate
│   ├── registry v0.0.1
│   └── registry-shared v0.0.1
└── registry-shared v0.0.1

If registry-shared becomes dirty, previously the pkg-hash of bar would NOT change.
This allowed us to detect the reason for it being dirty being a dependency unit.

However with the change in #16345, we now use unit_id (which include dependency unit unit_id's in the hash) in the pkg-dir.
This means the units are now unique as previously it was reused.
This has the side effect of losing the dirty reason tracking.

I suppose this is the expected behavior going forward.

@ranger-ross ranger-ross force-pushed the stablize-build-dir-new-layout branch 3 times, most recently from f4b1e28 to 304d68a Compare April 6, 2026 17:02
}

fn implicitly_enable_features_if_needed(&mut self) {
if self.fine_grain_locking && !self.build_dir_new_layout {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be removed now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could remove it now, but it would open the door for someone to using __CARGO_TEMPORARY_BUILD_DIR_NEW_LAYOUT_OPT_OUT=1 with -Zfine-grain-locking which I don't think would work. (I am not sure what would happen of the top of my head but I'd guess a panic)

I think we can leave it just to be safe and clean it up with everything else

Comment thread src/cargo/core/features.rs Outdated
impl Default for CliUnstable {
fn default() -> Self {
Self {
build_dir_new_layout: true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this custom default impl and set it to true in the else branch of the check for __CARGO_TEMPORARY_BUILD_DIR_NEW_LAYOUT_OPT_OUT?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, I originally was setting this in CliUnstable::parse() where (__CARGO_TEMPORARY_BUILD_DIR_NEW_LAYOUT_OPT_OUT is checked) but I realized that the unstable options are only parsed on nightly so that function does not get called.
I realized there are a few places CliUnstable::default() so instead of trying to add manual overriding to all of the callsites, I felt like adding this Default impl was the lesser of too evils.

But now that you mention it, I think there is a bug that __CARGO_TEMPORARY_BUILD_DIR_NEW_LAYOUT_OPT_OUT not work on stable since its checked in CliUnstable::parse().
I think this will also need to be moved into CliUnstable::default() as well.
I'll check and add a test for this explicitly.

@ranger-ross ranger-ross force-pushed the stablize-build-dir-new-layout branch from 304d68a to 5244670 Compare April 7, 2026 13:48
@rustbot

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. label Apr 8, 2026
@ranger-ross ranger-ross force-pushed the stablize-build-dir-new-layout branch from 5244670 to d9e8aeb Compare April 9, 2026 12:52
@ranger-ross ranger-ross force-pushed the stablize-build-dir-new-layout branch from 39f05de to bd61f62 Compare June 22, 2026 04:25
@ranger-ross

Copy link
Copy Markdown
Member Author

Alrighty, rust-lang/rust#155439 was finally merged.
Since its already pretty late into this beta branch cycle, I went ahead and bump the stabilization version to 1.99.0.
1.98.0 will beta branch on July 3rd, so we can merge after that and get the full 1.99.0 nightly cycle to surface any issues.

@ranger-ross ranger-ross force-pushed the stablize-build-dir-new-layout branch from bd61f62 to bc7890e Compare July 4, 2026 04:39
@rustbot

rustbot commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

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.

@weihanglo weihanglo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Half way through the last largg diff. This generally looks good. Let me pre-approve and will come back later and finish it off.

View changes since this review

Comment thread src/cargo/core/features.rs
Comment thread tests/testsuite/build.rs
Comment on lines +1692 to 1696
.parent().unwrap()
.parent().unwrap()
.parent().unwrap()
.parent().unwrap()
.parent().unwrap()

@weihanglo weihanglo Jul 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This becomes more annoying 😆

View changes since the review

Comment thread tests/testsuite/build_script_env.rs Outdated
p.cargo("check")
.with_stderr_data(str![[r#"
[COMPILING] has-dashes v0.0.1 ([ROOT]/foo)
...

@weihanglo weihanglo Jul 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhh good question. I don't remember why I changed this.
I reverted these and the tests are passing on my machine.

I'll go ahead and revert this test and see if CI passes

"#]]
} else {
str![[r#"
p.cargo("build --features bar -v")

@weihanglo weihanglo Jul 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to fix a Windows rebuild detection issue:

MSVC does not include hash in binary filename, so it gets recompiled.

I remembered this but surprisingly cannot found existing issue for it. I guess this is also worth calling out in changelog.

View changes since the review

@weihanglo weihanglo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Merging. I'll do a submodule update then.

View changes since this review

@weihanglo weihanglo added this pull request to the merge queue Jul 6, 2026
Merged via the queue into rust-lang:master with commit b26e1b2 Jul 6, 2026
29 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 6, 2026
@rust-rfcbot rust-rfcbot removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Jul 6, 2026
pull Bot pushed a commit to johnperez416/cargo that referenced this pull request Jul 7, 2026
### What does this PR try to resolve?

This PR reverts the final stabilization commit
(668039d) in
rust-lang#16807.

At today's Cargo meeting, we discussed the recent bug reports (notably
rust-lang#17183) from the new `build-dir` landing on nightly.
We felt it would be safer to revert so that are not on a shot clock to
fix the issues.

### How to test and review this PR?

no response

r? @epage
rust-bors Bot pushed a commit to rust-lang/rust that referenced this pull request Jul 8, 2026
Update cargo submodule

4 commits in 2f0e7011e0e9cb30cb772bf2ec1d69dce4dff4f3..59800466c5c41c444d264b1010b4d57e85a7117f
2026-07-05 12:10:34 +0000 to 2026-07-07 15:52:22 +0000
- Revert "feat: Stablize build-dir layout v2" (rust-lang/cargo#17187)
- Use a set when checking visited workspace members (rust-lang/cargo#17180)
- Stabilize `build-dir` layout v2  (rust-lang/cargo#16807)
- Change HashMaps and HashSets in Cargo to use Fxhasher (rust-lang/cargo#17169)
@rustbot rustbot added this to the 1.99.0 milestone Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-documenting-cargo-itself Area: Cargo's documentation A-testing-cargo-itself Area: cargo's tests A-unstable Area: nightly unstable support finished-final-comment-period FCP complete S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. T-cargo Team: Cargo to-announce

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Re-organize build-dir by package + hash, rather than artifact type

7 participants