Skip to content

Commit a4d2bc4

Browse files
authored
Rollup merge of rust-lang#95906 - jyn514:enforce-valid-paths, r=Mark-Simulacrum
Require all paths passed to `ShouldRun::paths` to exist on disk This has two benefits: 1. There is a clearer mental model of how bootstrap works. Steps correspond to paths on disk unless it's strictly impossible for them to do so (e.g. dist components). 2. Bootstrap has better checks for internal consistency. This caught several issues: - `src/sanitizers` doesn't exist; I changed it to just be a `sanitizers` alias. - `src/tools/lld` doesn't exist; I removed it, since `lld` alone already works. - `src/llvm` doesn't exist; removed it since `llvm` and `src/llvm-project` both work. - `src/lldb_batchmode.py` doesn't exist, it was moved to `src/etc`. - `install` was still using `src/librustc` instead of `compiler/rustc`. - None of the tools in `dist` / `install` allowed using `src/tools/X` to build them. This might be intentional - I can change them to aliases if you like. Builds on rust-lang#95901 and should not be merged before.
2 parents 7f6d83e + 0db70ca commit a4d2bc4

File tree

6 files changed

+66
-42
lines changed

6 files changed

+66
-42
lines changed

src/bootstrap/builder.rs

+24-1
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,19 @@ impl<'a> ShouldRun<'a> {
364364
self
365365
}
366366

