Skip to content

Allow building Miri with --features from miri-script #4396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Allow building Miri with --features from miri-script #4396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99282e2
Allow building Miri with --features from miri-script
Stypox Apr 9, 2025
dbef80a
Fix install_to_sysroot doing path concatenation twice
Stypox Jun 12, 2025
967a531
Implement --features for clippy and auto_actions
Stypox Jun 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 52 additions & 47 deletions miri-script/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,16 @@ 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.
return Ok(miri_sysroot.into());
}

// 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()]
Expand All @@ -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"])
Expand All @@ -71,7 +72,7 @@ impl MiriEnv {
}

impl Command {
fn auto_actions() -> Result<()> {
fn auto_actions(features: Vec<String>) -> Result<()> {
if env::var_os("MIRI_AUTO_OPS").is_some_and(|x| x == "no") {
return Ok(());
}
Expand All @@ -90,7 +91,7 @@ impl Command {
Self::fmt(vec![])?;
}
if auto_clippy {
Self::clippy(vec![])?;
Self::clippy(features, vec![])?;
}

Ok(())
Expand Down Expand Up @@ -159,14 +160,14 @@ impl Command {
pub fn exec(self) -> Result<()> {
// First, and crucially only once, run the auto-actions -- but not for all commands.
match &self {
Command::Install { .. }
| Command::Build { .. }
| Command::Check { .. }
| Command::Test { .. }
| Command::Run { .. }
| Command::Fmt { .. }
| Command::Doc { .. }
| Command::Clippy { .. } => Self::auto_actions()?,
Command::Install { features, .. }
| Command::Build { features, .. }
| Command::Check { features, .. }
| Command::Test { features, .. }
| Command::Run { features, .. }
| Command::Doc { features, .. }
| Command::Clippy { features, .. } => Self::auto_actions(features.clone())?,
| Command::Fmt { .. } => Self::auto_actions(vec![])?,
Copy link
Contributor Author

@Stypox Stypox Jun 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fmt does not have features, however the features might be needed for some auto action. Should I add features to Fmt too, and expand its doc to explain how it's not actually used for Fmt but rather for auto actions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this to fmt just for auto-clippy doesn't make sense IMO. I don't think auto-clippy should enable any features... but I don't use auto-clippy anyway. @oli-obk what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto-clippy should be indistinguishable from ./miri clippy, so no features, yes.

| Command::Toolchain { .. }
| Command::Bench { .. }
| Command::RustcPull { .. }
Expand All @@ -175,16 +176,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),
Expand Down Expand Up @@ -494,7 +495,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()?)
Expand Down Expand Up @@ -601,47 +602,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()?;

Expand All @@ -652,7 +654,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 {
Expand All @@ -672,10 +674,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(())
Expand All @@ -686,14 +688,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 `--`).
Expand All @@ -718,7 +723,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("--")
Expand Down
7 changes: 4 additions & 3 deletions miri-script/src/coverage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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/"
Expand Down
38 changes: 33 additions & 5 deletions miri-script/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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>,
Expand All @@ -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.
Expand All @@ -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>,
Expand Down Expand Up @@ -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);
Expand Down
Loading
Loading