Skip to content

Enable Cargo's new build-dir layout#155439

Open
ranger-ross wants to merge 7 commits into
rust-lang:mainfrom
ranger-ross:cargo-new-build-dir-layout
Open

Enable Cargo's new build-dir layout#155439
ranger-ross wants to merge 7 commits into
rust-lang:mainfrom
ranger-ross:cargo-new-build-dir-layout

Conversation

@ranger-ross
Copy link
Copy Markdown
Member

@ranger-ross ranger-ross commented Apr 17, 2026

View all comments

This PR enables the new Cargo build-dir layout in boostrap builds with -Zbuild-dir-new-layout.

See: #t-infra/bootstrap > Has anyone tested `./x` with the new build-dir layout?

Tracked in: rust-lang/cargo#15010

r? @bjorn3

cc: @epage

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 17, 2026
Comment thread src/bootstrap/src/core/build_steps/compile.rs Outdated
// got a list of prefix/extensions and we basically just need to find the
// most recent file in the `deps` folder corresponding to each one.
let contents = target_deps_dir
// most recent file in the `build` folder corresponding to each one.
Copy link
Copy Markdown
Member

@weihanglo weihanglo Apr 17, 2026

Choose a reason for hiding this comment

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

Not yet read the code by myself. Just wonder why bootstrap are doing this? And if bootstrap needs to do it, it should be a feature in Cargo, at least a (perma-)unstable feature.

View changes since the review

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.

See the commit block around line 2374. Basically we need the non-uplifted name that contains the unique hash. The artifact notifications provide the uplifted name where possible.

@rust-log-analyzer

This comment has been minimized.

@ranger-ross ranger-ross force-pushed the cargo-new-build-dir-layout branch from bf07fb8 to eae2cbf Compare April 17, 2026 15:27
//
// Cargo's build folder is structured as `build/<pkg>/<hash>/out/<artifacts>` so
// we need to traverse multiple directory layers to get to actual files.
let read_dir = |path: &Path| path.read_dir().ok().into_iter().flatten().filter_map(Result::ok);
Copy link
Copy Markdown
Member Author

@ranger-ross ranger-ross Apr 17, 2026

Choose a reason for hiding this comment

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

checks failed because if there are any lingering files/dirs in <build-dir>/<target>/build that are not using the new layout it panics.

Update the original code to not panic if we call .read_dir() on a non-directory. Lets see if CI is happy with this

View changes since the review

@ranger-ross
Copy link
Copy Markdown
Member Author

Looks like the miri job timed out.
Out of curiosity, how stable is this job?