367+
// single alias, which does not correspond to any on-disk path
368+
pub fn alias(mut self, alias: &str) -> Self {
369+
assert!(
370+
!self.builder.src.join(alias).exists(),
371+
"use `builder.path()` for real paths: {}",
372+
alias
373+
);
374+
self.paths.insert(PathSet::Set(
375+
std::iter::once(TaskPath { path: alias.into(), kind: Some(self.kind) }).collect(),
376+
));
377+
self
378+
}
379+
367380
// single, non-aliased path
368381
pub fn path(self, path: &str) -> Self {
369382
self.paths(&[path])
@@ -372,7 +385,17 @@ impl<'a> ShouldRun<'a> {
372385
// multiple aliases for the same job
373386
pub fn paths(mut self, paths: &[&str]) -> Self {
374387
self.paths.insert(PathSet::Set(
375-
paths.iter().map(|p| TaskPath { path: p.into(), kind: Some(self.kind) }).collect(),
388+
paths
389+
.iter()
390+
.map(|p| {
391+
assert!(
392+
self.builder.src.join(p).exists(),
393+
"`should_run.paths` should correspond to real on-disk paths - use `alias` if there is no relevant path: {}",
394+
p
395+
);
396+
TaskPath { path: p.into(), kind: Some(self.kind) }
397+
})
398+
.collect(),
376399
));
377400
self
378401
}

src/bootstrap/dist.rs

+23-22
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ impl Step for Docs {
6161

6262
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
6363
let default = run.builder.config.docs;
64-
run.path("rust-docs").default_condition(default)
64+
run.alias("rust-docs").default_condition(default)
6565
}
6666

6767
fn make_run(run: RunConfig<'_>) {
@@ -94,7 +94,7 @@ impl Step for RustcDocs {
9494

9595
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
9696
let builder = run.builder;
97-
run.path("rustc-docs").default_condition(builder.config.compiler_docs)
97+
run.alias("rustc-docs").default_condition(builder.config.compiler_docs)
9898
}
9999

100100
fn make_run(run: RunConfig<'_>) {
@@ -272,7 +272,7 @@ impl Step for Mingw {
272272
const DEFAULT: bool = true;
273273

274274
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
275-
run.path("rust-mingw")
275+
run.alias("rust-mingw")
276276
}
277277

278278
fn make_run(run: RunConfig<'_>) {
@@ -313,7 +313,7 @@ impl Step for Rustc {
313313
const ONLY_HOSTS: bool = true;
314314

315315
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
316-
run.path("rustc")
316+
run.alias("rustc")
317317
}
318318

319319
fn make_run(run: RunConfig<'_>) {
@@ -456,7 +456,7 @@ impl Step for DebuggerScripts {
456456
type Output = ();
457457

458458
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
459-
run.path("src/lldb_batchmode.py")
459+
run.path("src/etc/lldb_batchmode.py")
460460
}
461461

462462
fn make_run(run: RunConfig<'_>) {
@@ -547,7 +547,7 @@ impl Step for Std {
547547
const DEFAULT: bool = true;
548548

549549
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
550-
run.path("rust-std")
550+
run.alias("rust-std")
551551
}
552552

553553
fn make_run(run: RunConfig<'_>) {
@@ -594,7 +594,7 @@ impl Step for RustcDev {
594594
const ONLY_HOSTS: bool = true;
595595

596596
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
597-
run.path("rustc-dev")
597+
run.alias("rustc-dev")
598598
}
599599

600600
fn make_run(run: RunConfig<'_>) {
@@ -653,7 +653,7 @@ impl Step for Analysis {
653653

654654
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
655655
let default = should_build_extended_tool(&run.builder, "analysis");
656-
run.path("rust-analysis").default_condition(default)
656+
run.alias("rust-analysis").default_condition(default)
657657
}
658658

659659
fn make_run(run: RunConfig<'_>) {
@@ -790,7 +790,7 @@ impl Step for Src {
790790
const ONLY_HOSTS: bool = true;
791791

792792
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
793-
run.path("rust-src")
793+
run.alias("rust-src")
794794
}
795795

796796
fn make_run(run: RunConfig<'_>) {
@@ -848,7 +848,7 @@ impl Step for PlainSourceTarball {
848848

849849
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
850850
let builder = run.builder;
851-
run.path("rustc-src").default_condition(builder.config.rust_dist_src)
851+
run.alias("rustc-src").default_condition(builder.config.rust_dist_src)
852852
}
853853

854854
fn make_run(run: RunConfig<'_>) {
@@ -942,7 +942,7 @@ impl Step for Cargo {
942942

943943
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
944944
let default = should_build_extended_tool(&run.builder, "cargo");
945-
run.path("cargo").default_condition(default)
945+
run.alias("cargo").default_condition(default)
946946
}
947947

948948
fn make_run(run: RunConfig<'_>) {
@@ -998,7 +998,7 @@ impl Step for Rls {
998998

999999
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
10001000
let default = should_build_extended_tool(&run.builder, "rls");
1001-
run.path("rls").default_condition(default)
1001+
run.alias("rls").default_condition(default)
10021002
}
10031003

10041004
fn make_run(run: RunConfig<'_>) {
@@ -1045,7 +1045,7 @@ impl Step for RustAnalyzer {
10451045

10461046
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
10471047
let default = should_build_extended_tool(&run.builder, "rust-analyzer");
1048-
run.path("rust-analyzer").default_condition(default)
1048+
run.alias("rust-analyzer").default_condition(default)
10491049
}
10501050

10511051
fn make_run(run: RunConfig<'_>) {
@@ -1101,7 +1101,7 @@ impl Step for Clippy {
11011101

11021102
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
11031103
let default = should_build_extended_tool(&run.builder, "clippy");
1104-
run.path("clippy").default_condition(default)
1104+
run.alias("clippy").default_condition(default)
11051105
}
11061106

11071107
fn make_run(run: RunConfig<'_>) {
@@ -1152,7 +1152,7 @@ impl Step for Miri {
11521152

11531153
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
11541154
let default = should_build_extended_tool(&run.builder, "miri");
1155-
run.path("miri").default_condition(default)
1155+
run.alias("miri").default_condition(default)
11561156
}
11571157

11581158
fn make_run(run: RunConfig<'_>) {
@@ -1212,7 +1212,7 @@ impl Step for Rustfmt {
12121212

12131213
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
12141214
let default = should_build_extended_tool(&run.builder, "rustfmt");
1215-
run.path("rustfmt").default_condition(default)
1215+
run.alias("rustfmt").default_condition(default)
12161216
}
12171217

12181218
fn make_run(run: RunConfig<'_>) {
@@ -1271,7 +1271,7 @@ impl Step for RustDemangler {
12711271
// we run the step by default when only `extended = true`, and decide whether to actually
12721272
// run it or not later.
12731273
let default = run.builder.config.extended;
1274-
run.path("rust-demangler").default_condition(default)
1274+
run.alias("rust-demangler").default_condition(default)
12751275
}
12761276

12771277
fn make_run(run: RunConfig<'_>) {
@@ -1324,7 +1324,7 @@ impl Step for Extended {
13241324

13251325
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
13261326
let builder = run.builder;
1327-
run.path("extended").default_condition(builder.config.extended)
1327+
run.alias("extended").default_condition(builder.config.extended)
13281328
}
13291329

13301330
fn make_run(run: RunConfig<'_>) {
@@ -1968,7 +1968,8 @@ impl Step for LlvmTools {
19681968

19691969
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
19701970
let default = should_build_extended_tool(&run.builder, "llvm-tools");
1971-
run.path("llvm-tools").default_condition(default)
1971+
// FIXME: allow using the names of the tools themselves?
1972+
run.alias("llvm-tools").default_condition(default)
19721973
}
19731974

19741975
fn make_run(run: RunConfig<'_>) {
@@ -2022,7 +2023,7 @@ impl Step for RustDev {
20222023
const ONLY_HOSTS: bool = true;
20232024

20242025
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
2025-
run.path("rust-dev")
2026+
run.alias("rust-dev")
20262027
}
20272028

20282029
fn make_run(run: RunConfig<'_>) {
@@ -2098,7 +2099,7 @@ impl Step for BuildManifest {
20982099
const ONLY_HOSTS: bool = true;
20992100

21002101
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
2101-
run.path("build-manifest")
2102+
run.alias("build-manifest")
21022103
}
21032104

21042105
fn make_run(run: RunConfig<'_>) {
@@ -2130,7 +2131,7 @@ impl Step for ReproducibleArtifacts {
21302131
const ONLY_HOSTS: bool = true;
21312132

21322133
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
2133-
run.path("reproducible-artifacts")
2134+
run.alias("reproducible-artifacts")
21342135
}
21352136

21362137
fn make_run(run: RunConfig<'_>) {

src/bootstrap/install.rs

+14-14
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ fn prepare_dir(mut path: PathBuf) -> String {
9393
macro_rules! install {
9494
(($sel:ident, $builder:ident, $_config:ident),
9595
$($name:ident,
96-
$path:expr,
96+
$condition_name: ident = $path_or_alias: literal,
9797
$default_cond:expr,
9898
only_hosts: $only_hosts:expr,
9999
$run_item:block $(, $c:ident)*;)+) => {
@@ -108,7 +108,7 @@ macro_rules! install {
108108
#[allow(dead_code)]
109109
fn should_build(config: &Config) -> bool {
110110
config.extended && config.tools.as_ref()
111-
.map_or(true, |t| t.contains($path))
111+
.map_or(true, |t| t.contains($path_or_alias))
112112
}
113113
}
114114

@@ -120,7 +120,7 @@ macro_rules! install {
120120

121121
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
122122
let $_config = &run.builder.config;
123-
run.path($path).default_condition($default_cond)
123+
run.$condition_name($path_or_alias).default_condition($default_cond)
124124
}
125125

126126
fn make_run(run: RunConfig<'_>) {
@@ -138,11 +138,11 @@ macro_rules! install {
138138
}
139139

140140
install!((self, builder, _config),
141-
Docs, "src/doc", _config.docs, only_hosts: false, {
141+
Docs, path = "src/doc", _config.docs, only_hosts: false, {
142142
let tarball = builder.ensure(dist::Docs { host: self.target }).expect("missing docs");
143143
install_sh(builder, "docs", self.compiler.stage, Some(self.target), &tarball);
144144
};
145-
Std, "library/std", true, only_hosts: false, {
145+
Std, path = "library/std", true, only_hosts: false, {
146146
for target in &builder.targets {
147147
// `expect` should be safe, only None when host != build, but this
148148
// only runs when host == build
@@ -153,13 +153,13 @@ install!((self, builder, _config),
153153
install_sh(builder, "std", self.compiler.stage, Some(*target), &tarball);
154154
}
155155
};
156-
Cargo, "cargo", Self::should_build(_config), only_hosts: true, {
156+
Cargo, alias = "cargo", Self::should_build(_config), only_hosts: true, {
157157
let tarball = builder
158158
.ensure(dist::Cargo { compiler: self.compiler, target: self.target })
159159
.expect("missing cargo");
160160
install_sh(builder, "cargo", self.compiler.stage, Some(self.target), &tarball);
161161
};
162-
Rls, "rls", Self::should_build(_config), only_hosts: true, {
162+
Rls, alias = "rls", Self::should_build(_config), only_hosts: true, {
163163
if let Some(tarball) = builder.ensure(dist::Rls { compiler: self.compiler, target: self.target }) {
164164
install_sh(builder, "rls", self.compiler.stage, Some(self.target), &tarball);
165165
} else {
@@ -168,7 +168,7 @@ install!((self, builder, _config),
168168
);
169169
}
170170
};
171-
RustAnalyzer, "rust-analyzer", Self::should_build(_config), only_hosts: true, {
171+
RustAnalyzer, alias = "rust-analyzer", Self::should_build(_config), only_hosts: true, {
172172
if let Some(tarball) =
173173
builder.ensure(dist::RustAnalyzer { compiler: self.compiler, target: self.target })
174174
{
@@ -179,13 +179,13 @@ install!((self, builder, _config),
179179
);
180180
}
181181
};
182-
Clippy, "clippy", Self::should_build(_config), only_hosts: true, {
182+
Clippy, alias = "clippy", Self::should_build(_config), only_hosts: true, {
183183
let tarball = builder
184184
.ensure(dist::Clippy { compiler: self.compiler, target: self.target })
185185
.expect("missing clippy");
186186
install_sh(builder, "clippy", self.compiler.stage, Some(self.target), &tarball);
187187
};
188-
Miri, "miri", Self::should_build(_config), only_hosts: true, {
188+
Miri, alias = "miri", Self::should_build(_config), only_hosts: true, {
189189
if let Some(tarball) = builder.ensure(dist::Miri { compiler: self.compiler, target: self.target }) {
190190
install_sh(builder, "miri", self.compiler.stage, Some(self.target), &tarball);
191191
} else {
@@ -194,7 +194,7 @@ install!((self, builder, _config),
194194
);
195195
}
196196
};
197-
Rustfmt, "rustfmt", Self::should_build(_config), only_hosts: true, {
197+
Rustfmt, alias = "rustfmt", Self::should_build(_config), only_hosts: true, {
198198
if let Some(tarball) = builder.ensure(dist::Rustfmt {
199199
compiler: self.compiler,
200200
target: self.target
@@ -206,7 +206,7 @@ install!((self, builder, _config),
206206
);
207207
}
208208
};
209-
RustDemangler, "rust-demangler", Self::should_build(_config), only_hosts: true, {
209+
RustDemangler, alias = "rust-demangler", Self::should_build(_config), only_hosts: true, {
210210
// Note: Even though `should_build` may return true for `extended` default tools,
211211
// dist::RustDemangler may still return None, unless the target-dependent `profiler` config
212212
// is also true, or the `tools` array explicitly includes "rust-demangler".
@@ -222,7 +222,7 @@ install!((self, builder, _config),
222222
);
223223
}
224224
};
225-
Analysis, "analysis", Self::should_build(_config), only_hosts: false, {
225+
Analysis, alias = "analysis", Self::should_build(_config), only_hosts: false, {
226226
// `expect` should be safe, only None with host != build, but this
227227
// only uses the `build` compiler
228228
let tarball = builder.ensure(dist::Analysis {
@@ -234,7 +234,7 @@ install!((self, builder, _config),
234234
}).expect("missing analysis");
235235
install_sh(builder, "analysis", self.compiler.stage, Some(self.target), &tarball);
236236
};
237-
Rustc, "src/librustc", true, only_hosts: true, {
237+
Rustc, path = "compiler/rustc", true, only_hosts: true, {
238238
let tarball = builder.ensure(dist::Rustc {
239239
compiler: builder.compiler(builder.top_stage, self.target),
240240
});

src/bootstrap/native.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ impl Step for Llvm {
122122
const ONLY_HOSTS: bool = true;
123123

124124
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
125-
run.path("src/llvm-project").path("src/llvm-project/llvm").path("src/llvm")
125+
run.path("src/llvm-project").path("src/llvm-project/llvm")
126126
}
127127

128128
fn make_run(run: RunConfig<'_>) {
@@ -605,7 +605,7 @@ impl Step for Lld {
605605
const ONLY_HOSTS: bool = true;
606606

607607
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
608-
run.path("src/llvm-project/lld").path("src/tools/lld")
608+
run.path("src/llvm-project/lld")
609609
}
610610

611611
fn make_run(run: RunConfig<'_>) {
@@ -771,7 +771,7 @@ impl Step for Sanitizers {
771771
type Output = Vec<SanitizerRuntime>;
772772

773773
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
774-
run.path("src/llvm-project/compiler-rt").path("src/sanitizers")
774+
run.alias("sanitizers")
775775
}
776776

777777
fn make_run(run: RunConfig<'_>) {

src/bootstrap/test.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2288,7 +2288,7 @@ impl Step for Distcheck {
22882288
type Output = ();
22892289

22902290
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
2291-
run.path("distcheck")
2291+
run.alias("distcheck")
22922292
}
22932293

22942294
fn make_run(run: RunConfig<'_>) {

0 commit comments

Comments
 (0)