Skip to content

Commit f7cc467

Browse files
alexcrichtonalexcrichton
committed
rustbuild: Tweak how stage1 compilers are selected
This commit furthers the previous one to ensure that we don't build an extra stage of the compiler in CI. A test has been added to rustbuild to ensure that this doesn't regress, and then in debugging this test it was hunted down that the `dist::Std` target was the one erroneously pulling in the wrong compiler. The `dist::Std` step was updated to instead account for the "full bootstrap" or not flag, ensuring that the correct compiler for compiling the final standard library was used. This was another use of the `force_use_stage1` function which was in theory supposed to be pretty central, so existing users were all evaluated and a new function, `Builder::compiler_for`, was introduced. All existing users of `force_use_stage1` have been updated to use `compiler_for`, where the semantics of `compiler_for` are similar to that of `compiler` except that it doesn't guarantee the presence of a sysroot for the arguments passed (as they may be modified). Perhaps one day we can unify `compiler` and `compiler_for`, but the usage of `Builder::compiler` is so ubiquitous it would take quite some time to evaluate whether each one needs the sysroot or not, so it's hoped that can be done in parallel.
1 parent ad52c77 commit f7cc467

File tree

5 files changed

+98
-98
lines changed

5 files changed

+98
-98
lines changed

‎src/bootstrap/builder.rs

+63-38
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,25 @@ impl<'a> Builder<'a> {
577577
})
578578
}
579579

