Skip to content

Commit 7919999

Browse files
committed
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.
1 parent ab56b52 commit 7919999

File tree

5 files changed

+124
-52
lines changed

5 files changed

+124
-52
lines changed

miri-script/src/commands.rs

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,16 @@ impl MiriEnv {
3232
&mut self,
3333
quiet: bool,
3434
target: Option<impl AsRef<OsStr>>,
35+
features: &[String],
3536
) -> Result<PathBuf> {
3637
if let Some(miri_sysroot) = self.sh.var_os("MIRI_SYSROOT") {
3738
// Sysroot already set, use that.
3839
return Ok(miri_sysroot.into());
3940
}
4041

4142
// Make sure everything is built. Also Miri itself.
42-
self.build(".", &[], quiet)?;
43-
self.build("cargo-miri", &[], quiet)?;
43+
self.build(".", features, &[], quiet)?;
44+
self.build("cargo-miri", &[], &[], quiet)?;
4445

4546
let target_flag = if let Some(target) = &target {
4647
vec![OsStr::new("--target"), target.as_ref()]
@@ -58,7 +59,7 @@ impl MiriEnv {
5859
}
5960

6061
let mut cmd = self
61-
.cargo_cmd("cargo-miri", "run")
62+
.cargo_cmd("cargo-miri", "run", &[])
6263
.arg("--quiet")
6364
.arg("--")
6465
.args(&["miri", "setup", "--print-sysroot"])
@@ -175,14 +176,14 @@ impl Command {
175176
}
176177
// Then run the actual command.
177178
match self {
178-
Command::Install { flags } => Self::install(flags),
179-
Command::Build { flags } => Self::build(flags),
180-
Command::Check { flags } => Self::check(flags),
181-
Command::Test { bless, flags, target, coverage } =>
182-
Self::test(bless, flags, target, coverage),
183-
Command::Run { dep, verbose, target, edition, flags } =>
184-
Self::run(dep, verbose, target, edition, flags),
185-
Command::Doc { flags } => Self::doc(flags),
179+
Command::Install { features, flags } => Self::install(features, flags),
180+
Command::Build { features, flags } => Self::build(features, flags),
181+
Command::Check { features, flags } => Self::check(features, flags),
182+
Command::Test { bless, flags, target, coverage, features } =>
183+
Self::test(bless, flags, target, coverage, features),
184+
Command::Run { dep, verbose, target, edition, features, flags } =>
185+
Self::run(dep, verbose, target, edition, features, flags),
186+
Command::Doc { features, flags } => Self::doc(features, flags),
186187
Command::Fmt { flags } => Self::fmt(flags),
187188
Command::Clippy { flags } => Self::clippy(flags),
188189
Command::Bench { target, no_install, save_baseline, load_baseline, benches } =>
@@ -494,7 +495,7 @@ impl Command {
494495

495496
if !no_install {
496497
// Make sure we have an up-to-date Miri installed and selected the right toolchain.
497-
Self::install(vec![])?;
498+
Self::install(vec![], vec![])?;
498499
}
499500
let results_json_dir = if save_baseline.is_some() || load_baseline.is_some() {
500501
Some(TempDir::new()?)
@@ -601,31 +602,31 @@ impl Command {
601602
Ok(())
602603
}
603604

604-
fn install(flags: Vec<String>) -> Result<()> {
605+
fn install(features: Vec<String>, flags: Vec<String>) -> Result<()> {
605606
let e = MiriEnv::new()?;
606-
e.install_to_sysroot(e.miri_dir.clone(), &flags)?;
607-
e.install_to_sysroot(path!(e.miri_dir / "cargo-miri"), &flags)?;
607+
e.install_to_sysroot(e.miri_dir.clone(), &features, &flags)?;
608+
e.install_to_sysroot(path!(e.miri_dir / "cargo-miri"), &[], &flags)?;
608609
Ok(())
609610
}
610611

611-
fn build(flags: Vec<String>) -> Result<()> {
612+
fn build(features: Vec<String>, flags: Vec<String>) -> Result<()> {
612613
let e = MiriEnv::new()?;
613-
e.build(".", &flags, /* quiet */ false)?;
614-
e.build("cargo-miri", &flags, /* quiet */ false)?;
614+
e.build(".", &features, &flags, /* quiet */ false)?;
615+
e.build("cargo-miri", &[], &flags, /* quiet */ false)?;
615616
Ok(())
616617
}
617618

618-
fn check(flags: Vec<String>) -> Result<()> {
619+
fn check(features: Vec<String>, flags: Vec<String>) -> Result<()> {
619620
let e = MiriEnv::new()?;
620-
e.check(".", &flags)?;
621-
e.check("cargo-miri", &flags)?;
621+
e.check(".", &features, &flags)?;
622+
e.check("cargo-miri", &[], &flags)?;
622623
Ok(())
623624
}
624625

625-
fn doc(flags: Vec<String>) -> Result<()> {
626+
fn doc(features: Vec<String>, flags: Vec<String>) -> Result<()> {
626627
let e = MiriEnv::new()?;
627-
e.doc(".", &flags)?;
628-
e.doc("cargo-miri", &flags)?;
628+
e.doc(".", &features, &flags)?;
629+
e.doc("cargo-miri", &[], &flags)?;
629630
Ok(())
630631
}
631632

@@ -642,6 +643,7 @@ impl Command {
642643
mut flags: Vec<String>,
643644
target: Option<String>,
644645
coverage: bool,
646+
features: Vec<String>,
645647
) -> Result<()> {
646648
let mut e = MiriEnv::new()?;
647649

@@ -652,7 +654,7 @@ impl Command {
652654
}
653655

654656
// Prepare a sysroot. (Also builds cargo-miri, which we need.)
655-
e.build_miri_sysroot(/* quiet */ false, target.as_deref())?;
657+
e.build_miri_sysroot(/* quiet */ false, target.as_deref(), &features)?;
656658

657659
// Forward information to test harness.
658660
if bless {
@@ -675,7 +677,7 @@ impl Command {
675677
e.test(".", &flags)?;
676678

677679
if let Some(coverage) = &coverage {
678-
coverage.show_coverage_report(&e)?;
680+
coverage.show_coverage_report(&e, &features)?;
679681
}
680682

681683
Ok(())
@@ -686,14 +688,17 @@ impl Command {
686688
verbose: bool,
687689
target: Option<String>,
688690
edition: Option<String>,
691+
features: Vec<String>,
689692
flags: Vec<String>,
690693
) -> Result<()> {
691694
let mut e = MiriEnv::new()?;
692695

693696
// Preparation: get a sysroot, and get the miri binary.
694-
let miri_sysroot = e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref())?;
695-
let miri_bin =
696-
e.build_get_binary(".").context("failed to get filename of miri executable")?;
697+
let miri_sysroot =
698+
e.build_miri_sysroot(/* quiet */ !verbose, target.as_deref(), &features)?;
699+
let miri_bin = e
700+
.build_get_binary(".", &features)
701+
.context("failed to get filename of miri executable")?;
697702

698703
// More flags that we will pass before `flags`
699704
// (because `flags` may contain `--`).
@@ -718,7 +723,7 @@ impl Command {
718723
// The basic command that executes the Miri driver.
719724
let mut cmd = if dep {
720725
// We invoke the test suite as that has all the logic for running with dependencies.
721-
e.cargo_cmd(".", "test")
726+
e.cargo_cmd(".", "test", &features)
722727
.args(&["--test", "ui"])
723728
.args(quiet_flag)
724729
.arg("--")

miri-script/src/coverage.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ impl CoverageReport {
4949

5050
/// show_coverage_report will print coverage information using the artifact
5151
/// files in `self.path`.
52-
pub fn show_coverage_report(&self, e: &MiriEnv) -> Result<()> {
52+
pub fn show_coverage_report(&self, e: &MiriEnv, features: &[String]) -> Result<()> {
5353
let profraw_files = self.profraw_files()?;
5454

5555
let profdata_bin = path!(e.libdir / ".." / "bin" / "llvm-profdata");
@@ -63,8 +63,9 @@ impl CoverageReport {
6363

6464
// Create the coverage report.
6565
let cov_bin = path!(e.libdir / ".." / "bin" / "llvm-cov");
66-
let miri_bin =
67-
e.build_get_binary(".").context("failed to get filename of miri executable")?;
66+
let miri_bin = e
67+
.build_get_binary(".", features)
68+
.context("failed to get filename of miri executable")?;
6869
cmd!(
6970
e.sh,
7071
"{cov_bin} report --instr-profile={merged_file} --object {miri_bin} --sources src/"

miri-script/src/main.rs

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,30 @@ pub enum Command {
1414
/// Sets up the rpath such that the installed binary should work in any
1515
/// working directory.
1616
Install {
17+
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
18+
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
19+
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
20+
features: Vec<String>,
1721
/// Flags that are passed through to `cargo install`.
1822
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
1923
flags: Vec<String>,
2024
},
2125
/// Build Miri.
2226
Build {
27+
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
28+
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
29+
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
30+
features: Vec<String>,
2331
/// Flags that are passed through to `cargo build`.
2432
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
2533
flags: Vec<String>,
2634
},
2735
/// Check Miri.
2836
Check {
37+
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
38+
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
39+
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
40+
features: Vec<String>,
2941
/// Flags that are passed through to `cargo check`.
3042
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
3143
flags: Vec<String>,
@@ -47,6 +59,10 @@ pub enum Command {
4759
/// Produce coverage report.
4860
#[arg(long)]
4961
coverage: bool,
62+
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
63+
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
64+
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
65+
features: Vec<String>,
5066
/// Flags that are passed through to the test harness.
5167
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
5268
flags: Vec<String>,
@@ -67,6 +83,10 @@ pub enum Command {
6783
/// The Rust edition.
6884
#[arg(long)]
6985
edition: Option<String>,
86+
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
87+
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
88+
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
89+
features: Vec<String>,
7090
/// Flags that are passed through to `miri`.
7191
///
7292
/// The flags set in `MIRIFLAGS` are added in front of these flags.
@@ -75,6 +95,10 @@ pub enum Command {
7595
},
7696
/// Build documentation.
7797
Doc {
98+
/// Pass features to cargo invocations on the "miri" crate in the root. This option does
99+
/// **not** apply to other crates, so e.g. these features won't be used on "cargo-miri".
100+
#[arg(long, value_delimiter = ',', action = clap::ArgAction::Append)]
101+
features: Vec<String>,
78102
/// Flags that are passed through to `cargo doc`.
79103
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
80104
flags: Vec<String>,
@@ -144,10 +168,10 @@ impl Command {
144168
}
145169

146170
match self {
147-
Self::Install { flags }
148-
| Self::Build { flags }
149-
| Self::Check { flags }
150-
| Self::Doc { flags }
171+
Self::Install { flags, .. }
172+
| Self::Build { flags, .. }
173+
| Self::Check { flags, .. }
174+
| Self::Doc { flags, .. }
151175
| Self::Fmt { flags }
152176
| Self::Toolchain { flags }
153177
| Self::Clippy { flags }

miri-script/src/util.rs

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ pub fn flagsplit(flags: &str) -> Vec<String> {
2626
flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect()
2727
}
2828

29+
/// Turns a list of features into a list of arguments to pass to cargo invocations.
30+
/// Each feature will go in its own argument, e.g. "--features feat1 --features feat2".
31+
fn features_to_args(features: &[String]) -> impl IntoIterator<Item = &str> {
32+
features.iter().flat_map(|feat| ["--features", feat])
33+
}
34+
2935
/// Some extra state we track for building Miri, such as the right RUSTFLAGS.
3036
pub struct MiriEnv {
3137
/// miri_dir is the root of the miri repository checkout we are working in.
@@ -116,44 +122,70 @@ impl MiriEnv {
116122
Ok(MiriEnv { miri_dir, toolchain, sh, sysroot, cargo_extra_flags, libdir })
117123
}
118124

119-
pub fn cargo_cmd(&self, crate_dir: impl AsRef<OsStr>, cmd: &str) -> Cmd<'_> {
125+
/// Make sure the `features` you pass here exist for the specified `crate_dir`. For example, the
126+
/// "--features" parameter of [crate::Command]s is intended only for the "miri" root crate.
127+
pub fn cargo_cmd(
128+
&self,
129+
crate_dir: impl AsRef<OsStr>,
130+
cmd: &str,
131+
features: &[String],
132+
) -> Cmd<'_> {
120133
let MiriEnv { toolchain, cargo_extra_flags, .. } = self;
121134
let manifest_path = path!(self.miri_dir / crate_dir.as_ref() / "Cargo.toml");
135+
let features = features_to_args(features);
122136
cmd!(
123137
self.sh,
124-
"cargo +{toolchain} {cmd} {cargo_extra_flags...} --manifest-path {manifest_path}"
138+
"cargo +{toolchain} {cmd} {cargo_extra_flags...} --manifest-path {manifest_path} {features...}"
125139
)
126140
}
127141

142+
/// Make sure the `features` you pass here exist for the specified `path`. For example, the
143+
/// "--features" parameter of [crate::Command]s is intended only for the "miri" root crate.
128144
pub fn install_to_sysroot(
129145
&self,
130146
path: impl AsRef<OsStr>,
147+
features: &[String],
131148
args: impl IntoIterator<Item = impl AsRef<OsStr>>,
132149
) -> Result<()> {
133150
let MiriEnv { sysroot, toolchain, cargo_extra_flags, .. } = self;
134151
let path = path!(self.miri_dir / path.as_ref());
152+
let features = features_to_args(features);
135153
// Install binaries to the miri toolchain's `sysroot` so they do not interact with other toolchains.
136154
// (Not using `cargo_cmd` as `install` is special and doesn't use `--manifest-path`.)
137-
cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?;
155+
cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {features...} {args...}").run()?;
138156
Ok(())
139157
}
140158

141-
pub fn build(&self, crate_dir: impl AsRef<OsStr>, args: &[String], quiet: bool) -> Result<()> {
159+
pub fn build(
160+
&self,
161+
crate_dir: impl AsRef<OsStr>,
162+
features: &[String],
163+
args: &[String],
164+
quiet: bool,
165+
) -> Result<()> {
142166
let quiet_flag = if quiet { Some("--quiet") } else { None };
143167
// We build all targets, since building *just* the bin target doesnot include
144168
// `dev-dependencies` and that changes feature resolution. This also gets us more
145169
// parallelism in `./miri test` as we build Miri and its tests together.
146-
let mut cmd =
147-
self.cargo_cmd(crate_dir, "build").args(&["--all-targets"]).args(quiet_flag).args(args);
170+
let mut cmd = self
171+
.cargo_cmd(crate_dir, "build", features)
172+
.args(&["--all-targets"])
173+
.args(quiet_flag)
174+
.args(args);
148175
cmd.set_quiet(quiet);
149176
cmd.run()?;
150177
Ok(())
151178
}
152179

153180
/// Returns the path to the main crate binary. Assumes that `build` has been called before.
154-
pub fn build_get_binary(&self, crate_dir: impl AsRef<OsStr>) -> Result<PathBuf> {
155-
let cmd =
156-
self.cargo_cmd(crate_dir, "build").args(&["--all-targets", "--message-format=json"]);
181+
pub fn build_get_binary(
182+
&self,
183+
crate_dir: impl AsRef<OsStr>,
184+
features: &[String],
185+
) -> Result<PathBuf> {
186+
let cmd = self
187+
.cargo_cmd(crate_dir, "build", features)
188+
.args(&["--all-targets", "--message-format=json"]);
157189
let output = cmd.output()?;
158190
let mut bin = None;
159191
for line in output.stdout.lines() {
@@ -174,23 +206,33 @@ impl MiriEnv {
174206
bin.ok_or_else(|| anyhow!("found no binary in cargo output"))
175207
}
176208

177-
pub fn check(&self, crate_dir: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
178-
self.cargo_cmd(crate_dir, "check").arg("--all-targets").args(args).run()?;
209+
pub fn check(
210+
&self,
211+
crate_dir: impl AsRef<OsStr>,
212+
features: &[String],
213+
args: &[String],
214+
) -> Result<()> {
215+
self.cargo_cmd(crate_dir, "check", features).arg("--all-targets").args(args).run()?;
179216
Ok(())
180217
}
181218

182-
pub fn doc(&self, crate_dir: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
183-
self.cargo_cmd(crate_dir, "doc").args(args).run()?;
219+
pub fn doc(
220+
&self,
221+
crate_dir: impl AsRef<OsStr>,
222+
features: &[String],
223+
args: &[String],
224+
) -> Result<()> {
225+
self.cargo_cmd(crate_dir, "doc", features).args(args).run()?;
184226
Ok(())
185227
}
186228

187229
pub fn clippy(&self, crate_dir: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
188-
self.cargo_cmd(crate_dir, "clippy").arg("--all-targets").args(args).run()?;
230+
self.cargo_cmd(crate_dir, "clippy", &[]).arg("--all-targets").args(args).run()?;
189231
Ok(())
190232
}
191233

192234
pub fn test(&self, crate_dir: impl AsRef<OsStr>, args: &[String]) -> Result<()> {
193-
self.cargo_cmd(crate_dir, "test").args(args).run()?;
235+
self.cargo_cmd(crate_dir, "test", &[]).args(args).run()?;
194236
Ok(())
195237
}
196238

output.tracy

451 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)