Skip to content

feat: new arg --cargo-config for passing --config to cargo #1913

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.

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

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion collector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ The following options alter the behaviour of the `bench_local` subcommand.
by `rustup` will be used. This is usually fine, though in rare cases it may
cause local results to not exactly match production results, because Cargo
sometimes begins passing (or stops passing) various flags to rustc.
- `--cargo-config <CONFIG>`: a Cargo configuration value or a path to a Cargo
configuration file. This flag can be specified multiple times, and will be
passed to the Cargo executable as the value of the flag
[`--config`](https://doc.rust-lang.org/nightly/cargo/commands/cargo.html#option-cargo---config).
- `--db <DATABASE>`: a path (relative or absolute) to a sqlite database file in
which the timing data will be placed. It will be created if it does not
already exist. The default is `results.db`. Alternatively, the collector
Expand Down Expand Up @@ -195,7 +199,7 @@ and discover all benchmarks within them. If you only want to run benchmark(s) fr
you can use this to speed up the runtime benchmarking or profiling commands.

The `bench_runtime_local` command also shares some options with the `bench_local` command, notably
`--id`, `--db`, `--cargo`, `--include`, `--exclude` and `--iterations`.
`--id`, `--db`, `--cargo`, `--cargo-config`, `--include`, `--exclude` and `--iterations`.

### How to view the measurements on your own machine

Expand Down Expand Up @@ -465,6 +469,7 @@ fashion to the one chosen for `bench_local`.

The following options alter the behaviour of the `profile_local` subcommand.
- `--cargo <CARGO>`: as for `bench_local`.
- `--cargo-config <CONFIG>`: as for `bench_local`.
- `--exclude <EXCLUDE>`: as for `bench_local`.
- `--id <ID>`: an identifer that will form part of the output filenames.
- `--include <INCLUDE>`: as for `bench_local`.
Expand Down
3 changes: 3 additions & 0 deletions collector/src/artifact_stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ pub fn compile_and_get_stats(

let mut cmd = Command::new(&toolchain.components.cargo);
cmd.arg("build").arg("--target-dir").arg(tempdir.path());
for config in &toolchain.components.cargo_configs {
cmd.arg("--config").arg(config);
}
match profile {
CargoProfile::Debug => {}
CargoProfile::Release => {
Expand Down
14 changes: 9 additions & 5 deletions collector/src/bin/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,10 @@ struct LocalOptions {
#[arg(long)]
cargo: Option<PathBuf>,

/// Arguments passed to `cargo --config <value>`.
#[arg(long)]
cargo_config: Vec<String>,

/// Exclude all benchmarks matching a prefix in this comma-separated list
#[arg(long, value_delimiter = ',')]
exclude: Vec<String>,
Expand Down Expand Up @@ -845,7 +849,7 @@ fn main_result() -> anyhow::Result<i32> {
*ToolchainConfig::default()
.rustdoc(opts.rustdoc.as_deref())
.clippy(opts.clippy.as_deref())
.cargo(local.cargo.as_deref())
.cargo(local.cargo.as_deref(), local.cargo_config.as_slice())
.id(local.id.as_deref()),
"",
target_triple,
Expand Down Expand Up @@ -1070,7 +1074,7 @@ fn main_result() -> anyhow::Result<i32> {
*ToolchainConfig::default()
.rustdoc(opts.rustdoc.as_deref())
.clippy(opts.clippy.as_deref())
.cargo(local.cargo.as_deref())
.cargo(local.cargo.as_deref(), local.cargo_config.as_slice())
.id(local.id.as_deref()),
suffix,
target_triple.clone(),
Expand Down Expand Up @@ -1228,7 +1232,7 @@ fn binary_stats_compile(
&[codegen_backend],
&local.rustc,
*ToolchainConfig::default()
.cargo(local.cargo.as_deref())
.cargo(local.cargo.as_deref(), local.cargo_config.as_slice())
.id(local.id.as_deref()),
"",
target_triple.to_string(),
Expand All @@ -1240,7 +1244,7 @@ fn binary_stats_compile(
&[codegen_backend2],
&rustc,
*ToolchainConfig::default()
.cargo(local.cargo.as_deref())
.cargo(local.cargo.as_deref(), local.cargo_config.as_slice())
.id(local.id.as_deref()),
"",
target_triple.to_string(),
Expand Down Expand Up @@ -1484,7 +1488,7 @@ fn get_local_toolchain_for_runtime_benchmarks(
&[CodegenBackend::Llvm],
&local.rustc,
*ToolchainConfig::default()
.cargo(local.cargo.as_deref())
.cargo(local.cargo.as_deref(), local.cargo_config.as_slice())
.id(local.id.as_deref()),
"",
target_triple.to_string(),
Expand Down
4 changes: 4 additions & 0 deletions collector/src/compile/execute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ impl<'a> CargoProcess<'a> {
if let Some(c) = &self.toolchain.components.clippy {
cmd.env("CLIPPY", &*FAKE_CLIPPY).env("CLIPPY_REAL", c);
}

for config in &self.toolchain.components.cargo_configs {
cmd.arg("--config").arg(config);
}
cmd
}

Expand Down
4 changes: 4 additions & 0 deletions collector/src/runtime/benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,10 @@ fn start_cargo_build(
#[cfg(feature = "precise-cachegrind")]
command.arg("--features").arg("benchlib/precise-cachegrind");

for config in &toolchain.components.cargo_configs {
command.arg("--config").arg(config);
}

CargoArtifactIter::from_cargo_cmd(command)
.map_err(|error| anyhow::anyhow!("Failed to start cargo: {:?}", error))
}
Expand Down
14 changes: 9 additions & 5 deletions collector/src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ pub struct ToolchainComponents {
pub rustdoc: Option<PathBuf>,
pub clippy: Option<PathBuf>,
pub cargo: PathBuf,
pub cargo_configs: Vec<String>,
pub lib_rustc: Option<PathBuf>,
pub lib_std: Option<PathBuf>,
pub lib_test: Option<PathBuf>,
Expand Down Expand Up @@ -329,6 +330,8 @@ pub struct ToolchainConfig<'a> {
rustdoc: Option<&'a Path>,
clippy: Option<&'a Path>,
cargo: Option<&'a Path>,
/// For `cargo --config <value>`.
cargo_configs: &'a [String],
id: Option<&'a str>,
}

Expand All @@ -343,8 +346,9 @@ impl<'a> ToolchainConfig<'a> {
self
}

pub fn cargo(&mut self, cargo: Option<&'a Path>) -> &mut Self {
pub fn cargo(&mut self, cargo: Option<&'a Path>, configs: &'a [String]) -> &mut Self {
self.cargo = cargo;
self.cargo_configs = configs;
self
}

Expand Down Expand Up @@ -520,13 +524,13 @@ pub fn get_local_toolchain(
debug!("found cargo: {:?}", &cargo);
cargo
};

let lib_dir = get_lib_dir_from_rustc(&rustc).context("Cannot find libdir for rustc")?;

let mut components =
ToolchainComponents::from_binaries_and_libdir(rustc, rustdoc, clippy, cargo, &lib_dir)?;
components.cargo_configs = toolchain_config.cargo_configs.to_vec();
Ok(Toolchain {
components: ToolchainComponents::from_binaries_and_libdir(
rustc, rustdoc, clippy, cargo, &lib_dir,
)?,
components,
id,
triple: target_triple,
})
Expand Down
Loading