580+
/// Similar to `compiler`, except handles the full-bootstrap option to
581+
/// silently use the stage1 compiler instead of a stage2 compiler if one is
582+
/// requested.
583+
///
584+
/// Note that this does *not* have the side effect of creating
585+
/// `compiler(stage, host)`, unlike `compiler` above which does have such
586+
/// a side effect. The returned compiler here can only be used to compile
587+
/// new artifacts, it can't be used to rely on the presence of a particular
588+
/// sysroot.
589+
///
590+
/// See `force_use_stage1` for documentation on what each argument is.
591+
pub fn compiler_for(&self, stage: u32, host: Interned<String>, target: Interned<String>) -> Compiler {
592+
if self.build.force_use_stage1(Compiler { stage, host }, target) {
593+
self.compiler(1, self.config.build)
594+
} else {
595+
self.compiler(stage, host)
596+
}
597+
}
598+
580599
pub fn sysroot(&self, compiler: Compiler) -> Interned<PathBuf> {
581600
self.ensure(compile::Sysroot { compiler })
582601
}
@@ -750,11 +769,7 @@ impl<'a> Builder<'a> {
750769
// This is for the original compiler, but if we're forced to use stage 1, then
751770
// std/test/rustc stamps won't exist in stage 2, so we need to get those from stage 1, since
752771
// we copy the libs forward.
753-
let cmp = if self.force_use_stage1(compiler, target) {
754-
self.compiler(1, compiler.host)
755-
} else {
756-
compiler
757-
};
772+
let cmp = self.compiler_for(compiler.stage, compiler.host, target);
758773

759774
let libstd_stamp = match cmd {
760775
"check" => check::libstd_stamp(self, cmp, target),
@@ -1371,7 +1386,7 @@ mod __test {
13711386
assert_eq!(
13721387
first(builder.cache.all::<dist::Std>()),
13731388
&[dist::Std {
1374-
compiler: Compiler { host: a, stage: 2 },
1389+
compiler: Compiler { host: a, stage: 1 },
13751390
target: a,
13761391
},]
13771392
);
@@ -1408,7 +1423,7 @@ mod __test {
14081423
first(builder.cache.all::<dist::Std>()),
14091424
&[
14101425
dist::Std {
1411-
compiler: Compiler { host: a, stage: 2 },
1426+
compiler: Compiler { host: a, stage: 1 },
14121427
target: a,
14131428
},
14141429
dist::Std {
@@ -1455,18 +1470,51 @@ mod __test {
14551470
first(builder.cache.all::<dist::Std>()),
14561471
&[
14571472
dist::Std {
1458-
compiler: Compiler { host: a, stage: 2 },
1473+
compiler: Compiler { host: a, stage: 1 },
14591474
target: a,
14601475
},
14611476
dist::Std {
1462-
compiler: Compiler { host: a, stage: 2 },
1477+
compiler: Compiler { host: a, stage: 1 },
14631478
target: b,
14641479
},
14651480
]
14661481
);
14671482
assert_eq!(first(builder.cache.all::<dist::Src>()), &[dist::Src]);
14681483
}
14691484

1485+
#[test]
1486+
fn dist_only_cross_host() {
1487+
let a = INTERNER.intern_str("A");
1488+
let b = INTERNER.intern_str("B");
1489+
let mut build = Build::new(configure(&["B"], &[]));
1490+
build.config.docs = false;
1491+
build.hosts = vec![b];
1492+
let mut builder = Builder::new(&build);
1493+
builder.run_step_descriptions(&Builder::get_step_descriptions(Kind::Dist), &[]);
1494+
1495+
assert_eq!(
1496+
first(builder.cache.all::<dist::Rustc>()),
1497+
&[
1498+
dist::Rustc {
1499+
compiler: Compiler { host: b, stage: 2 }
1500+
},
1501+
]
1502+
);
1503+
assert_eq!(
1504+
first(builder.cache.all::<compile::Rustc>()),
1505+
&[
1506+
compile::Rustc {
1507+
compiler: Compiler { host: a, stage: 0 },
1508+
target: a,
1509+
},
1510+
compile::Rustc {
1511+
compiler: Compiler { host: a, stage: 1 },
1512+
target: b,
1513+
},
1514+
]
1515+
);
1516+
}
1517+
14701518
#[test]
14711519
fn dist_with_targets_and_hosts() {
14721520
let build = Build::new(configure(&["B"], &["C"]));
@@ -1508,11 +1556,11 @@ mod __test {
15081556
first(builder.cache.all::<dist::Std>()),
15091557
&[
15101558
dist::Std {
1511-
compiler: Compiler { host: a, stage: 2 },
1559+
compiler: Compiler { host: a, stage: 1 },
15121560
target: a,
15131561
},
15141562
dist::Std {
1515-
compiler: Compiler { host: a, stage: 2 },
1563+
compiler: Compiler { host: a, stage: 1 },
15161564
target: b,
15171565
},
15181566
dist::Std {
@@ -1557,11 +1605,11 @@ mod __test {
15571605
first(builder.cache.all::<dist::Std>()),
15581606
&[
15591607
dist::Std {
1560-
compiler: Compiler { host: a, stage: 2 },
1608+
compiler: Compiler { host: a, stage: 1 },
15611609
target: a,
15621610
},
15631611
dist::Std {
1564-
compiler: Compiler { host: a, stage: 2 },
1612+
compiler: Compiler { host: a, stage: 1 },
15651613
target: b,
15661614
},
15671615
dist::Std {
@@ -1608,11 +1656,11 @@ mod __test {
16081656
first(builder.cache.all::<dist::Std>()),
16091657
&[
16101658
dist::Std {
1611-
compiler: Compiler { host: a, stage: 2 },
1659+
compiler: Compiler { host: a, stage: 1 },
16121660
target: a,
16131661
},
16141662
dist::Std {
1615-
compiler: Compiler { host: a, stage: 2 },
1663+
compiler: Compiler { host: a, stage: 1 },
16161664
target: b,
16171665
},
16181666
]
@@ -1662,10 +1710,6 @@ mod __test {
16621710
compiler: Compiler { host: a, stage: 1 },
16631711
target: b,
16641712
},
1665-
compile::Test {
1666-
compiler: Compiler { host: a, stage: 2 },
1667-
target: b,
1668-
},
16691713
]
16701714
);
16711715
assert_eq!(
@@ -1718,10 +1762,6 @@ mod __test {
17181762
compiler: Compiler { host: b, stage: 2 },
17191763
target: a,
17201764
},
1721-
compile::Rustc {
1722-
compiler: Compiler { host: a, stage: 0 },
1723-
target: b,
1724-
},
17251765
compile::Rustc {
17261766
compiler: Compiler { host: a, stage: 1 },
17271767
target: b,
@@ -1756,10 +1796,6 @@ mod __test {
17561796
compiler: Compiler { host: b, stage: 2 },
17571797
target: a,
17581798
},
1759-
compile::Test {
1760-
compiler: Compiler { host: a, stage: 0 },
1761-
target: b,
1762-
},
17631799
compile::Test {
17641800
compiler: Compiler { host: a, stage: 1 },
17651801
target: b,
@@ -1806,9 +1842,6 @@ mod __test {
18061842
compile::Assemble {
18071843
target_compiler: Compiler { host: a, stage: 1 },
18081844
},
1809-
compile::Assemble {
1810-
target_compiler: Compiler { host: b, stage: 1 },
1811-
},
18121845
compile::Assemble {
18131846
target_compiler: Compiler { host: a, stage: 2 },
18141847
},
@@ -1828,10 +1861,6 @@ mod __test {
18281861
compiler: Compiler { host: a, stage: 1 },
18291862
target: a,
18301863
},
1831-
compile::Rustc {
1832-
compiler: Compiler { host: a, stage: 0 },
1833-
target: b,
1834-
},
18351864
compile::Rustc {
18361865
compiler: Compiler { host: a, stage: 1 },
18371866
target: b,
@@ -1858,10 +1887,6 @@ mod __test {
18581887
compiler: Compiler { host: b, stage: 2 },
18591888
target: a,
18601889
},
1861-
compile::Test {
1862-
compiler: Compiler { host: a, stage: 0 },
1863-
target: b,
1864-
},
18651890
compile::Test {
18661891
compiler: Compiler { host: a, stage: 1 },
18671892
target: b,

‎src/bootstrap/compile.rs

+16-13
Original file line numberDiff line numberDiff line change
@@ -70,20 +70,20 @@ impl Step for Std {
7070

7171
builder.ensure(StartupObjects { compiler, target });
7272

73-
if builder.force_use_stage1(compiler, target) {
74-
let from = builder.compiler(1, builder.config.build);
73+
let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target);
74+
if compiler_to_use != compiler {
7575
builder.ensure(Std {
76-
compiler: from,
76+
compiler: compiler_to_use,
7777
target,
7878
});
79-
builder.info(&format!("Uplifting stage1 std ({} -> {})", from.host, target));
79+
builder.info(&format!("Uplifting stage1 std ({} -> {})", compiler_to_use.host, target));
8080

8181
// Even if we're not building std this stage, the new sysroot must
8282
// still contain the third party objects needed by various targets.
8383
copy_third_party_objects(builder, &compiler, target);
8484

8585
builder.ensure(StdLink {
86-
compiler: from,
86+
compiler: compiler_to_use,
8787
target_compiler: compiler,
8888
target,
8989
});
@@ -402,15 +402,16 @@ impl Step for Test {
402402
return;
403403
}
404404

405-
if builder.force_use_stage1(compiler, target) {
405+
let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target);
406+
if compiler_to_use != compiler {
406407
builder.ensure(Test {
407-
compiler: builder.compiler(1, builder.config.build),
408+
compiler: compiler_to_use,
408409
target,
409410
});
410411
builder.info(
411412
&format!("Uplifting stage1 test ({} -> {})", builder.config.build, target));
412413
builder.ensure(TestLink {
413-
compiler: builder.compiler(1, builder.config.build),
414+
compiler: compiler_to_use,
414415
target_compiler: compiler,
415416
target,
416417
});
@@ -527,15 +528,16 @@ impl Step for Rustc {
527528
return;
528529
}
529530

530-
if builder.force_use_stage1(compiler, target) {
531+
let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target);
532+
if compiler_to_use != compiler {
531533
builder.ensure(Rustc {
532-
compiler: builder.compiler(1, builder.config.build),
534+
compiler: compiler_to_use,
533535
target,
534536
});
535537
builder.info(&format!("Uplifting stage1 rustc ({} -> {})",
536538
builder.config.build, target));
537539
builder.ensure(RustcLink {
538-
compiler: builder.compiler(1, builder.config.build),
540+
compiler: compiler_to_use,
539541
target_compiler: compiler,
540542
target,
541543
});
@@ -691,9 +693,10 @@ impl Step for CodegenBackend {
691693
return;
692694
}
693695

694-
if builder.force_use_stage1(compiler, target) {
696+
let compiler_to_use = builder.compiler_for(compiler.stage, compiler.host, target);
697+
if compiler_to_use != compiler {
695698
builder.ensure(CodegenBackend {
696-
compiler: builder.compiler(1, builder.config.build),
699+
compiler: compiler_to_use,
697700
target,
698701
backend,
699702
});

‎src/bootstrap/dist.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,11 @@ impl Step for Std {
647647

648648
fn make_run(run: RunConfig<'_>) {
649649
run.builder.ensure(Std {
650-
compiler: run.builder.compiler(run.builder.top_stage, run.builder.config.build),
650+
compiler: run.builder.compiler_for(
651+
run.builder.top_stage,
652+
run.builder.config.build,
653+
run.target,
654+
),
651655
target: run.target,
652656
});
653657
}
@@ -757,13 +761,10 @@ impl Step for Analysis {
757761

758762
builder.ensure(Std { compiler, target });
759763

760-
// Package save-analysis from stage1 if not doing a full bootstrap, as the
761-
// stage2 artifacts is simply copied from stage1 in that case.
762-
let compiler = if builder.force_use_stage1(compiler, target) {
763-
builder.compiler(1, compiler.host)
764-
} else {
765-
compiler.clone()
766-
};
764+
// Find the actual compiler (handling the full bootstrap option) which
765+
// produced the save-analysis data because that data isn't copied
766+
// through the sysroot uplifting.
767+
let compiler = builder.compiler_for(compiler.stage, compiler.host, target);
767768

768769
let image = tmpdir(builder).join(format!("{}-{}-image", name, target));
769770

‎src/bootstrap/doc.rs

+5-30
Original file line numberDiff line numberDiff line change
@@ -475,12 +475,7 @@ impl Step for Std {
475475
builder.info(&format!("Documenting stage{} std ({})", stage, target));
476476
let out = builder.doc_out(target);
477477
t!(fs::create_dir_all(&out));
478-
let compiler = builder.compiler(stage, builder.config.build);
479-
let compiler = if builder.force_use_stage1(compiler, target) {
480-
builder.compiler(1, compiler.host)
481-
} else {
482-
compiler
483-
};
478+
let compiler = builder.compiler_for(stage, builder.config.build, target);
484479

485480
builder.ensure(compile::Std { compiler, target });
486481
let out_dir = builder.stage_out(compiler, Mode::Std)
@@ -563,12 +558,7 @@ impl Step for Test {
563558
builder.info(&format!("Documenting stage{} test ({})", stage, target));
564559
let out = builder.doc_out(target);
565560
t!(fs::create_dir_all(&out));
566-
let compiler = builder.compiler(stage, builder.config.build);
567-
let compiler = if builder.force_use_stage1(compiler, target) {
568-
builder.compiler(1, compiler.host)
569-
} else {
570-
compiler
571-
};
561+
let compiler = builder.compiler_for(stage, builder.config.build, target);
572562

573563
// Build libstd docs so that we generate relative links
574564
builder.ensure(Std { stage, target });
@@ -632,12 +622,7 @@ impl Step for WhitelistedRustc {
632622
builder.info(&format!("Documenting stage{} whitelisted compiler ({})", stage, target));
633623
let out = builder.doc_out(target);
634624
t!(fs::create_dir_all(&out));
635-
let compiler = builder.compiler(stage, builder.config.build);
636-
let compiler = if builder.force_use_stage1(compiler, target) {
637-
builder.compiler(1, compiler.host)
638-
} else {
639-
compiler
640-
};
625+
let compiler = builder.compiler_for(stage, builder.config.build, target);
641626

642627
// Build libstd docs so that we generate relative links
643628
builder.ensure(Std { stage, target });
@@ -706,12 +691,7 @@ impl Step for Rustc {
706691
t!(fs::create_dir_all(&out));
707692

708693
// Get the correct compiler for this stage.
709-
let compiler = builder.compiler(stage, builder.config.build);
710-
let compiler = if builder.force_use_stage1(compiler, target) {
711-
builder.compiler(1, compiler.host)
712-
} else {
713-
compiler
714-
};
694+
let compiler = builder.compiler_for(stage, builder.config.build, target);
715695

716696
if !builder.config.compiler_docs {
717697
builder.info("\tskipping - compiler/librustdoc docs disabled");
@@ -807,12 +787,7 @@ impl Step for Rustdoc {
807787
t!(fs::create_dir_all(&out));
808788

809789
// Get the correct compiler for this stage.
810-
let compiler = builder.compiler(stage, builder.config.build);
811-
let compiler = if builder.force_use_stage1(compiler, target) {
812-
builder.compiler(1, compiler.host)
813-
} else {
814-
compiler
815-
};
790+
let compiler = builder.compiler_for(stage, builder.config.build, target);
816791

817792
if !builder.config.compiler_docs {
818793
builder.info("\tskipping - compiler/librustdoc docs disabled");

0 commit comments

Comments
 (0)