mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #3006 - RalfJung:miri-script-shell, r=RalfJung

Browse Source 
miri-script and cargo-miri cleanups

At least in an initial test, `cmd!(Shell::new()?, "env")` does show the full environment. Let's see what CI says.

`@osiewicz` why did you add this code to forward the host environment?
This commit is contained in:
bors 2023-08-02 18:51:05 +00:00
commit 042cfd871f
5 changed files with 41 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/tools/miri/cargo-miri/src/phases.rs
View File

@ -94,7 +94,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
let target = target.as_ref().unwrap_or(host);
// We always setup.
setup(&subcommand, target, &rustc_version, verbose);
let miri_sysroot = setup(&subcommand, target, &rustc_version, verbose);
// Invoke actual cargo for the job, but with different flags.
// We re-use `cargo test` and `cargo run`, which makes target and binary handling very easy but
@ -159,6 +159,8 @@ pub fn phase_cargo_miri(mut args: impl Iterator<Item = String>) {
// Forward all further arguments (not consumed by `ArgSplitFlagValue`) to cargo.
cmd.args(args);
// Let it know where the Miri sysroot lives.
cmd.env("MIRI_SYSROOT", miri_sysroot);
// Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation,
// i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish
// the two codepaths. (That extra argument is why we prefer this over setting `RUSTC`.)

21
src/tools/miri/cargo-miri/src/setup.rs
View File

@ -13,13 +13,20 @@ use crate::util::*;
/// Performs the setup required to make `cargo miri` work: Getting a custom-built libstd. Then sets
/// `MIRI_SYSROOT`. Skipped if `MIRI_SYSROOT` is already set, in which case we expect the user has
/// done all this already.
pub fn setup(subcommand: &MiriCommand, target: &str, rustc_version: &VersionMeta, verbose: usize) {
pub fn setup(
subcommand: &MiriCommand,
target: &str,
rustc_version: &VersionMeta,
verbose: usize,
) -> PathBuf {
let only_setup = matches!(subcommand, MiriCommand::Setup);
let ask_user = !only_setup;
let print_sysroot = only_setup && has_arg_flag("--print-sysroot"); // whether we just print the sysroot path
if !only_setup && std::env::var_os("MIRI_SYSROOT").is_some() {
// Skip setup step if MIRI_SYSROOT is explicitly set, *unless* we are `cargo miri setup`.
return;
if !only_setup {
if let Some(sysroot) = std::env::var_os("MIRI_SYSROOT") {
// Skip setup step if MIRI_SYSROOT is explicitly set, *unless* we are `cargo miri setup`.
return sysroot.into();
}
}
// Determine where the rust sources are located. The env var trumps auto-detection.
@ -92,6 +99,8 @@ pub fn setup(subcommand: &MiriCommand, target: &str, rustc_version: &VersionMeta
command.env("RUSTC", &cargo_miri_path);
}
command.env("MIRI_CALLED_FROM_SETUP", "1");
// Miri expects `MIRI_SYSROOT` to be set when invoked in target mode. Even if that directory is empty.
command.env("MIRI_SYSROOT", &sysroot_dir);
// Make sure there are no other wrappers getting in our way (Cc
// https://github.com/rust-lang/miri/issues/1421,
// https://github.com/rust-lang/miri/issues/2429). Looks like setting
@ -117,8 +126,6 @@ pub fn setup(subcommand: &MiriCommand, target: &str, rustc_version: &VersionMeta
// the user might have set, which is consistent with normal `cargo build` that does
// not apply `RUSTFLAGS` to the sysroot either.
let rustflags = &["-Cdebug-assertions=off", "-Coverflow-checks=on"];
// Make sure all target-level Miri invocations know their sysroot.
std::env::set_var("MIRI_SYSROOT", &sysroot_dir);
// Do the build.
if print_sysroot {
@ -159,4 +166,6 @@ pub fn setup(subcommand: &MiriCommand, target: &str, rustc_version: &VersionMeta
// Print just the sysroot and nothing else to stdout; this way we do not need any escaping.
println!("{}", sysroot_dir.display());
}
sysroot_dir
}

40
src/tools/miri/cargo-miri/src/util.rs
View File

@ -1,7 +1,5 @@
use std::collections::HashMap;
use std::env;
use std::ffi::OsString;
use std::fmt::Write as _;
use std::fs::File;
use std::io::{self, BufWriter, Read, Write};
use std::ops::Not;
@ -247,46 +245,10 @@ pub fn local_crates(metadata: &Metadata) -> String {
local_crates
}
fn env_vars_from_cmd(cmd: &Command) -> Vec<(String, String)> {
let mut envs = HashMap::new();
for (key, value) in std::env::vars() {
envs.insert(key, value);
}
for (key, value) in cmd.get_envs() {
if let Some(value) = value {
envs.insert(key.to_string_lossy().to_string(), value.to_string_lossy().to_string());
} else {
envs.remove(&key.to_string_lossy().to_string());
}
}
let mut envs: Vec<_> = envs.into_iter().collect();
envs.sort();
envs
}
/// Debug-print a command that is going to be run.
pub fn debug_cmd(prefix: &str, verbose: usize, cmd: &Command) {
if verbose == 0 {
return;
}
// We only do a single `eprintln!` call to minimize concurrency interactions.
let mut out = prefix.to_string();
writeln!(out, " running command: env \\").unwrap();
if verbose > 1 {
// Print the full environment this will be called in.
for (key, value) in env_vars_from_cmd(cmd) {
writeln!(out, "{key}={value:?} \\").unwrap();
}
} else {
// Print only what has been changed for this `cmd`.
for (var, val) in cmd.get_envs() {
if let Some(val) = val {
writeln!(out, "{}={val:?} \\", var.to_string_lossy()).unwrap();
} else {
writeln!(out, "--unset={}", var.to_string_lossy()).unwrap();
}
}
}
write!(out, "{cmd:?}").unwrap();
eprintln!("{out}");
eprintln!("{prefix} running command: {cmd:?}");
}

34
src/tools/miri/miri-script/src/commands.rs
View File

@ -6,7 +6,7 @@ use std::ops::Not;
use anyhow::{anyhow, bail, Context, Result};
use path_macro::path;
use walkdir::WalkDir;
use xshell::cmd;
use xshell::{cmd, Shell};
use crate::util::*;
use crate::Command;
@ -16,17 +16,25 @@ const JOSH_FILTER: &str =
":rev(75dd959a3a40eb5b4574f8d2e23aa6efbeb33573:prefix=src/tools/miri):/src/tools/miri";
impl MiriEnv {
fn build_miri_sysroot(&mut self) -> Result<()> {
fn build_miri_sysroot(&mut self, quiet: bool) -> Result<()> {
if self.sh.var("MIRI_SYSROOT").is_ok() {
// Sysroot already set, use that.
return Ok(());
}
let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml");
let Self { toolchain, cargo_extra_flags, .. } = &self;
// Make sure everything is built. Also Miri itself.
self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?;
self.build(&manifest_path, &[], quiet)?;
let target = &match self.sh.var("MIRI_TEST_TARGET") {
Ok(target) => vec!["--target".into(), target],
Err(_) => vec![],
};
if !quiet {
eprintln!("$ (buildig Miri sysroot)");
}
let output = cmd!(self.sh,
"cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} --
miri setup --print-sysroot {target...}"
@ -91,7 +99,7 @@ impl Command {
// Make sure rustup-toolchain-install-master is installed.
which::which("rustup-toolchain-install-master")
.context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?;
let sh = shell()?;
let sh = Shell::new()?;
sh.change_dir(miri_dir()?);
let new_commit = Some(sh.read_file("rust-version")?.trim().to_owned());
let current_commit = {
@ -130,7 +138,7 @@ impl Command {
}
fn rustc_pull(commit: Option<String>) -> Result<()> {
let sh = shell()?;
let sh = Shell::new()?;
sh.change_dir(miri_dir()?);
let commit = commit.map(Result::Ok).unwrap_or_else(|| {
let rust_repo_head =
@ -177,7 +185,7 @@ impl Command {
}
fn rustc_push(github_user: String, branch: String) -> Result<()> {
let sh = shell()?;
let sh = Shell::new()?;
sh.change_dir(miri_dir()?);
let base = sh.read_file("rust-version")?.trim().to_owned();
// Make sure the repo is clean.
@ -265,7 +273,7 @@ impl Command {
let Some((command_name, trailing_args)) = command.split_first() else {
bail!("expected many-seeds command to be non-empty");
};
let sh = shell()?;
let sh = Shell::new()?;
for seed in seed_start..seed_end {
println!("Trying seed: {seed}");
let mut miriflags = env::var_os("MIRIFLAGS").unwrap_or_default();
@ -293,7 +301,7 @@ impl Command {
// Make sure we have an up-to-date Miri installed and selected the right toolchain.
Self::install(vec![])?;
let sh = shell()?;
let sh = Shell::new()?;
sh.change_dir(miri_dir()?);
let benches_dir = "bench-cargo-miri";
let benches = if benches.is_empty() {
@ -365,9 +373,8 @@ impl Command {
fn test(bless: bool, flags: Vec<OsString>) -> Result<()> {
Self::auto_actions()?;
let mut e = MiriEnv::new()?;
// First build, and get a sysroot.
e.build(path!(e.miri_dir / "Cargo.toml"), &[], /* quiet */ true)?;
e.build_miri_sysroot()?;
// Prepare a sysroot.
e.build_miri_sysroot(/* quiet */ false)?;
// Then test, and let caller control flags.
// Only in root project as `cargo-miri` has no tests.
@ -393,12 +400,11 @@ impl Command {
let miriflags = e.sh.var("MIRIFLAGS").unwrap_or_default();
e.sh.set_var("MIRIFLAGS", format!("{miriflags} --target {target}"));
}
// First build, and get a sysroot.
let miri_manifest = path!(e.miri_dir / "Cargo.toml");
e.build(&miri_manifest, &[], /* quiet */ true)?;
e.build_miri_sysroot()?;
// Prepare a sysroot.
e.build_miri_sysroot(/* quiet */ true)?;
// Then run the actual command.
let miri_manifest = path!(e.miri_dir / "Cargo.toml");
let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default();
let miri_flags = flagsplit(&miri_flags);
let toolchain = &e.toolchain;

13
src/tools/miri/miri-script/src/util.rs
View File

@ -13,21 +13,12 @@ pub fn miri_dir() -> std::io::Result<PathBuf> {
/// Queries the active toolchain for the Miri dir.
pub fn active_toolchain() -> Result<String> {
let sh = shell()?;
let sh = Shell::new()?;
sh.change_dir(miri_dir()?);
let stdout = cmd!(sh, "rustup show active-toolchain").read()?;
Ok(stdout.split_whitespace().next().context("Could not obtain active Rust toolchain")?.into())
}
pub fn shell() -> Result<Shell> {
let sh = Shell::new()?;
// xshell does not propagate parent's env variables by default.
for (k, v) in std::env::vars_os() {
sh.set_var(k, v);
}
Ok(sh)
}
pub fn flagsplit(flags: &str) -> Vec<String> {
// This code is taken from `RUSTFLAGS` handling in cargo.
flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect()
@ -50,7 +41,7 @@ pub struct MiriEnv {
impl MiriEnv {
pub fn new() -> Result<Self> {
let toolchain = active_toolchain()?;
let sh = shell()?; // we are preserving the current_dir on this one, so paths resolve properly!
let sh = Shell::new()?; // we are preserving the current_dir on this one, so paths resolve properly!
let miri_dir = miri_dir()?;
let sysroot = cmd!(sh, "rustc +{toolchain} --print sysroot").read()?.into();