-
Notifications
You must be signed in to change notification settings - Fork 2.8k
docs(layout): Updated layout module docs to document new layout #16502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
| //! | ||
| //! As of early 2026, Cargo is restructuring the layout and supports 2 layouts. | ||
| //! * the original layout organizing files by type | ||
| //! * the new "build unit" based layout | ||
| //! | ||
| //! For more info the layout transition see: [#15010](https://github.com/rust-lang/cargo/issues/15010) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we track removal of this on stabilization?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it could be a bullet point to remove in the tracking issue to remove this in the stabilization PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is fine. Be sure to call that out in this PR description.
| //! # Note that named profiles will soon be included as separate directories | ||
| //! # here. They have a restricted format, similar to Rust identifiers, so |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean about named profiles will be separate directories?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this was copied over. Probably should be removed from the other section in a separate commit
src/cargo/core/compiler/layout.rs
Outdated
| //! # This is the directory for the rustc artifacts of this unit except build | ||
| //! # scripts, examples, and test and bench executables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These exceptions are removed, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For build-scripts its still applicable, as they are compiled into $pkgname/$META/build-script.
Though, that could probably be merged into $pkgname/$META/deps. (also thinking about it that would allow us to rename build-script-execution to build-script for shorter paths on windows)
For tests and benches I believe you are correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh looks like you beat me to that idea in this comment https://github.com/rust-lang/cargo/pull/16502/files#r2686490751 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I mentioned at #16515 (review), I'm tempted to generalize the layout. My ideas are:
- change
build-script-executedtorunas it just contains information general to running an external process - change
depstooutand movebuild-script-executedtooutas well, sooutrepresents the writeable directory for a build unit to write output to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that idea!
My only thought is maybe we use output instead of out to avoid having 2 types of our-dir's which could be a bit confusing.
something like
build-dir/debug/build/<pkgname>/<hash>
fingerprint/
output/
run/
| //! # If the unit is a build script compilation, the build script | ||
| //! # binary will be stored here. | ||
| //! build-script/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need these to be different than deps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, we can probably merge them together as mentioned in https://github.com/rust-lang/cargo/pull/16502/files#r2694115028
ca93c71 to
080e303
Compare
| //! | ||
| //! # Each artifact dependency gets in its own directory. | ||
| //! artifact/$kind | ||
| //! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While writing this I realized that the artifact deps functionality is still pointing to <build-dir>/<profile>/deps/artifact/$pkgname-$META for the new layout 🫠
I updated this to be the correct location (<build-dir>/<profile>/build/$pkgname/$META/deps/artifact/<kind> ) in the docs and will fix this in a separate PR
### What does this PR try to resolve? While documenting the `build-dir` layout changes in #16502 I noticed that we are still creating the `<build-dir>/<profile>/examples` directory when `-Zbuild-dir-new-layout` is passed even though its no longer used. This PR fixes it cc tracking issue: #15010 ### How to test and review this PR? Updating the existing examples test to catch this particular case r? @epage
### What does this PR try to resolve? In #16502 (comment) there was a discussion about merging the `build-script` and `deps` dirs in the `build-dir`. Currently: ``` build-dir/<profile>/build/<pkgname>/<hash>/ deps/ # rustc output build-script/ # build script binary build-script-execution/ # the output of running a build script ``` Note: For build-scripts `deps` is empty. This PR: 1. moves the build script binaries from `build-script` to `deps` to simplify the layout 2. renames `build-script-execution` to `build-script` to simplify the dir naming and shorten the path length for windows ``` build-dir/<profile>/build/<pkgname>/<hash>/ deps/ # rustc output including build-script bins build-script/ # the output of running a build script ``` cc tracking issue: #15010 ### How to test and review this PR? see the test changes r? @epage
### What does this PR try to resolve? This spawned out of #16502 (comment) when I noticed artifact dependencies are not using the new build-dir layout. This PR moves them from `<build-dir>/<profile>/deps/artifact/$pkgname-$META` (old layout) to `<build-dir>/<profile>/build/$pkgname/$META/deps/artifact/<kind>` when `-Zbuild-dir-new-layout` is enabled. cc tracking issue: #15010 ### How to test and review this PR? Added new test specifically for artifact deps r? @epage
What does this PR try to resolve?
This PR updates the layout module docs to reflect the update layout structure changes made as part of #15010
How to test and review this PR?
no response
Follow up needed