Skip to content

Commit 6f3896d

Browse files
authored
Refactor Layout into BuildDirLayout and ArtifactDirLayout (#16092)
### What does this PR try to resolve? After the recent [`build-dir` changes](#14125), 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 `struct`s 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. 1. The first commit adds a `timings()` to the layout to bring it in line with other directories in `target` 2. Split `Layout` internally while keeping the pub interface unchanged. 3. Change the `Layout` public interface to expose `.build_dir()` and `.artifact_dir()` to make it explicit which directory is being used.
2 parents a19ac1f + 03332a7 commit 6f3896d

File tree

8 files changed

+170
-99
lines changed

8 files changed

+170
-99
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,7 +1141,7 @@ impl RustDocFingerprint {
11411141

11421142
let fingerprint_path = build_runner
11431143
.files()
1144-
.host_root()
1144+
.host_build_root()
11451145
.join(".rustdoc_fingerprint.json");
11461146
let write_fingerprint = || -> CargoResult<()> {
11471147
paths::write(
@@ -1181,7 +1181,7 @@ impl RustDocFingerprint {
11811181
.bcx
11821182
.all_kinds
11831183
.iter()
1184-
.map(|kind| build_runner.files().layout(*kind).doc())
1184+
.map(|kind| build_runner.files().layout(*kind).artifact_dir().doc())
11851185
.filter(|path| path.exists())
11861186
.try_for_each(|path| clean_doc(path))?;
11871187
write_fingerprint()?;

src/cargo/core/compiler/build_runner/compilation_files.rs

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,13 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
211211
// Docscrape units need to have doc/ set as the out_dir so sources for reverse-dependencies
212212
// will be put into doc/ and not into deps/ where the *.examples files are stored.
213213
if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
214-
self.layout(unit.kind).doc().to_path_buf()
214+
self.layout(unit.kind).artifact_dir().doc().to_path_buf()
215215
} else if unit.mode.is_doc_test() {
216216
panic!("doc tests do not have an out dir");
217217
} else if unit.target.is_custom_build() {
218218
self.build_script_dir(unit)
219219
} else if unit.target.is_example() {
220-
self.layout(unit.kind).build_examples().to_path_buf()
220+
self.layout(unit.kind).build_dir().examples().to_path_buf()
221221
} else if unit.artifact.is_true() {
222222
self.artifact_dir(unit)
223223
} else {
@@ -250,36 +250,41 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
250250

251251
/// Returns the final artifact path for the host (`/…/target/debug`)
252252
pub fn host_dest(&self) -> &Path {
253-
self.host.dest()
253+
self.host.artifact_dir().dest()
254254
}
255255

256-
/// Returns the root of the build output tree for the host (`/…/target`)
257-
pub fn host_root(&self) -> &Path {
258-
self.host.root()
256+
/// Returns the root of the build output tree for the host (`/…/build-dir`)
257+
pub fn host_build_root(&self) -> &Path {
258+
self.host.build_dir().root()
259259
}
260260

261261
/// Returns the host `deps` directory path.
262262
pub fn host_deps(&self, unit: &Unit) -> PathBuf {
263263
let dir = self.pkg_dir(unit);
264-
self.host.deps(&dir)
264+
self.host.build_dir().deps(&dir)
265265
}
266266

267267
/// Returns the directories where Rust crate dependencies are found for the
268268
/// specified unit.
269269
pub fn deps_dir(&self, unit: &Unit) -> PathBuf {
270270
let dir = self.pkg_dir(unit);
271-
self.layout(unit.kind).deps(&dir)
271+
self.layout(unit.kind).build_dir().deps(&dir)
272272
}
273273

274274
/// Directory where the fingerprint for the given unit should go.
275275
pub fn fingerprint_dir(&self, unit: &Unit) -> PathBuf {
276276
let dir = self.pkg_dir(unit);
277-
self.layout(unit.kind).fingerprint(&dir)
277+
self.layout(unit.kind).build_dir().fingerprint(&dir)
278278
}
279279

280280
/// Directory where incremental output for the given unit should go.
281281
pub fn incremental_dir(&self, unit: &Unit) -> &Path {
282-
self.layout(unit.kind).incremental()
282+
self.layout(unit.kind).build_dir().incremental()
283+
}
284+
285+
/// Directory where timing output should go.
286+
pub fn timings_dir(&self) -> &Path {
287+
self.host.artifact_dir().timings()
283288
}
284289

285290
/// Returns the path for a file in the fingerprint directory.
@@ -314,7 +319,9 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
314319
assert!(!unit.mode.is_run_custom_build());
315320
assert!(self.metas.contains_key(unit));
316321
let dir = self.pkg_dir(unit);
317-
self.layout(CompileKind::Host).build_script(&dir)
322+
self.layout(CompileKind::Host)
323+
.build_dir()
324+
.build_script(&dir)
318325
}
319326

320327
/// Returns the directory for compiled artifacts files.
@@ -338,7 +345,11 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
338345
invalid
339346
),
340347
};
341-
self.layout(unit.kind).artifact().join(dir).join(kind)
348+
self.layout(unit.kind)
349+
.build_dir()
350+
.artifact()
351+
.join(dir)
352+
.join(kind)
342353
}
343354

344355
/// Returns the directory where information about running a build script
@@ -348,7 +359,9 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
348359
assert!(unit.target.is_custom_build());
349360
assert!(unit.mode.is_run_custom_build());
350361
let dir = self.pkg_dir(unit);
351-
self.layout(unit.kind).build_script_execution(&dir)
362+
self.layout(unit.kind)
363+
.build_dir()
364+
.build_script_execution(&dir)
352365
}
353366

354367
/// Returns the "`OUT_DIR`" directory for running a build script.
@@ -367,7 +380,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
367380
bcx: &BuildContext<'_, '_>,
368381
) -> CargoResult<PathBuf> {
369382
assert!(target.is_bin());
370-
let dest = self.layout(kind).dest();
383+
let dest = self.layout(kind).artifact_dir().dest();
371384
let info = bcx.target_data.info(kind);
372385
let (file_types, _) = info
373386
.rustc_outputs(
@@ -435,11 +448,14 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
435448
let filename = file_type.uplift_filename(&unit.target);
436449
let uplift_path = if unit.target.is_example() {
437450
// Examples live in their own little world.
438-
self.layout(unit.kind).examples().join(filename)
451+
self.layout(unit.kind)
452+
.artifact_dir()
453+
.examples()
454+
.join(filename)
439455
} else if unit.target.is_custom_build() {
440456
self.build_script_dir(unit).join(filename)
441457
} else {
442-
self.layout(unit.kind).dest().join(filename)
458+
self.layout(unit.kind).artifact_dir().dest().join(filename)
443459
};
444460
if from_path == uplift_path {
445461
// This can happen with things like examples that reside in the

src/cargo/core/compiler/build_runner/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
401401
let layout = files.layout(kind);
402402
self.compilation
403403
.root_output
404-
.insert(kind, layout.dest().to_path_buf());
404+
.insert(kind, layout.artifact_dir().dest().to_path_buf());
405405
if self.bcx.gctx.cli_unstable().build_dir_new_layout {
406406
for (unit, _) in self.bcx.unit_graph.iter() {
407407
let dep_dir = self.files().deps_dir(unit);
@@ -411,7 +411,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
411411
} else {
412412
self.compilation
413413
.deps_output
414-
.insert(kind, layout.legacy_deps().to_path_buf());
414+
.insert(kind, layout.build_dir().legacy_deps().to_path_buf());
415415
}
416416
}
417417
Ok(())

src/cargo/core/compiler/layout.rs

Lines changed: 112 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -111,38 +111,8 @@ use std::path::{Path, PathBuf};
111111
///
112112
/// See module docs for more information.
113113
pub struct Layout {
114-
/// The root directory: `/path/to/target`.
115-
/// If cross compiling: `/path/to/target/$TRIPLE`.
116-
root: PathBuf,
117-
/// The final artifact destination: `$root/debug` (or `release`).
118-
dest: PathBuf,
119-
/// The directory with rustc artifacts: `$dest/deps`
120-
deps: PathBuf,
121-
/// The directory for build scripts: `$dest/build`
122-
build: PathBuf,
123-
/// The directory for artifacts, i.e. binaries, cdylibs, staticlibs: `$dest/deps/artifact`
124-
artifact: PathBuf,
125-
/// The directory for incremental files: `$dest/incremental`
126-
incremental: PathBuf,
127-
/// The directory for fingerprints: `$dest/.fingerprint`
128-
fingerprint: PathBuf,
129-
/// The directory for examples: `$dest/examples`
130-
examples: PathBuf,
131-
/// The directory for pre-uplifted examples: `$build-dir/debug/examples`
132-
build_examples: PathBuf,
133-
/// The directory for rustdoc output: `$root/doc`
134-
doc: PathBuf,
135-
/// The directory for temporary data of integration tests and benches: `$dest/tmp`
136-
tmp: PathBuf,
137-
/// The lockfile for a build (`.cargo-lock`). Will be unlocked when this
138-
/// struct is `drop`ped.
139-
_lock: FileLock,
140-
/// Same as `_lock` but for the build directory.
141-
///
142-
/// Will be `None` when the build-dir and target-dir are the same path as we cannot
143-
/// lock the same path twice.
144-
_build_lock: Option<FileLock>,
145-
is_new_layout: bool,
114+
artifact_dir: ArtifactDirLayout,
115+
build_dir: BuildDirLayout,
146116
}
147117

148118
impl Layout {
@@ -182,9 +152,10 @@ impl Layout {
182152
// For now we don't do any more finer-grained locking on the artifact
183153
// directory, so just lock the entire thing for the duration of this
184154
// compile.
185-
let lock = dest.open_rw_exclusive_create(".cargo-lock", ws.gctx(), "build directory")?;
155+
let artifact_dir_lock =
156+
dest.open_rw_exclusive_create(".cargo-lock", ws.gctx(), "build directory")?;
186157

187-
let build_lock = if root != build_root {
158+
let build_dir_lock = if root != build_root {
188159
Some(build_dest.open_rw_exclusive_create(
189160
".cargo-lock",
190161
ws.gctx(),
@@ -201,23 +172,112 @@ impl Layout {
201172
let artifact = deps.join("artifact");
202173

203174
Ok(Layout {
204-
deps,
205-
build: build_dest.join("build"),
206-
artifact,
207-
incremental: build_dest.join("incremental"),
208-
fingerprint: build_dest.join(".fingerprint"),
209-
examples: dest.join("examples"),
210-
build_examples: build_dest.join("examples"),
211-
doc: root.join("doc"),
212-
tmp: build_root.join("tmp"),
213-
root,
214-
dest,
215-
_lock: lock,
216-
_build_lock: build_lock,
217-
is_new_layout,
175+
artifact_dir: ArtifactDirLayout {
176+
dest: dest.clone(),
177+
examples: dest.join("examples"),
178+
doc: root.join("doc"),
179+
timings: root.join("cargo-timings"),
180+
_lock: artifact_dir_lock,
181+
},
182+
build_dir: BuildDirLayout {
183+
root: build_root.clone(),
184+
deps,
185+
build: build_dest.join("build"),
186+
artifact,
187+
incremental: build_dest.join("incremental"),
188+
fingerprint: build_dest.join(".fingerprint"),
189+
examples: build_dest.join("examples"),
190+
tmp: build_root.join("tmp"),
191+
_lock: build_dir_lock,
192+
is_new_layout,
193+
},
218194
})
219195
}
220196

197+
/// Makes sure all directories stored in the Layout exist on the filesystem.
198+
pub fn prepare(&mut self) -> CargoResult<()> {
199+
self.artifact_dir.prepare()?;
200+
self.build_dir.prepare()?;
201+
202+
Ok(())
203+
}
204+
205+
pub fn artifact_dir(&self) -> &ArtifactDirLayout {
206+
&self.artifact_dir
207+
}
208+
209+
pub fn build_dir(&self) -> &BuildDirLayout {
210+
&self.build_dir
211+
}
212+
}
213+
214+
pub struct ArtifactDirLayout {
215+
/// The final artifact destination: `<artifact-dir>/debug` (or `release`).
216+
dest: PathBuf,
217+
/// The directory for examples
218+
examples: PathBuf,
219+
/// The directory for rustdoc output
220+
doc: PathBuf,
221+
/// The directory for --timings output
222+
timings: PathBuf,
223+
/// The lockfile for a build (`.cargo-lock`). Will be unlocked when this
224+
/// struct is `drop`ped.
225+
_lock: FileLock,
226+
}
227+
228+
impl ArtifactDirLayout {
229+
/// Makes sure all directories stored in the Layout exist on the filesystem.
230+
pub fn prepare(&mut self) -> CargoResult<()> {
231+
paths::create_dir_all(&self.examples)?;
232+
233+
Ok(())
234+
}
235+
/// Fetch the destination path for final artifacts (`/…/target/debug`).
236+
pub fn dest(&self) -> &Path {
237+
&self.dest
238+
}
239+
/// Fetch the examples path.
240+
pub fn examples(&self) -> &Path {
241+
&self.examples
242+
}
243+
/// Fetch the doc path.
244+
pub fn doc(&self) -> &Path {
245+
&self.doc
246+
}
247+
/// Fetch the cargo-timings path.
248+
pub fn timings(&self) -> &Path {
249+
&self.timings
250+
}
251+
}
252+
253+
pub struct BuildDirLayout {
254+
/// The root directory: `/path/to/build-dir`.
255+
/// If cross compiling: `/path/to/build-dir/$TRIPLE`.
256+
root: PathBuf,
257+
/// The directory with rustc artifacts
258+
deps: PathBuf,
259+
/// The primary directory for build files
260+
build: PathBuf,
261+
/// The directory for artifacts, i.e. binaries, cdylibs, staticlibs
262+
artifact: PathBuf,
263+
/// The directory for incremental files
264+
incremental: PathBuf,
265+
/// The directory for fingerprints
266+
fingerprint: PathBuf,
267+
/// The directory for pre-uplifted examples: `build-dir/debug/examples`
268+
examples: PathBuf,
269+
/// The directory for temporary data of integration tests and benches
270+
tmp: PathBuf,
271+
/// The lockfile for a build (`.cargo-lock`). Will be unlocked when this
272+
/// struct is `drop`ped.
273+
///
274+
/// Will be `None` when the build-dir and target-dir are the same path as we cannot
275+
/// lock the same path twice.
276+
_lock: Option<FileLock>,
277+
is_new_layout: bool,
278+
}
279+
280+
impl BuildDirLayout {
221281
/// Makes sure all directories stored in the Layout exist on the filesystem.
222282
pub fn prepare(&mut self) -> CargoResult<()> {
223283
if !self.is_new_layout {
@@ -226,16 +286,10 @@ impl Layout {
226286
}
227287
paths::create_dir_all(&self.incremental)?;
228288
paths::create_dir_all(&self.examples)?;
229-
paths::create_dir_all(&self.build_examples)?;
230289
paths::create_dir_all(&self.build)?;
231290

232291
Ok(())
233292
}
234-
235-
/// Fetch the destination path for final artifacts (`/…/target/debug`).
236-
pub fn dest(&self) -> &Path {
237-
&self.dest
238-
}
239293
/// Fetch the deps path.
240294
pub fn deps(&self, pkg_dir: &str) -> PathBuf {
241295
if self.is_new_layout {
@@ -248,22 +302,13 @@ impl Layout {
248302
pub fn legacy_deps(&self) -> &Path {
249303
&self.deps
250304
}
251-
/// Fetch the examples path.
252-
pub fn examples(&self) -> &Path {
253-
&self.examples
254-
}
255-
/// Fetch the build examples path.
256-
pub fn build_examples(&self) -> &Path {
257-
&self.build_examples
258-
}
259-
/// Fetch the doc path.
260-
pub fn doc(&self) -> &Path {
261-
&self.doc
262-
}
263-
/// Fetch the root path (`/…/target`).
264305
pub fn root(&self) -> &Path {
265306
&self.root
266307
}
308+
/// Fetch the build examples path.
309+
pub fn examples(&self) -> &Path {
310+
&self.examples
311+
}
267312
/// Fetch the incremental path.
268313
pub fn incremental(&self) -> &Path {
269314
&self.incremental

0 commit comments

Comments
 (0)