Allow building Miri with --features from miri-script

Otherwise there was no way to pass e.g. `--features tracing` just to the `cargo` commands issued on the root repository:
CARGO_EXTRA_FLAGS applies the flags to the "cargo-miri" crate, too, which does not make sense for crate-specific features.

Fix install_to_sysroot doing path concatenation twice. Since the second concatenation was against an absolute path, it didn't do anything. This also makes the interface of install_to_sysroot() similar to that of cargo_cmd().

Implement --features for clippy, also fix not passing features to one of the cargo invocations for test
This commit is contained in:
Stypox 2025-04-09 10:34:03 +02:00
parent f5e10748e5
commit a009612691
No known key found for this signature in database
GPG key ID: 4BDF1B40A49FDD23
4 changed files with 153 additions and 65 deletions

View file

@ -32,6 +32,7 @@ impl MiriEnv {
&mut self,
quiet: bool,
target: Option<impl AsRef<OsStr>>,
features: &[String],
) -> Result<PathBuf> {
if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") {
// Sysroot already set, use that.
@ -39,8 +40,8 @@ impl MiriEnv {
}
// Make sure everything is built. Also Miri itself.
self.build(".", &[], quiet)?;
self.build("cargo-miri", &[], quiet)?;
self.build(".", features, &[], quiet)?;
self.build("cargo-miri", &[], &[], quiet)?;
let target_flag = if let Some(target) = &target {
vec![OsStr::new("--target"), target.as_ref()]
@ -58,7 +59,7 @@ impl MiriEnv {
}
let mut cmd = self
.cargo_cmd("cargo-miri", "run")
.cargo_cmd("cargo-miri", "run", &[])
.arg("--quiet")
.arg("--")
.args(&["miri", "setup", "--print-sysroot"])
@ -90,7 +91,9 @@ impl Command {
Self::fmt(vec![])?;
}
if auto_clippy {
Self::clippy(vec![])?;
// no features for auto actions, see
// https://github.com/rust-lang/miri/pull/4396#discussion_r2149654845
Self::clippy(vec![], vec![])?;
}
Ok(())
@ -175,16 +178,16 @@ impl Command {
}
// Then run the actual command.
match self {
Command::Install { flags } => Self::install(flags),
Command::Build { flags } => Self::build(flags),
Command::Check { flags } => Self::check(flags),
Command::Test { bless, flags, target, coverage } =>
Self::test(bless, flags, target, coverage),
Command::Run { dep, verbose, target, edition, flags } =>
Self::run(dep, verbose, target, edition, flags),
Command::Doc { flags } => Self::doc(flags),
Command::Install { features, flags } => Self::install(features, flags),
Command::Build { features, flags } => Self::build(features, flags),
Command::Check { features, flags } => Self::check(features, flags),
Command::Test { bless, target, coverage, features, flags } =>
Self::test(bless, target, coverage, features, flags),
Command::Run { dep, verbose, target, edition, features, flags } =>
Self::run(dep, verbose, target, edition, features, flags),
Command::Doc { features, flags } => Self::doc(features, flags),
Command::Fmt { flags } => Self::fmt(flags),
Command::Clippy { flags } => Self::clippy(flags),
Command::Clippy { features, flags } => Self::clippy(features, flags),
Command::Bench { target, no_install, save_baseline, load_baseline, benches } =>
Self::bench(target, no_install, save_baseline, load_baseline, benches),
Command::Toolchain { flags } => Self::toolchain(flags),
@ -494,7 +497,7 @@ impl Command {
if !no_install {
// Make sure we have an up-to-date Miri installed and selected the right toolchain.
Self::install(vec![])?;
Self::install(vec![], vec![])?;
}
let results_json_dir = if save_baseline.is_some() || load_baseline.is_some() {
Some(TempDir::new()?)
@ -601,47 +604,48 @@ impl Command {
Ok(())
}
fn install(flags: Vec<String>) -> Result<()> {
fn install(features: Vec<String>, flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.install_to_sysroot(e.miri_dir.clone(), &flags)?;
e.install_to_sysroot(path!(e.miri_dir / "cargo-miri"), &flags)?;
e.install_to_sysroot(".", &features, &flags)?;
e.install_to_sysroot("cargo-miri", &[], &flags)?;
Ok(())
}
fn build(flags: Vec<String>) -> Result<()> {
fn build(features: Vec<String>, flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.build(".", &flags, /* quiet */ false)?;
e.build("cargo-miri", &flags, /* quiet */ false)?;
e.build(".", &features, &flags, /* quiet */ false)?;
e.build("cargo-miri", &[], &flags, /* quiet */ false)?;
Ok(())
}
fn check(flags: Vec<String>) -> Result<()> {
fn check(features: Vec<String>, flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.check(".", &flags)?;
e.check("cargo-miri", &flags)?;
e.check(".", &features, &flags)?;
e.check("cargo-miri", &[], &flags)?;
Ok(())
}
fn doc(flags: Vec<String>) -> Result<()> {
fn doc(features: Vec<String>, flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.doc(".", &flags)?;
e.doc("cargo-miri", &flags)?;
e.doc(".", &features, &flags)?;
e.doc("cargo-miri", &[], &flags)?;
Ok(())
}
fn clippy(flags: Vec<String>) -> Result<()> {
fn clippy(features: Vec<String>, flags: Vec<String>) -> Result<()> {
let e = MiriEnv::new()?;
e.clippy(".", &flags)?;
e.clippy("cargo-miri", &flags)?;
e.clippy("miri-script", &flags)?;
e.clippy(".", &features, &flags)?;
e.clippy("cargo-miri", &[], &flags)?;
e.clippy("miri-script", &[], &flags)?;
Ok(())
}
fn test(
bless: bool,
mut flags: Vec<String>,
target: Option<String>,
coverage: bool,
features: Vec<String>,
mut flags: Vec<String>,
) -> Result<()> {
let mut e = MiriEnv::new()?;
@ -652,7 +656,7 @@ impl Command {
}
// Prepare a sysroot. (Also builds cargo-miri, which we need.)
e.build_miri_sysroot(/* quiet */ false, target.as_deref())?;
e.build_miri_sysroot(/* quiet */ false, target.as_deref(), &features)?;
// Forward information to test harness.
if bless {
@ -672,10 +676,10 @@ impl Command {
// Then test, and let caller control flags.
// Only in root project as `cargo-miri` has no tests.
e.test(".", &flags)?;
e.test(".", &features, &flags)?;
if let Some(coverage) = &coverage {
coverage.show_coverage_report(&e)?;
coverage.show_coverage_report(&e, &features)?;
}
Ok(())
@ -686,14 +690,17 @@ impl Command {
verbose: bool,
target: Option<String>,
edition: Option<String>,
features: Vec<String>,
flags: Vec<String>,
) -> Result<()> {
let mut e = MiriEnv::new()?;
// Preparation: get a sysroot, and get the miri binary.
let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?;
let miri_bin =
e.build_get_binary(".").context("failed to get filename of miri executable")?;
let miri_sysroot =
e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref(), &features)?;
let miri_bin = e
.build_get_binary(".", &features)
.context("failed to get filename of miri executable")?;
// More flags that we will pass before `flags`
// (because `flags` may contain `--`).
@ -718,7 +725,7 @@ impl Command {
// The basic command that executes the Miri driver.
let mut cmd = if dep {
// We invoke the test suite as that has all the logic for running with dependencies.
e.cargo_cmd(".", "test")
e.cargo_cmd(".", "test", &features)
.args(&["--test", "ui"])
.args(quiet_flag)
.arg("--")

View file

@ -49,7 +49,7 @@ impl CoverageReport {
/// show_coverage_report will print coverage information using the artifact
/// files in `self.path`.
pub fn show_coverage_report(&self, e: &MiriEnv) -> Result<()> {
pub fn show_coverage_report(&self, e: &MiriEnv, features: &[String]) -> Result<()> {
let profraw_files = self.profraw_files()?;
let profdata_bin = path!(e.libdir / ".." / "bin" / "llvm-profdata");
@ -63,8 +63,9 @@ impl CoverageReport {
// Create the coverage report.
let cov_bin = path!(e.libdir / ".." / "bin" / "llvm-cov");
let miri_bin =
e.build_get_binary(".").context("failed to get filename of miri executable")?;
let miri_bin = e
.build_get_binary(".", features)
.context("failed to get filename of miri executable")?;
cmd!(
e.sh,
"{cov_bin} report --instr-profile={merged_file} --object {miri_bin} --sources src/"

View file

@ -14,24 +14,40 @@ pub enum Command {
/// Sets up the rpath such that the installed binary should work in any
/// working directory.
Install {
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
features: Vec<String>,
/// Flags that are passed through to `cargo install`.
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
flags: Vec<String>,
},
/// Build Miri.
Build {
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
features: Vec<String>,
/// Flags that are passed through to `cargo build`.
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
flags: Vec<String>,
},
/// Check Miri.
Check {
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
features: Vec<String>,
/// Flags that are passed through to `cargo check`.
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
flags: Vec<String>,
},
/// Check Miri with Clippy.
Clippy {
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
features: Vec<String>,
/// Flags that are passed through to `cargo clippy`.
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
flags: Vec<String>,
@ -47,6 +63,10 @@ pub enum Command {
/// Produce coverage report.
#[arg(long)]
coverage: bool,
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
features: Vec<String>,
/// Flags that are passed through to the test harness.
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
flags: Vec<String>,
@ -67,6 +87,10 @@ pub enum Command {
/// The Rust edition.
#[arg(long)]
edition: Option<String>,
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
features: Vec<String>,
/// Flags that are passed through to `miri`.
///
/// The flags set in `MIRIFLAGS` are added in front of these flags.
@ -75,6 +99,10 @@ pub enum Command {
},
/// Build documentation.
Doc {
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
features: Vec<String>,
/// Flags that are passed through to `cargo doc`.
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
flags: Vec<String>,
@ -144,13 +172,13 @@ impl Command {
}
match self {
Self::Install { flags }
| Self::Build { flags }
| Self::Check { flags }
| Self::Doc { flags }
Self::Install { flags, .. }
| Self::Build { flags, .. }
| Self::Check { flags, .. }
| Self::Doc { flags, .. }
| Self::Fmt { flags }
| Self::Toolchain { flags }
| Self::Clippy { flags }
| Self::Clippy { flags, .. }
| Self::Run { flags, .. }
| Self::Test { flags, .. } => {
flags.extend(remainder);

View file

@ -26,6 +26,12 @@ pub fn flagsplit(flags: &str) -> Vec<String> {
flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect()
}
/// Turns a list of features into a list of arguments to pass to cargo invocations.
/// Each feature will go in its own argument, e.g. "--features feat1 --features feat2".
fn features_to_args(features: &[String]) -> impl IntoIterator<Item = &str> {
features.iter().flat_map(|feat| ["--features", feat])
}
/// Some extra state we track for building Miri, such as the right RUSTFLAGS.
pub struct MiriEnv {
/// miri_dir is the root of the miri repository checkout we are working in.
@ -116,44 +122,70 @@ impl MiriEnv {
Ok(MiriEnv { miri_dir, toolchain, sh, sysroot, cargo_extra_flags, libdir })
}
pub fn cargo_cmd(&self, crate_dir: impl AsRef<OsStr>, cmd: &str) -> Cmd<'_> {
/// Make sure the `features` you pass here exist for the specified `crate_dir`. For example, the
/// "--features" parameter of [crate::Command]s is intended only for the "miri" root crate.
pub fn cargo_cmd(
&self,
crate_dir: impl AsRef<OsStr>,
cmd: &str,
features: &[String],
) -> Cmd<'_> {
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
let manifest_path = path!(self.miri_dir / crate_dir.as_ref() / "Cargo.toml");
let features = features_to_args(features);
cmd!(
self.sh,
"cargo +{toolchain} {cmd} {cargo_extra_flags...} --manifest-path {manifest_path}"
"cargo +{toolchain} {cmd} {cargo_extra_flags...} --manifest-path {manifest_path} {features...}"
)
}
/// Make sure the `features` you pass here exist for the specified `crate_dir`. For example, the
/// "--features" parameter of [crate::Command]s is intended only for the "miri" root crate.
pub fn install_to_sysroot(
&self,
path: impl AsRef<OsStr>,
crate_dir: impl AsRef<OsStr>,
features: &[String],
args: impl IntoIterator<Item = impl AsRef<OsStr>>,
) -> Result<()> {
let MiriEnv { sysroot, toolchain, cargo_extra_flags, .. } = self;
let path = path!(self.miri_dir / path.as_ref());
let path = path!(self.miri_dir / crate_dir.as_ref());
let features = features_to_args(features);
// Install binaries to the miri toolchain's `sysroot` so they do not interact with other toolchains.
// (Not using `cargo_cmd` as `install` is special and doesn't use `--manifest-path`.)
cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?;
cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {features...} {args...}").run()?;
Ok(())
}
pub fn build(&self, crate_dir: impl AsRef<OsStr>, args: &[String], quiet: bool) -> Result<()> {
pub fn build(
&self,
crate_dir: impl AsRef<OsStr>,
features: &[String],
args: &[String],
quiet: bool,
) -> Result<()> {
let quiet_flag = if quiet { Some("--quiet") } else { None };
// We build all targets, since building *just* the bin target doesnot include
// `dev-dependencies` and that changes feature resolution. This also gets us more
// parallelism in `./miri test` as we build Miri and its tests together.
let mut cmd =
self.cargo_cmd(crate_dir, "build").args(&["--all-targets"]).args(quiet_flag).args(args);
let mut cmd = self
.cargo_cmd(crate_dir, "build", features)
.args(&["--all-targets"])
.args(quiet_flag)
.args(args);
cmd.set_quiet(quiet);
cmd.run()?;
Ok(())
}
/// Returns the path to the main crate binary. Assumes that `build` has been called before.
pub fn build_get_binary(&self, crate_dir: impl AsRef<OsStr>) -> Result<PathBuf> {
let cmd =
self.cargo_cmd(crate_dir, "build").args(&["--all-targets", "--message-format=json"]);
pub fn build_get_binary(
&self,
crate_dir: impl AsRef<OsStr>,
features: &[String],
) -> Result<PathBuf> {
let cmd = self
.cargo_cmd(crate_dir, "build", features)
.args(&["--all-targets", "--message-format=json"]);
let output = cmd.output()?;
let mut bin = None;
for line in output.stdout.lines() {
@ -174,23 +206,43 @@ impl MiriEnv {
bin.ok_or_else(|| anyhow!("found no binary in cargo output"))
}
pub fn check(&self, crate_dir: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
self.cargo_cmd(crate_dir, "check").arg("--all-targets").args(args).run()?;
pub fn check(
&self,
crate_dir: impl AsRef<OsStr>,
features: &[String],
args: &[String],
) -> Result<()> {
self.cargo_cmd(crate_dir, "check", features).arg("--all-targets").args(args).run()?;
Ok(())
}
pub fn doc(&self, crate_dir: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
self.cargo_cmd(crate_dir, "doc").args(args).run()?;
pub fn doc(
&self,
crate_dir: impl AsRef<OsStr>,
features: &[String],
args: &[String],
) -> Result<()> {
self.cargo_cmd(crate_dir, "doc", features).args(args).run()?;
Ok(())
}
pub fn clippy(&self, crate_dir: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
self.cargo_cmd(crate_dir, "clippy").arg("--all-targets").args(args).run()?;
pub fn clippy(
&self,
crate_dir: impl AsRef<OsStr>,
features: &[String],
args: &[String],
) -> Result<()> {
self.cargo_cmd(crate_dir, "clippy", features).arg("--all-targets").args(args).run()?;
Ok(())
}
pub fn test(&self, crate_dir: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
self.cargo_cmd(crate_dir, "test").args(args).run()?;
pub fn test(
&self,
crate_dir: impl AsRef<OsStr>,
features: &[String],
args: &[String],
) -> Result<()> {
self.cargo_cmd(crate_dir, "test", features).args(args).run()?;
Ok(())
}