From 6cfa7ef2ba1a4781605697c78901c9c10353c58f Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 19 Sep 2022 19:08:02 +0000 Subject: [PATCH] Remove miri from the submodule list and require it for CI to pass --- src/README.md | 2 +- src/bootstrap/check.rs | 2 +- src/bootstrap/dist.rs | 83 +++----- src/bootstrap/doc.rs | 2 +- src/bootstrap/install.rs | 11 +- src/bootstrap/lib.rs | 9 +- src/bootstrap/test.rs | 195 ++++++++---------- src/bootstrap/tool.rs | 4 +- .../x86_64-gnu-tools/checktools.sh | 2 +- src/tools/build-manifest/src/main.rs | 24 --- src/tools/publish_toolstate.py | 17 +- triagebot.toml | 2 +- 12 files changed, 136 insertions(+), 217 deletions(-) diff --git a/src/README.md b/src/README.md index 3d2e6acd576..90ab8026970 100644 --- a/src/README.md +++ b/src/README.md @@ -2,7 +2,7 @@ This directory contains the source code of the rust project, including: - The test suite - The bootstrapping build system -- Various submodules for tools, like cargo, miri, etc. +- Various submodules for tools, like cargo, etc. For more information on how various parts of the compiler work, see the [rustc dev guide]. diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 5c085bedf0e..9937b08561c 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -456,7 +456,7 @@ fn stamp( // behavior, treat it as in-tree so that any new warnings in clippy will be // rejected. tool_check_step!(Clippy, "src/tools/clippy", SourceType::InTree); -tool_check_step!(Miri, "src/tools/miri", SourceType::Submodule); +tool_check_step!(Miri, "src/tools/miri", SourceType::InTree); tool_check_step!(Rls, "src/tools/rls", SourceType::InTree); tool_check_step!(Rustfmt, "src/tools/rustfmt", SourceType::InTree); diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index c9ee3c1c7d6..05664ca2179 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -35,18 +35,6 @@ pub fn tmpdir(builder: &Builder<'_>) -> PathBuf { builder.out.join("tmp/dist") } -fn missing_tool(tool_name: &str, skip: bool) { - if skip { - println!("Unable to build {}, skipping dist", tool_name) - } else { - let help = "note: not all tools are available on all nightlies\nhelp: see https://forge.rust-lang.org/infra/toolstate.html for more information"; - panic!( - "Unable to build submodule tool {} (use `missing-tools = true` to ignore this failure)\n{}", - tool_name, help - ) - } -} - fn should_build_extended_tool(builder: &Builder<'_>, tool: &str) -> bool { if !builder.config.extended { return false; @@ -1209,18 +1197,9 @@ fn run(self, builder: &Builder<'_>) -> Option { let compiler = self.compiler; let target = self.target; - let miri = builder - .ensure(tool::Miri { compiler, target, extra_features: Vec::new() }) - .or_else(|| { - missing_tool("miri", builder.build.config.missing_tools); - None - })?; - let cargomiri = builder - .ensure(tool::CargoMiri { compiler, target, extra_features: Vec::new() }) - .or_else(|| { - missing_tool("cargo miri", builder.build.config.missing_tools); - None - })?; + let miri = builder.ensure(tool::Miri { compiler, target, extra_features: Vec::new() })?; + let cargomiri = + builder.ensure(tool::CargoMiri { compiler, target, extra_features: Vec::new() })?; let mut tarball = Tarball::new(builder, "miri", &target.triple); tarball.set_overlay(OverlayKind::Miri); @@ -1451,7 +1430,7 @@ fn filter(contents: &str, marker: &str) -> String { let xform = |p: &Path| { let mut contents = t!(fs::read_to_string(p)); - for tool in &["rust-demangler", "rust-analyzer", "miri", "rustfmt"] { + for tool in &["rust-demangler", "rust-analyzer", "rustfmt"] { if !built_tools.contains(tool) { contents = filter(&contents, tool); } @@ -1491,7 +1470,8 @@ fn filter(contents: &str, marker: &str) -> String { prepare("rust-std"); prepare("rust-analysis"); prepare("clippy"); - for tool in &["rust-docs", "rust-demangler", "rust-analyzer", "miri"] { + prepare("miri"); + for tool in &["rust-docs", "rust-demangler", "rust-analyzer"] { if built_tools.contains(tool) { prepare(tool); } @@ -1550,7 +1530,8 @@ fn filter(contents: &str, marker: &str) -> String { prepare("rust-docs"); prepare("rust-std"); prepare("clippy"); - for tool in &["rust-demangler", "rust-analyzer", "miri"] { + prepare("miri"); + for tool in &["rust-demangler", "rust-analyzer"] { if built_tools.contains(tool) { prepare(tool); } @@ -1689,25 +1670,23 @@ fn filter(contents: &str, marker: &str) -> String { .arg(etc.join("msi/remove-duplicates.xsl")), ); } - if built_tools.contains("miri") { - builder.run( - Command::new(&heat) - .current_dir(&exe) - .arg("dir") - .arg("miri") - .args(&heat_flags) - .arg("-cg") - .arg("MiriGroup") - .arg("-dr") - .arg("Miri") - .arg("-var") - .arg("var.MiriDir") - .arg("-out") - .arg(exe.join("MiriGroup.wxs")) - .arg("-t") - .arg(etc.join("msi/remove-duplicates.xsl")), - ); - } + builder.run( + Command::new(&heat) + .current_dir(&exe) + .arg("dir") + .arg("miri") + .args(&heat_flags) + .arg("-cg") + .arg("MiriGroup") + .arg("-dr") + .arg("Miri") + .arg("-var") + .arg("var.MiriDir") + .arg("-out") + .arg(exe.join("MiriGroup.wxs")) + .arg("-t") + .arg(etc.join("msi/remove-duplicates.xsl")), + ); builder.run( Command::new(&heat) .current_dir(&exe) @@ -1755,6 +1734,7 @@ fn filter(contents: &str, marker: &str) -> String { .arg("-dStdDir=rust-std") .arg("-dAnalysisDir=rust-analysis") .arg("-dClippyDir=clippy") + .arg("-dMiriDir=miri") .arg("-arch") .arg(&arch) .arg("-out") @@ -1768,9 +1748,6 @@ fn filter(contents: &str, marker: &str) -> String { if built_tools.contains("rust-analyzer") { cmd.arg("-dRustAnalyzerDir=rust-analyzer"); } - if built_tools.contains("miri") { - cmd.arg("-dMiriDir=miri"); - } if target.ends_with("windows-gnu") { cmd.arg("-dGccDir=rust-mingw"); } @@ -1784,15 +1761,13 @@ fn filter(contents: &str, marker: &str) -> String { candle("CargoGroup.wxs".as_ref()); candle("StdGroup.wxs".as_ref()); candle("ClippyGroup.wxs".as_ref()); + candle("MiriGroup.wxs".as_ref()); if built_tools.contains("rust-demangler") { candle("RustDemanglerGroup.wxs".as_ref()); } if built_tools.contains("rust-analyzer") { candle("RustAnalyzerGroup.wxs".as_ref()); } - if built_tools.contains("miri") { - candle("MiriGroup.wxs".as_ref()); - } candle("AnalysisGroup.wxs".as_ref()); if target.ends_with("windows-gnu") { @@ -1822,6 +1797,7 @@ fn filter(contents: &str, marker: &str) -> String { .arg("StdGroup.wixobj") .arg("AnalysisGroup.wixobj") .arg("ClippyGroup.wixobj") + .arg("MiriGroup.wixobj") .current_dir(&exe); if built_tools.contains("rust-analyzer") { @@ -1830,9 +1806,6 @@ fn filter(contents: &str, marker: &str) -> String { if built_tools.contains("rust-demangler") { cmd.arg("RustDemanglerGroup.wixobj"); } - if built_tools.contains("miri") { - cmd.arg("MiriGroup.wixobj"); - } if target.ends_with("windows-gnu") { cmd.arg("GccGroup.wixobj"); diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index 819af658748..5d021b8ab16 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -851,7 +851,7 @@ fn run(self, builder: &Builder<'_>) { in_tree = true ); tool_doc!(Clippy, "clippy", "src/tools/clippy", ["clippy_utils"], in_tree = true); -tool_doc!(Miri, "miri", "src/tools/miri", ["miri"], in_tree = false); +tool_doc!(Miri, "miri", "src/tools/miri", ["miri"], in_tree = true); #[derive(Ord, PartialOrd, Debug, Copy, Clone, Hash, PartialEq, Eq)] pub struct ErrorIndex { diff --git a/src/bootstrap/install.rs b/src/bootstrap/install.rs index d34aa15c515..7672b7c9135 100644 --- a/src/bootstrap/install.rs +++ b/src/bootstrap/install.rs @@ -200,13 +200,10 @@ fn run($sel, $builder: &Builder<'_>) { install_sh(builder, "clippy", self.compiler.stage, Some(self.target), &tarball); }; Miri, alias = "miri", Self::should_build(_config), only_hosts: true, { - if let Some(tarball) = builder.ensure(dist::Miri { compiler: self.compiler, target: self.target }) { - install_sh(builder, "miri", self.compiler.stage, Some(self.target), &tarball); - } else { - builder.info( - &format!("skipping Install miri stage{} ({})", self.compiler.stage, self.target), - ); - } + let tarball = builder + .ensure(dist::Miri { compiler: self.compiler, target: self.target }) + .expect("missing miri"); + install_sh(builder, "miri", self.compiler.stage, Some(self.target), &tarball); }; Rustfmt, alias = "rustfmt", Self::should_build(_config), only_hosts: true, { if let Some(tarball) = builder.ensure(dist::Rustfmt { diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index b7d271e6494..2b8c2a2c134 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -540,13 +540,8 @@ pub fn new(config: Config) -> Build { // Make sure we update these before gathering metadata so we don't get an error about missing // Cargo.toml files. - let rust_submodules = [ - "src/tools/rust-installer", - "src/tools/cargo", - "src/tools/miri", - "library/backtrace", - "library/stdarch", - ]; + let rust_submodules = + ["src/tools/rust-installer", "src/tools/cargo", "library/backtrace", "library/stdarch"]; for s in rust_submodules { build.update_submodule(Path::new(s)); } diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 9d286ddd6d1..1617875ec23 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -484,116 +484,103 @@ fn run(self, builder: &Builder<'_>) { // Except if we are at stage 2, the bootstrap loop is complete and we can stick with our current stage. let compiler_std = builder.compiler(if stage < 2 { stage + 1 } else { stage }, host); - let miri = - builder.ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() }); - let cargo_miri = builder.ensure(tool::CargoMiri { - compiler, - target: self.host, - extra_features: Vec::new(), - }); + let miri = builder + .ensure(tool::Miri { compiler, target: self.host, extra_features: Vec::new() }) + .expect("in-tree tool"); + let _cargo_miri = builder + .ensure(tool::CargoMiri { compiler, target: self.host, extra_features: Vec::new() }) + .expect("in-tree tool"); // The stdlib we need might be at a different stage. And just asking for the // sysroot does not seem to populate it, so we do that first. builder.ensure(compile::Std::new(compiler_std, host)); let sysroot = builder.sysroot(compiler_std); - if let (Some(miri), Some(_cargo_miri)) = (miri, cargo_miri) { - let mut cargo = - builder.cargo(compiler, Mode::ToolRustc, SourceType::Submodule, host, "install"); - cargo.arg("xargo"); - // Configure `cargo install` path. cargo adds a `bin/`. - cargo.env("CARGO_INSTALL_ROOT", &builder.out); + let mut cargo = + builder.cargo(compiler, Mode::ToolRustc, SourceType::Submodule, host, "install"); + cargo.arg("xargo"); + // Configure `cargo install` path. cargo adds a `bin/`. + cargo.env("CARGO_INSTALL_ROOT", &builder.out); - let mut cargo = Command::from(cargo); - if !try_run(builder, &mut cargo) { - return; - } - - // # Run `cargo miri setup`. - let mut cargo = tool::prepare_tool_cargo( - builder, - compiler, - Mode::ToolRustc, - host, - "run", - "src/tools/miri/cargo-miri", - SourceType::Submodule, - &[], - ); - cargo.add_rustc_lib_path(builder, compiler); - cargo.arg("--").arg("miri").arg("setup"); - - // Tell `cargo miri setup` where to find the sources. - cargo.env("XARGO_RUST_SRC", builder.src.join("library")); - // Tell it where to find Miri. - cargo.env("MIRI", &miri); - // Debug things. - cargo.env("RUST_BACKTRACE", "1"); - // Let cargo-miri know where xargo ended up. - cargo.env("XARGO_CHECK", builder.out.join("bin").join("xargo-check")); - - let mut cargo = Command::from(cargo); - if !try_run(builder, &mut cargo) { - return; - } - - // # Determine where Miri put its sysroot. - // To this end, we run `cargo miri setup --print-sysroot` and capture the output. - // (We do this separately from the above so that when the setup actually - // happens we get some output.) - // We re-use the `cargo` from above. - cargo.arg("--print-sysroot"); - - // FIXME: Is there a way in which we can re-use the usual `run` helpers? - let miri_sysroot = if builder.config.dry_run { - String::new() - } else { - builder.verbose(&format!("running: {:?}", cargo)); - let out = cargo - .output() - .expect("We already ran `cargo miri setup` before and that worked"); - assert!(out.status.success(), "`cargo miri setup` returned with non-0 exit code"); - // Output is "\n". - let stdout = String::from_utf8(out.stdout) - .expect("`cargo miri setup` stdout is not valid UTF-8"); - let sysroot = stdout.trim_end(); - builder.verbose(&format!("`cargo miri setup --print-sysroot` said: {:?}", sysroot)); - sysroot.to_owned() - }; - - // # Run `cargo test`. - let mut cargo = tool::prepare_tool_cargo( - builder, - compiler, - Mode::ToolRustc, - host, - "test", - "src/tools/miri", - SourceType::Submodule, - &[], - ); - cargo.add_rustc_lib_path(builder, compiler); - - // miri tests need to know about the stage sysroot - cargo.env("MIRI_SYSROOT", miri_sysroot); - cargo.env("MIRI_HOST_SYSROOT", sysroot); - cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler)); - cargo.env("MIRI", miri); - // propagate --bless - if builder.config.cmd.bless() { - cargo.env("MIRI_BLESS", "Gesundheit"); - } - - cargo.arg("--").args(builder.config.cmd.test_args()); - - let mut cargo = Command::from(cargo); - if !try_run(builder, &mut cargo) { - return; - } - - // # Done! - builder.save_toolstate("miri", ToolState::TestPass); - } else { - eprintln!("failed to test miri: could not build"); + let mut cargo = Command::from(cargo); + if !try_run(builder, &mut cargo) { + return; } + + // # Run `cargo miri setup`. + let mut cargo = tool::prepare_tool_cargo( + builder, + compiler, + Mode::ToolRustc, + host, + "run", + "src/tools/miri/cargo-miri", + SourceType::Submodule, + &[], + ); + cargo.add_rustc_lib_path(builder, compiler); + cargo.arg("--").arg("miri").arg("setup"); + + // Tell `cargo miri setup` where to find the sources. + cargo.env("XARGO_RUST_SRC", builder.src.join("library")); + // Tell it where to find Miri. + cargo.env("MIRI", &miri); + // Debug things. + cargo.env("RUST_BACKTRACE", "1"); + // Let cargo-miri know where xargo ended up. + cargo.env("XARGO_CHECK", builder.out.join("bin").join("xargo-check")); + + let mut cargo = Command::from(cargo); + builder.run(&mut cargo); + + // # Determine where Miri put its sysroot. + // To this end, we run `cargo miri setup --print-sysroot` and capture the output. + // (We do this separately from the above so that when the setup actually + // happens we get some output.) + // We re-use the `cargo` from above. + cargo.arg("--print-sysroot"); + + // FIXME: Is there a way in which we can re-use the usual `run` helpers? + let miri_sysroot = if builder.config.dry_run { + String::new() + } else { + builder.verbose(&format!("running: {:?}", cargo)); + let out = + cargo.output().expect("We already ran `cargo miri setup` before and that worked"); + assert!(out.status.success(), "`cargo miri setup` returned with non-0 exit code"); + // Output is "\n". + let stdout = String::from_utf8(out.stdout) + .expect("`cargo miri setup` stdout is not valid UTF-8"); + let sysroot = stdout.trim_end(); + builder.verbose(&format!("`cargo miri setup --print-sysroot` said: {:?}", sysroot)); + sysroot.to_owned() + }; + + // # Run `cargo test`. + let mut cargo = tool::prepare_tool_cargo( + builder, + compiler, + Mode::ToolRustc, + host, + "test", + "src/tools/miri", + SourceType::Submodule, + &[], + ); + cargo.add_rustc_lib_path(builder, compiler); + + // miri tests need to know about the stage sysroot + cargo.env("MIRI_SYSROOT", miri_sysroot); + cargo.env("MIRI_HOST_SYSROOT", sysroot); + cargo.env("RUSTC_LIB_PATH", builder.rustc_libdir(compiler)); + cargo.env("MIRI", miri); + // propagate --bless + if builder.config.cmd.bless() { + cargo.env("MIRI_BLESS", "Gesundheit"); + } + + cargo.arg("--").args(builder.config.cmd.test_args()); + + let mut cargo = Command::from(cargo); + builder.run(&mut cargo); } } diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 5d0c7d2bd9d..ff6f7909a5a 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -868,8 +868,8 @@ fn run(mut $sel, $builder: &Builder<'_>) -> Option { Cargofmt, "src/tools/rustfmt", "cargo-fmt", stable=true, in_tree=true, {}; CargoClippy, "src/tools/clippy", "cargo-clippy", stable=true, in_tree=true, {}; Clippy, "src/tools/clippy", "clippy-driver", stable=true, in_tree=true, {}; - Miri, "src/tools/miri", "miri", stable=false, {}; - CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=false, {}; + Miri, "src/tools/miri", "miri", stable=false, in_tree=true, {}; + CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=false, in_tree=true, {}; Rls, "src/tools/rls", "rls", stable=true, {}; // FIXME: tool_std is not quite right, we shouldn't allow nightly features. // But `builder.cargo` doesn't know how to handle ToolBootstrap in stages other than 0, diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh b/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh index 0fb8f41a7ec..cf00c285b0a 100755 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh @@ -14,7 +14,6 @@ python3 "$X_PY" test --stage 2 --no-fail-fast \ src/doc/rust-by-example \ src/doc/embedded-book \ src/doc/edition-guide \ - src/tools/miri \ set -e @@ -23,3 +22,4 @@ cat /tmp/toolstate/toolstates.json python3 "$X_PY" test --stage 2 check-tools python3 "$X_PY" test --stage 2 src/tools/clippy python3 "$X_PY" test --stage 2 src/tools/rustfmt +python3 "$X_PY" test --stage 2 src/tools/miri diff --git a/src/tools/build-manifest/src/main.rs b/src/tools/build-manifest/src/main.rs index 1a6760d8c68..5fec2c39621 100644 --- a/src/tools/build-manifest/src/main.rs +++ b/src/tools/build-manifest/src/main.rs @@ -239,7 +239,6 @@ fn main() { impl Builder { fn build(&mut self) { - self.check_toolstate(); let manifest = self.build_manifest(); let channel = self.versions.channel().to_string(); @@ -261,29 +260,6 @@ fn build(&mut self) { t!(self.checksums.store_cache()); } - /// If a tool does not pass its tests on *any* of Linux and Windows, don't ship - /// it on *all* targets, because tools like Miri can "cross-run" programs for - /// different targets, for example, run a program for `x86_64-pc-windows-msvc` - /// on `x86_64-unknown-linux-gnu`. - /// Right now, we do this only for Miri. - fn check_toolstate(&mut self) { - for file in &["toolstates-linux.json", "toolstates-windows.json"] { - let toolstates: Option> = File::open(self.input.join(file)) - .ok() - .and_then(|f| serde_json::from_reader(&f).ok()); - let toolstates = toolstates.unwrap_or_else(|| { - println!("WARNING: `{}` missing/malformed; assuming all tools failed", file); - HashMap::default() // Use empty map if anything went wrong. - }); - // Mark some tools as missing based on toolstate. - if toolstates.get("miri").map(|s| &*s as &str) != Some("test-pass") { - println!("Miri tests are not passing, removing component"); - self.versions.disable_version(&PkgType::Miri); - break; - } - } - } - fn build_manifest(&mut self) -> Manifest { let mut manifest = Manifest { manifest_version: "2".to_string(), diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index c0cef8f7b0a..9c16ef2cbec 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -30,7 +30,6 @@ except ImportError: # These should be collaborators of the rust-lang/rust repository (with at least # read privileges on it). CI will fail otherwise. MAINTAINERS = { - 'miri': {'oli-obk', 'RalfJung'}, 'book': {'carols10cents'}, 'nomicon': {'frewsxcv', 'Gankra', 'JohnTitor'}, 'reference': {'Havvy', 'matthewjasper', 'ehuss'}, @@ -41,7 +40,6 @@ MAINTAINERS = { } LABELS = { - 'miri': ['A-miri', 'C-bug'], 'book': ['C-bug'], 'nomicon': ['C-bug'], 'reference': ['C-bug'], @@ -52,7 +50,6 @@ LABELS = { } REPOS = { - 'miri': 'https://github.com/rust-lang/miri', 'book': 'https://github.com/rust-lang/book', 'nomicon': 'https://github.com/rust-lang/nomicon', 'reference': 'https://github.com/rust-lang/reference', @@ -239,16 +236,10 @@ def update_latest( message += '{} (cc {}).\n' \ .format(title, maintainers) # See if we need to create an issue. - if tool == 'miri': - # Create issue if tests used to pass before. Don't open a *second* - # issue when we regress from "test-fail" to "build-fail". - if old == 'test-pass': - create_issue_for_status = new - else: - # Create issue if things no longer build. - # (No issue for mere test failures to avoid spurious issues.) - if new == 'build-fail': - create_issue_for_status = new + # Create issue if things no longer build. + # (No issue for mere test failures to avoid spurious issues.) + if new == 'build-fail': + create_issue_for_status = new if create_issue_for_status is not None: try: diff --git a/triagebot.toml b/triagebot.toml index 12a55fda7ef..d358e59c245 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -356,7 +356,7 @@ cc = ["@ehuss"] cc = ["@rust-lang/clippy"] [mentions."src/tools/miri"] -message = "The Miri submodule was changed" +message = "The Miri subtree was changed" cc = ["@rust-lang/miri"] [mentions."src/tools/rustfmt"]