Refactor `Layout` into `BuildDirLayout` and `ArtifactDirLayout` #16092

ranger-ross · 2025-10-12T06:31:04Z

What does this PR try to resolve?

After the recent build-dir changes, I believe the Layout is now a bit overloaded now and no longer represents a single thing. For example its not immediately clear if fn root() is the root of the build-dir or artifact-dir.
Also duplicate functions are needed in some cases like examples() and build_examples().

This PR splits the layout into multiple self contained structs while keeping a high level Layout container struct.

Note: I chose the ArtifactDirLayout over TargetDirLayout as I believe that is the direction we are going in #6790 and there has been mention of eventually phasing out target-dir in the long term. (Though happy to change it if there is a good reason)

How to test and review this PR?

The existing tests should be sufficient.

Best reviewed commit by commit.

The first commit adds a timings() to the layout to bring it in line with other directories in target
Split Layout internally while keeping the pub interface unchanged.
Change the Layout public interface to expose .build_dir() and .artifact_dir() to make it explicit which directory is being used.

rustbot · 2025-10-12T06:31:08Z

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

weihanglo · 2025-10-12T20:38:03Z

src/cargo/core/compiler/build_context/target_info.rs

            .all_kinds
            .iter()
-            .map(|kind| build_runner.files().layout(*kind).doc())
+            .map(|kind| build_runner.files().layout(*kind).artifact_dir().doc())


The separation of ArtifactDirLayout and BuildDirLayout makes sense, as it the actually layout of two directories a lot clearer.

However, I feel like it should be opaque to anything outside layout.rs. Other module shouldn't concern where those build caches are actually stored. Exposing fn artifact_dir() and build_dir() seems to expose a bit too many implementation details to me. What do you think?

However, I feel like it should be opaque to anything outside layout.rs. Other module shouldn't concern where those build caches are actually stored

I understand where you are coming from but I think exposing artifact_dir vs build_dir is necessary.
We already do that by duplicating functions like examples() and build_examples(). If we keep everything in Layout we would probably eventually end up with root() and build_root(), dest() and build_dest(), etc.
At that point, I think its better to explicitly use the build_dir()/artifact_dir() to avoid confusion as to where the files are going.

Also I found that in most (all?) of the time I knew which directory I wanted move files to and the opaque nature of Layout felt like it was hurting more than it was helping.

For myself, I feel like exposing the split isn't about exposing implementation details but being clearer in communication and thinking about intermediate and final artifacts.

Fair! Thanks for the discussion!

weihanglo · 2025-10-12T20:41:08Z

src/cargo/core/compiler/layout.rs

 //!                 root-output
 //!                 # Stderr output from the build script.
 //!                 stderr
 //!


Semi-related to this PR: maybe we should update the module-level doc as well to reflect artifact-dir/build-dir changes so far

Agreed! Was considering to bundle those changes in this PR but end up deferring to keep this PR focused on the refactor changes.
Will open a follow up PR for the docs

epage · 2025-10-13T12:14:38Z

src/cargo/core/compiler/layout.rs

+    }
+}
+
+impl ArtifactDirLayout {


We generally prefer impl blocks to be next to their types.

I can understand keeping it here in this commit to make it easier to review but would be good to move in a follow up commit

sure, added that change in 6751e46

ranger-ross · 2025-10-13T13:34:21Z

Seems there is a test in CI that started failing in nightly-2025-10-13.

Trying to hunt down which change caused this but it seemingly just needs to be blessed.

weihanglo · 2025-10-13T13:50:01Z

It failed due to this rust-lang/rust#142390. We addressed it in advance by #16082 but not for nightly-only cargo tests.

… `ArtifactDirLayout`

…rtifactDirLayout`

Update cargo 17 commits in 81c3f77a467359c8be6bc747dc93ec66a6e4ce11..367fd9f213750cd40317803dd0a5a3ce3f0c676d 2025-10-10 18:41:02 +0000 to 2025-10-15 15:01:32 +0000 - test: Don't look for a specfic ANSI color (rust-lang/cargo#16118) - docs(guide): Clarify where to set config (rust-lang/cargo#16107) - test(rustfix): re-enable disabled test due to unused variables (rust-lang/cargo#16114) - Convert the "manifest has no things" warning to annotate_snippets. (rust-lang/cargo#16113) - doc: make it clearer that `target.<cfg>.linker` is supported (rust-lang/cargo#16112) - docs(guide): Cover feature-unification (rust-lang/cargo#16108) - fix(gctx): types are unsupported not unknown (rust-lang/cargo#16109) - fix(script): Tweak cargo script build-dir / target-dir (rust-lang/cargo#16086) - docs(gctx): explain Value deserialization step-by-step (rust-lang/cargo#16105) - docs(guide): Talk about removing unused features (rust-lang/cargo#16085) - test(config): exercise unsupported TOML types (rust-lang/cargo#16100) - docs(gctx): a bit more of how config deserialization works (rust-lang/cargo#16094) - Refactor `Layout` into `BuildDirLayout` and `ArtifactDirLayout` (rust-lang/cargo#16092) - Add alternative linker to the build performance guide (rust-lang/cargo#15991) - refactor(gctx): extract error to a module (rust-lang/cargo#16091) - fix: Fixed nightly tests failing due to unused_variables lint (rust-lang/cargo#16098) - fix(script): Store cargo script lockfiles in build-dir (rust-lang/cargo#16087) r? ghost

rustbot assigned ehuss Oct 12, 2025

weihanglo reviewed Oct 12, 2025

View reviewed changes

epage reviewed Oct 13, 2025

View reviewed changes

weihanglo approved these changes Oct 13, 2025

View reviewed changes

ranger-ross mentioned this pull request Oct 13, 2025

Nightly CI test failures due to unused assignment linting changes #16097

Closed

ranger-ross added 4 commits October 14, 2025 01:56

refactor(layout): Moved cargo timings directory to Layout

d89d1de

refactor(layout): Split Layout internally into BuildDirLayout and…

df7dbd6

… `ArtifactDirLayout`

refactor(layout): Moved Layout accessors to BuildDirLayout and `A…

ce76237

…rtifactDirLayout`

refactor(layout): Moved impl blocks next to thier struct definition

03332a7

ranger-ross force-pushed the refactor-layout-struct branch from 6751e46 to 03332a7 Compare October 13, 2025 16:57

weihanglo enabled auto-merge October 13, 2025 16:59

weihanglo added this pull request to the merge queue Oct 13, 2025

Merged via the queue into rust-lang:master with commit 6f3896d Oct 13, 2025
25 checks passed

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 13, 2025

rustbot mentioned this pull request Oct 13, 2025

(experiment) Fine grain locking #16089

Draft

ranger-ross deleted the refactor-layout-struct branch October 14, 2025 14:29

Muscraft mentioned this pull request Oct 15, 2025

Update cargo rust-lang/rust#147730

Merged

rustbot added this to the 1.92.0 milestone Oct 15, 2025

Refactor Layout into BuildDirLayout and ArtifactDirLayout #16092

Refactor Layout into BuildDirLayout and ArtifactDirLayout #16092

Uh oh!

Conversation

ranger-ross commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How to test and review this PR?

Uh oh!

rustbot commented Oct 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranger-ross commented Oct 13, 2025

Uh oh!

weihanglo commented Oct 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Refactor `Layout` into `BuildDirLayout` and `ArtifactDirLayout` #16092

Refactor `Layout` into `BuildDirLayout` and `ArtifactDirLayout` #16092

ranger-ross commented Oct 12, 2025 •

edited

Loading