Skip to content

Commit a347f8d

Browse files
committed
Fix staging of TestFloatParse
The tool wasn't useful for anything, it was only built as a part of the test, but we can just use `cargo test` and `cargo run` in the test, no need to (pre-)build the tool itself.
1 parent f0d15d3 commit a347f8d

File tree

3 files changed

+61
-74
lines changed

3 files changed

+61
-74
lines changed

src/bootstrap/src/core/build_steps/check.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use crate::core::build_steps::compile::{
88
};
99
use crate::core::build_steps::tool;
1010
use crate::core::build_steps::tool::{
11-
COMPILETEST_ALLOW_FEATURES, SourceType, ToolTargetBuildMode, get_tool_target_compiler,
12-
prepare_tool_cargo,
11+
COMPILETEST_ALLOW_FEATURES, SourceType, TEST_FLOAT_PARSE_ALLOW_FEATURES, ToolTargetBuildMode,
12+
get_tool_target_compiler, prepare_tool_cargo,
1313
};
1414
use crate::core::builder::{
1515
self, Alias, Builder, Cargo, Kind, RunConfig, ShouldRun, Step, StepMetadata, crate_description,
@@ -791,7 +791,7 @@ tool_check_step!(MiroptTestTools {
791791
tool_check_step!(TestFloatParse {
792792
path: "src/tools/test-float-parse",
793793
mode: |_builder| Mode::ToolStd,
794-
allow_features: tool::TestFloatParse::ALLOW_FEATURES
794+
allow_features: TEST_FLOAT_PARSE_ALLOW_FEATURES
795795
});
796796
tool_check_step!(FeaturesStatusDump {
797797
path: "src/tools/features-status-dump",

src/bootstrap/src/core/build_steps/test.rs

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ use crate::core::build_steps::llvm::get_llvm_version;
1818
use crate::core::build_steps::run::get_completion_paths;
1919
use crate::core::build_steps::synthetic_targets::MirOptPanicAbortSyntheticTarget;
2020
use crate::core::build_steps::tool::{
21-
self, COMPILETEST_ALLOW_FEATURES, RustcPrivateCompilers, SourceType, Tool, ToolTargetBuildMode,
22-
get_tool_target_compiler,
21+
self, COMPILETEST_ALLOW_FEATURES, RustcPrivateCompilers, SourceType,
22+
TEST_FLOAT_PARSE_ALLOW_FEATURES, Tool, ToolTargetBuildMode, get_tool_target_compiler,
2323
};
2424
use crate::core::build_steps::toolstate::ToolState;
2525
use crate::core::build_steps::{compile, dist, llvm};
@@ -2886,6 +2886,7 @@ impl Step for Crate {
28862886
}
28872887
}
28882888

2889+
/// Run cargo tests for the rustdoc crate.
28892890
/// Rustdoc is special in various ways, which is why this step is different from `Crate`.
28902891
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
28912892
pub struct CrateRustdoc {
@@ -2981,7 +2982,8 @@ impl Step for CrateRustdoc {
29812982

29822983
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
29832984
pub struct CrateRustdocJsonTypes {
2984-
host: TargetSelection,
2985+
build_compiler: Compiler,
2986+
target: TargetSelection,
29852987
}
29862988

29872989
impl Step for CrateRustdocJsonTypes {
@@ -2996,23 +2998,22 @@ impl Step for CrateRustdocJsonTypes {
29962998
fn make_run(run: RunConfig<'_>) {
29972999
let builder = run.builder;
29983000

2999-
builder.ensure(CrateRustdocJsonTypes { host: run.target });
3001+
builder.ensure(CrateRustdocJsonTypes {
3002+
build_compiler: get_tool_target_compiler(
3003+
builder,
3004+
ToolTargetBuildMode::Build(run.target),
3005+
),
3006+
target: run.target,
3007+
});
30003008
}
30013009

30023010
fn run(self, builder: &Builder<'_>) {
3003-
let target = self.host;
3004-
3005-
// Use the previous stage compiler to reuse the artifacts that are
3006-
// created when running compiletest for tests/rustdoc. If this used
3007-
// `compiler`, then it would cause rustdoc to be built *again*, which
3008-
// isn't really necessary.
3009-
let compiler = builder.compiler_for(builder.top_stage, target, target);
3010-
builder.ensure(compile::Rustc::new(compiler, target));
3011+
let target = self.target;
30113012

30123013
let cargo = tool::prepare_tool_cargo(
30133014
builder,
3014-
compiler,
3015-
Mode::ToolRustc,
3015+
self.build_compiler,
3016+
Mode::ToolTarget,
30163017
target,
30173018
builder.kind,
30183019
"src/rustdoc-json-types",
@@ -3021,7 +3022,7 @@ impl Step for CrateRustdocJsonTypes {
30213022
);
30223023

30233024
// FIXME: this looks very wrong, libtest doesn't accept `-C` arguments and the quotes are fishy.
3024-
let libtest_args = if self.host.contains("musl") {
3025+
let libtest_args = if target.contains("musl") {
30253026
["'-Ctarget-feature=-crt-static'"].as_slice()
30263027
} else {
30273028
&[]
@@ -3705,14 +3706,35 @@ impl Step for CodegenGCC {
37053706
}
37063707
}
37073708

3709+
/// Get a build compiler that can be used to test the standard library (i.e. its stage will
3710+
/// correspond to the stage that we want to test).
3711+
fn get_test_build_compiler_for_std(builder: &Builder<'_>) -> Compiler {
3712+
if builder.top_stage == 0 {
3713+
eprintln!(
3714+
"ERROR: cannot run tests on stage 0. `build.compiletest-allow-stage0` only works for compiletest test suites."
3715+
);
3716+
exit!(1);
3717+
}
3718+
builder.compiler(builder.top_stage, builder.host_target)
3719+
}
3720+
37083721
/// Test step that does two things:
37093722
/// - Runs `cargo test` for the `src/tools/test-float-parse` tool.
37103723
/// - Invokes the `test-float-parse` tool to test the standard library's
37113724
/// float parsing routines.
37123725
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
37133726
pub struct TestFloatParse {
3714-
path: PathBuf,
3715-
host: TargetSelection,
3727+
/// The build compiler which will build and run unit tests of `test-float-parse`, and which will
3728+
/// build the `test-float-parse` tool itself.
3729+
///
3730+
/// Note that the staging is a bit funny here, because this step essentially tests std, but it
3731+
/// also needs to build the tool. So if we test stage1 std, we build:
3732+
/// 1) stage1 rustc
3733+
/// 2) Use that to build stage1 libstd
3734+
/// 3) Use that to build and run *stage2* test-float-parse
3735+
build_compiler: Compiler,
3736+
/// Target for which we build std and test that std.
3737+
target: TargetSelection,
37163738
}
37173739

37183740
impl Step for TestFloatParse {
@@ -3725,47 +3747,47 @@ impl Step for TestFloatParse {
37253747
}
37263748

37273749
fn make_run(run: RunConfig<'_>) {
3728-
for path in run.paths {
3729-
let path = path.assert_single_path().path.clone();
3730-
run.builder.ensure(Self { path, host: run.target });
3731-
}
3750+
run.builder.ensure(Self {
3751+
build_compiler: get_test_build_compiler_for_std(run.builder),
3752+
target: run.target,
3753+
});
37323754
}
37333755

37343756
fn run(self, builder: &Builder<'_>) {
3735-
let bootstrap_host = builder.config.host_target;
3736-
let compiler = builder.compiler(builder.top_stage, bootstrap_host);
3737-
let path = self.path.to_str().unwrap();
3738-
let crate_name = self.path.iter().next_back().unwrap().to_str().unwrap();
3757+
let build_compiler = self.build_compiler;
3758+
let target = self.target;
37393759

3740-
builder.ensure(tool::TestFloatParse { host: self.host });
3760+
// Build the standard library that will be tested, and a stdlib for host code
3761+
builder.std(build_compiler, target);
3762+
builder.std(build_compiler, builder.host_target);
37413763

37423764
// Run any unit tests in the crate
37433765
let mut cargo_test = tool::prepare_tool_cargo(
37443766
builder,
3745-
compiler,
3767+
build_compiler,
37463768
Mode::ToolStd,
3747-
bootstrap_host,
3769+
target,
37483770
Kind::Test,
3749-
path,
3771+
"src/tools/test-float-parse",
37503772
SourceType::InTree,
37513773
&[],
37523774
);
3753-
cargo_test.allow_features(tool::TestFloatParse::ALLOW_FEATURES);
3775+
cargo_test.allow_features(TEST_FLOAT_PARSE_ALLOW_FEATURES);
37543776

3755-
run_cargo_test(cargo_test, &[], &[], crate_name, bootstrap_host, builder);
3777+
run_cargo_test(cargo_test, &[], &[], "test-float-parse", target, builder);
37563778

37573779
// Run the actual parse tests.
37583780
let mut cargo_run = tool::prepare_tool_cargo(
37593781
builder,
3760-
compiler,
3782+
build_compiler,
37613783
Mode::ToolStd,
3762-
bootstrap_host,
3784+
target,
37633785
Kind::Run,
3764-
path,
3786+
"src/tools/test-float-parse",
37653787
SourceType::InTree,
37663788
&[],
37673789
);
3768-
cargo_run.allow_features(tool::TestFloatParse::ALLOW_FEATURES);
3790+
cargo_run.allow_features(TEST_FLOAT_PARSE_ALLOW_FEATURES);
37693791

37703792
if !matches!(env::var("FLOAT_PARSE_TESTS_NO_SKIP_HUGE").as_deref(), Ok("1") | Ok("true")) {
37713793
cargo_run.args(["--", "--skip-huge"]);

src/bootstrap/src/core/build_steps/tool.rs

Lines changed: 1 addition & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,42 +1539,7 @@ tool_rustc_extended!(Rustfmt {
15391539
add_bins_to_sysroot: ["rustfmt"]
15401540
});
15411541

1542-
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
1543-
pub struct TestFloatParse {
1544-
pub host: TargetSelection,
1545-
}
1546-
1547-
impl TestFloatParse {
1548-
pub const ALLOW_FEATURES: &'static str = "f16,cfg_target_has_reliable_f16_f128";
1549-
}
1550-
1551-
impl Step for TestFloatParse {
1552-
type Output = ToolBuildResult;
1553-
const IS_HOST: bool = true;
1554-
const DEFAULT: bool = false;
1555-
1556-
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
1557-
run.path("src/tools/test-float-parse")
1558-
}
1559-
1560-
fn run(self, builder: &Builder<'_>) -> ToolBuildResult {
1561-
let bootstrap_host = builder.config.host_target;
1562-
let compiler = builder.compiler(builder.top_stage, bootstrap_host);
1563-
1564-
builder.ensure(ToolBuild {
1565-
build_compiler: compiler,
1566-
target: bootstrap_host,
1567-
tool: "test-float-parse",
1568-
mode: Mode::ToolStd,
1569-
path: "src/tools/test-float-parse",
1570-
source_type: SourceType::InTree,
1571-
extra_features: Vec::new(),
1572-
allow_features: Self::ALLOW_FEATURES,
1573-
cargo_args: Vec::new(),
1574-
artifact_kind: ToolArtifactKind::Binary,
1575-
})
1576-
}
1577-
}
1542+
pub const TEST_FLOAT_PARSE_ALLOW_FEATURES: &'static str = "f16,cfg_target_has_reliable_f16_f128";
15781543

15791544
impl Builder<'_> {
15801545
/// Gets a `BootstrapCommand` which is ready to run `tool` in `stage` built for

0 commit comments

Comments
 (0)