Skip to content

Commit 5ab6924

Browse files
borsbors
committed
Auto merge of #145874 - Kobzol:fix-dist-builds, r=jieyouxu
Remove unnecessary stage2 host builds from cross-compiled dist builders This is a repeated regression (#138004, #138123) that was reintroduced in #145472. I thought that we have a test for it, but alas, the "correct" test required `--disable-docs`. I added the test in this PR, and re-added the `dist::Std` build optimization that solves this. r? `@jieyouxu`
2 parents 4356e83 + c7f90c1 commit 5ab6924

File tree

3 files changed

+112
-31
lines changed

3 files changed

+112
-31
lines changed

‎src/bootstrap/src/core/build_steps/dist.rs‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -764,10 +764,22 @@ pub struct Std {
764764

765765
impl Std {
766766
pub fn new(builder: &Builder<'_>, target: TargetSelection) -> Self {
767-
Std {
768-
build_compiler: builder.compiler(builder.top_stage, builder.config.host_target),
769-
target,
770-
}
767+
// This is an important optimization mainly for CI.
768+
// Normally, to build stage N libstd, we need stage N rustc.
769+
// However, if we know that we will uplift libstd from stage 1 anyway, building the stage N
770+
// rustc can be wasteful.
771+
// In particular, if we do a cross-compiling dist stage 2 build from T1 to T2, we need:
772+
// - stage 2 libstd for T2 (uplifted from stage 1, where it was built by T1 rustc)
773+
// - stage 2 rustc for T2
774+
// However, without this optimization, we would also build stage 2 rustc for **T1**, which
775+
// is completely wasteful.
776+
let build_compiler =
777+
if compile::Std::should_be_uplifted_from_stage_1(builder, builder.top_stage, target) {
778+
builder.compiler(1, builder.host_target)
779+
} else {
780+
builder.compiler(builder.top_stage, builder.host_target)
781+
};
782+
Std { build_compiler, target }
771783
}
772784
}
773785

‎src/bootstrap/src/core/builder/mod.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
145145
}
146146

147147
/// Metadata that describes an executed step, mostly for testing and tracing.
148-
#[allow(unused)]
149-
#[derive(Debug, PartialEq, Eq)]
148+
#[derive(Clone, Debug, PartialEq, Eq)]
150149
pub struct StepMetadata {
151150
name: String,
152151
kind: Kind,

‎src/bootstrap/src/core/builder/tests.rs‎

Lines changed: 95 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,8 +1133,7 @@ mod snapshot {
11331133
[dist] mingw <host>
11341134
[build] rustc 0 <host> -> GenerateCopyright 1 <host>
11351135
[dist] rustc <host>
1136-
[build] rustc 2 <host> -> std 2 <host>
1137-
[dist] rustc 2 <host> -> std 2 <host>
1136+
[dist] rustc 1 <host> -> std 1 <host>
11381137
[dist] rustc 1 <host> -> rustc-dev 2 <host>
11391138
[dist] src <>
11401139
[dist] reproducible-artifacts <host>
@@ -1198,8 +1197,7 @@ mod snapshot {
11981197
[build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <host>
11991198
[build] rustc 0 <host> -> GenerateCopyright 1 <host>
12001199
[dist] rustc <host>
1201-
[build] rustc 2 <host> -> std 2 <host>
1202-
[dist] rustc 2 <host> -> std 2 <host>
1200+
[dist] rustc 1 <host> -> std 1 <host>
12031201
[dist] rustc 1 <host> -> rustc-dev 2 <host>
12041202
[dist] rustc 1 <host> -> analysis 2 <host>
12051203
[dist] src <>
@@ -1216,7 +1214,6 @@ mod snapshot {
12161214
[build] rustc 1 <host> -> miri 2 <host>
12171215
[build] rustc 1 <host> -> cargo-miri 2 <host>
12181216
[dist] rustc 1 <host> -> miri 2 <host>
1219-
[dist] rustc 1 <host> -> std 1 <host>
12201217
[dist] rustc 1 <host> -> extended 2 <host>
12211218
[dist] reproducible-artifacts <host>
12221219
");
@@ -1287,8 +1284,7 @@ mod snapshot {
12871284
[dist] mingw <target1>
12881285
[build] rustc 0 <host> -> GenerateCopyright 1 <host>
12891286
[dist] rustc <host>
1290-
[build] rustc 2 <host> -> std 2 <host>
1291-
[dist] rustc 2 <host> -> std 2 <host>
1287+
[dist] rustc 1 <host> -> std 1 <host>
12921288
[build] rustc 2 <host> -> std 2 <target1>
12931289
[dist] rustc 2 <host> -> std 2 <target1>
12941290
[dist] rustc 1 <host> -> rustc-dev 2 <host>
@@ -1350,8 +1346,7 @@ mod snapshot {
13501346
[dist] rustc <host>
13511347
[build] rustdoc 2 <target1>
13521348
[dist] rustc <target1>
1353-
[build] rustc 2 <host> -> std 2 <host>
1354-
[dist] rustc 2 <host> -> std 2 <host>
1349+
[dist] rustc 1 <host> -> std 1 <host>
13551350
[dist] rustc 1 <host> -> rustc-dev 2 <host>
13561351
[dist] rustc 1 <host> -> rustc-dev 2 <target1>
13571352
[dist] src <>
@@ -1433,10 +1428,8 @@ mod snapshot {
14331428
[dist] rustc <host>
14341429
[build] rustdoc 2 <target1>
14351430
[dist] rustc <target1>
1436-
[build] rustc 2 <host> -> std 2 <host>
1437-
[dist] rustc 2 <host> -> std 2 <host>
1438-
[build] rustc 2 <host> -> std 2 <target1>
1439-
[dist] rustc 2 <host> -> std 2 <target1>
1431+
[dist] rustc 1 <host> -> std 1 <host>
1432+
[dist] rustc 1 <host> -> std 1 <target1>
14401433
[dist] rustc 1 <host> -> rustc-dev 2 <host>
14411434
[dist] rustc 1 <host> -> rustc-dev 2 <target1>
14421435
[dist] src <>
@@ -1490,10 +1483,8 @@ mod snapshot {
14901483
");
14911484
}
14921485

1493-
/// This also serves as an important regression test for <https://github.com/rust-lang/rust/issues/138123>
1494-
/// and <https://github.com/rust-lang/rust/issues/138004>.
14951486
#[test]
1496-
fn dist_all_cross() {
1487+
fn dist_all_cross_extended() {
14971488
let ctx = TestCtx::new();
14981489
insta::assert_snapshot!(
14991490
ctx
@@ -1545,8 +1536,7 @@ mod snapshot {
15451536
[build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <target1>
15461537
[build] rustc 0 <host> -> GenerateCopyright 1 <host>
15471538
[dist] rustc <target1>
1548-
[build] rustc 2 <host> -> std 2 <target1>
1549-
[dist] rustc 2 <host> -> std 2 <target1>
1539+
[dist] rustc 1 <host> -> std 1 <target1>
15501540
[dist] rustc 1 <host> -> rustc-dev 2 <target1>
15511541
[dist] rustc 1 <host> -> analysis 2 <target1>
15521542
[dist] src <>
@@ -1564,15 +1554,82 @@ mod snapshot {
15641554
[build] rustc 1 <host> -> cargo-miri 2 <target1>
15651555
[dist] rustc 1 <host> -> miri 2 <target1>
15661556
[build] rustc 1 <host> -> LlvmBitcodeLinker 2 <target1>
1567-
[dist] rustc 1 <host> -> std 1 <target1>
15681557
[doc] rustc 2 <target1> -> std 2 <target1> crates=[]
15691558
[dist] rustc 1 <host> -> extended 2 <target1>
15701559
[dist] reproducible-artifacts <target1>
15711560
");
15721561
}
15731562

1574-
// Enable dist cranelift tarball by default with `x dist` if cranelift is enabled in
1575-
// `rust.codegen-backends`.
1563+
/// Simulates e.g. the powerpc64 builder, which is fully cross-compiled from x64, but it does
1564+
/// not build docs. Crutically, it shouldn't build host stage 2 rustc.
1565+
///
1566+
/// This is a regression test for <https://github.com/rust-lang/rust/issues/138123>
1567+
/// and <https://github.com/rust-lang/rust/issues/138004>.
1568+
#[test]
1569+
fn dist_all_cross_extended_no_docs() {
1570+
let ctx = TestCtx::new();
1571+
let steps = ctx
1572+
.config("dist")
1573+
.hosts(&[TEST_TRIPLE_1])
1574+
.targets(&[TEST_TRIPLE_1])
1575+
.args(&[
1576+
"--set",
1577+
"rust.channel=nightly",
1578+
"--set",
1579+
"build.extended=true",
1580+
"--set",
1581+
"build.docs=false",
1582+
])
1583+
.get_steps();
1584+
1585+
// Make sure that we don't build stage2 host rustc
1586+
steps.assert_no_match(|m| {
1587+
m.name == "rustc"
1588+
&& m.built_by.map(|b| b.stage) == Some(1)
1589+
&& *m.target.triple == host_target()
1590+
});
1591+
1592+
insta::assert_snapshot!(
1593+
steps.render(), @r"
1594+
[dist] mingw <target1>
1595+
[build] llvm <host>
1596+
[build] llvm <target1>
1597+
[build] rustc 0 <host> -> rustc 1 <host>
1598+
[build] rustc 0 <host> -> WasmComponentLd 1 <host>
1599+
[build] rustc 1 <host> -> std 1 <target1>
1600+
[build] rustc 1 <host> -> std 1 <host>
1601+
[build] rustc 1 <host> -> rustc 2 <target1>
1602+
[build] rustc 1 <host> -> WasmComponentLd 2 <target1>
1603+
[build] rustdoc 2 <target1>
1604+
[build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <target1>
1605+
[build] rustc 0 <host> -> GenerateCopyright 1 <host>
1606+
[build] rustc 0 <host> -> RustInstaller 1 <host>
1607+
[dist] rustc <target1>
1608+
[dist] rustc 1 <host> -> std 1 <target1>
1609+
[dist] rustc 1 <host> -> rustc-dev 2 <target1>
1610+
[dist] rustc 1 <host> -> analysis 2 <target1>
1611+
[dist] src <>
1612+
[build] rustc 1 <host> -> cargo 2 <target1>
1613+
[dist] rustc 1 <host> -> cargo 2 <target1>
1614+
[build] rustc 1 <host> -> rust-analyzer 2 <target1>
1615+
[dist] rustc 1 <host> -> rust-analyzer 2 <target1>
1616+
[build] rustc 1 <host> -> rustfmt 2 <target1>
1617+
[build] rustc 1 <host> -> cargo-fmt 2 <target1>
1618+
[dist] rustc 1 <host> -> rustfmt 2 <target1>
1619+
[build] rustc 1 <host> -> clippy-driver 2 <target1>
1620+
[build] rustc 1 <host> -> cargo-clippy 2 <target1>
1621+
[dist] rustc 1 <host> -> clippy 2 <target1>
1622+
[build] rustc 1 <host> -> miri 2 <target1>
1623+
[build] rustc 1 <host> -> cargo-miri 2 <target1>
1624+
[dist] rustc 1 <host> -> miri 2 <target1>
1625+
[build] rustc 1 <host> -> LlvmBitcodeLinker 2 <target1>
1626+
[dist] rustc 1 <host> -> extended 2 <target1>
1627+
[dist] reproducible-artifacts <target1>
1628+
");
1629+
}
1630+
1631+
/// Enable dist cranelift tarball by default with `x dist` if cranelift is enabled in
1632+
/// `rust.codegen-backends`.
15761633
#[test]
15771634
fn dist_cranelift_by_default() {
15781635
let ctx = TestCtx::new();
@@ -1619,8 +1676,7 @@ mod snapshot {
16191676
[build] rustc 0 <host> -> GenerateCopyright 1 <host>
16201677
[dist] rustc <host>
16211678
[dist] rustc 1 <host> -> rustc_codegen_cranelift 2 <host>
1622-
[build] rustc 2 <host> -> std 2 <host>
1623-
[dist] rustc 2 <host> -> std 2 <host>
1679+
[dist] rustc 1 <host> -> std 1 <host>
16241680
[dist] rustc 1 <host> -> rustc-dev 2 <host>
16251681
[dist] src <>
16261682
[dist] reproducible-artifacts <host>
@@ -2351,8 +2407,7 @@ mod snapshot {
23512407
[doc] rustc 1 <host> -> releases 2 <host>
23522408
[build] rustc 0 <host> -> RustInstaller 1 <host>
23532409
[dist] docs <host>
2354-
[build] rustc 2 <host> -> std 2 <host>
2355-
[dist] rustc 2 <host> -> std 2 <host>
2410+
[dist] rustc 1 <host> -> std 1 <host>
23562411
[build] rustc 1 <host> -> rust-analyzer-proc-macro-srv 2 <host>
23572412
[build] rustc 0 <host> -> GenerateCopyright 1 <host>
23582413
[dist] rustc <host>
@@ -2426,6 +2481,21 @@ impl ExecutedSteps {
24262481
}
24272482
}
24282483

2484+
/// Make sure that no metadata matches the given `func`.
2485+
#[track_caller]
2486+
fn assert_no_match<F>(&self, func: F)
2487+
where
2488+
F: Fn(StepMetadata) -> bool,
2489+
{
2490+
for metadata in self.steps.iter().filter_map(|s| s.metadata.clone()) {
2491+
if func(metadata.clone()) {
2492+
panic!(
2493+
"Metadata {metadata:?} was found, even though it should have not been present"
2494+
);
2495+
}
2496+
}
2497+
}
2498+
24292499
fn contains(&self, metadata: &StepMetadata) -> bool {
24302500
self.steps
24312501
.iter()

0 commit comments

Comments
 (0)