(Sorry, haven't touch the rl/r repo much so I wondering if the failure is related to the new layout or if its normal for spurious failures)

@mati865
Copy link
Copy Markdown
Member

mati865 commented Apr 19, 2026

x86_64 Linux jobs rarely hang, I've retried it to be sure.

@ranger-ross
Copy link
Copy Markdown
Member Author

Okay it's hanged on both runs so it's probably the changes.

Is there a way for me to run this check on my machine? I was trying to parse through the GitHub actions to see what command is being run but I was struggling to find it.

@weihanglo
Copy link
Copy Markdown
Member

@ranger-ross see https://rustc-dev-guide.rust-lang.org/tests/docker.html#testing-with-docker

cargo run --manifest-path src/ci/citool/Cargo.toml run-local x86_64-gnu-miri

This is the easiest way to run it locally.

@ranger-ross
Copy link
Copy Markdown
Member Author

Okay, so I did some testing and locally it runs fine. (takes around ~20 mins on my machine)

After looking a bit closer at the job logs, it actually never got stuck.
It just take WAY longer than normal....
According to the github stats x86_64-gnu-miri normally takes ~58 mins to run, but we took over 6 hrs and hit the timeout.

Looking at the other jobs, most of them did not have a large perf regression (except x86_64-gnu-tools which was also pretty bad)

That is concerning... 😅

@ranger-ross

This comment was marked as outdated.

@ranger-ross ranger-ross force-pushed the cargo-new-build-dir-layout branch from eae2cbf to ad47c85 Compare April 27, 2026 15:15
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 27, 2026

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue. labels Apr 27, 2026
@rustbot

This comment has been minimized.

@ranger-ross ranger-ross changed the title Enable Cargo's new build-dir layout in boostrap Enable Cargo's new build-dir layout Apr 27, 2026
Comment thread src/tools/miri/test-cargo-miri/run-test.py Outdated
Comment thread src/tools/compiletest/src/runtest/run_make.rs Outdated
@ranger-ross
Copy link
Copy Markdown
Member Author

I realized the issue is not with the speed of compilation, but rather that for some reason in CI the compiler (and many other units) are being re-built multiple times which slows everything down and times out.

I haven't figured out why yet. While testing the CI on my fork, I've noticed its inconsistent and sometimes everything only builds once. I'm still investigating why this could be happening. rust-lang/cargo#16345 comes to mind as something that might change this behavior, but I am still unsure whats happening.

I went ahead and pushed some new changes with more fixes for tests and cranelift.

I'll continue investigating this week, but any rl/r experts see any obvious issues with my changes lmk
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 27, 2026
Comment thread compiler/rustc_codegen_cranelift/build_system/build_sysroot.rs
Comment thread src/tools/rustfmt/src/test/mod.rs Outdated
Comment on lines +1080 to +1086
// Chop off `deps`.
// Chop off `out`.
me.pop();
// Chop off `<hash>`.
me.pop();
// Chop off `<pkgname>`.
me.pop();
// Chop off `build`.
Copy link
Copy Markdown
Contributor

@ytmimi ytmimi Apr 27, 2026

Choose a reason for hiding this comment

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

Is this going to break things in rustfmt's CI when we do a subtree push?

View changes since the review

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.

ahh yeah it would probably break, maybe better for me to remove that from this PR and raise a separate PR in the rustfmt repo?

I'm still trying to understand what changes we need in this PR verse what changes can be split out into smaller more targeted changes

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.

In that case, I'd prefer we open up a PR to add the new cargo build dir layout support to rustfmt directly and verify that things are working before making any changes to the rustfmt subtree here in r-l/rust

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.

fair enough. I tested on my fork and I think rust-lang/rust CI fails with out rustfmt changes.

I went ahead and opened rust-lang/rustfmt#6879

Copy link
Copy Markdown
Contributor

@ytmimi ytmimi Apr 29, 2026

Choose a reason for hiding this comment

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

@ranger-ross If you're able to duplicate the rustfmt code changes you made in rust-lang/rustfmt#6879 here that would be great.

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 added those commits to this PR. I also noticed that RUSTFLAGS: -D warning was missing from the github actions workflows in rust-lang/rust so I went ahead and added that while I was at it. I guess it doesn't matter for this repo but oh well.

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.

We've recently made some changes to those CI files in rustfmt so it's possible that the RUSTFLAGS: -D warning changes just haven't been synced up yet, but I don't think there's any issue in explicitly including that here.

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request May 31, 2026
Enable Cargo's new build-dir layout


try-job: x86_64-msvc-1
@jieyouxu jieyouxu self-assigned this May 31, 2026
@Kobzol
Copy link
Copy Markdown
Member

Kobzol commented May 31, 2026

(Can't do that right now because of the pending try build, but please also do a "normal" try build and run perf. on this PR, to ensure that it doesn't break rustc-perf).

@ranger-ross
Copy link
Copy Markdown
Member Author

Sure.
From reading the rustc contributor guide I take it that the command should be @bors try @rust-timer queue, correct?

@rust-timer

This comment was marked as off-topic.

@jieyouxu
Copy link
Copy Markdown
Member

Hm, I forgot if rust-timer was gated behind a separate perm.
@bors delegate=try
@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 31, 2026

✌️ @ranger-ross, you can now perform try builds on this pull request!

You can now post @bors try to start a try build.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 31, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request May 31, 2026
@jieyouxu
Copy link
Copy Markdown
Member

... AAAA. I forgot it cancels the msvc job, d'oh. Sorry. We can just let the normal try job run w/ perf, then we'll just queue another run of msvc. 🤦

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 31, 2026

☀️ Try build successful (CI)
Build commit: f8202da (f8202da0c3bf8db1ff27204fd160db6594220b76, parent: c315891193c35827c2d789adce686f8a5481778f)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (f8202da): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 3.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
3.3% [2.8%, 4.2%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.3% [2.8%, 4.2%] 3

Cycles

Results (secondary 22.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
22.0% [22.0%, 22.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 509.639s -> 510.066s (0.08%)
Artifact size: 400.69 MiB -> 400.72 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 31, 2026
@ranger-ross
Copy link
Copy Markdown
Member Author

@bors try jobs=x86_64-msvc-1

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request May 31, 2026
Enable Cargo's new build-dir layout


try-job: x86_64-msvc-1
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 31, 2026

💔 Test for 9da18bc failed: CI. Failed job:

This allows compiletest to support the new Cargo `build-dir` layout
which passes more `-L` flags as the `deps` dir has been split per build
unit. This can be an issue on Windows as the max command size is fairly
small.
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 1, 2026

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@ranger-ross
Copy link
Copy Markdown
Member Author

Okay, slowly but surely making progress. The latest test failure was a new error, this time due to compiletest not using arg files when invoking rustc.

Added another commit with the changes to move compiletests to use arg files. (I duplicated the ArgFileCommand code from bootstrap to compiletest, if there is smart way to share code between these, I'd be happy to refactor it)

Hopefully this is the last blocker before the windows job is happy 🙏

@bors try jobs=x86_64-msvc-1

@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request Jun 1, 2026
Enable Cargo's new build-dir layout


try-job: x86_64-msvc-1
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 1, 2026

☀️ Try build successful (CI)
Build commit: 10faafd (10faafd107c8233cd0797381cc78ab8fcf62ef21, parent: 4804ad7e93e1b31f4605b7083871d0d3d85a2afe)

@ranger-ross
Copy link
Copy Markdown
Member Author

Okay window CI is passing, I think this is finally ready for a re-review :)

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 1, 2026
@jieyouxu jieyouxu removed their assignment Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: Our Github Actions CI A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustfmt Relevant to the rustfmt team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.