Skip to content

Commit

Permalink
Try to read RUSTFLAGS from configs, etc.
Browse files Browse the repository at this point in the history
  • Loading branch information
sourcefrog committed Oct 31, 2022
1 parent 174ad0e commit d1c173c
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 10 deletions.
32 changes: 32 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ tracing-appender = "0.2"
tracing-subscriber = "0.3"
walkdir = "2.3"
whoami = "1.2"
dirs = "4.0.0"

[dependencies.regex]
version = "1.5"
Expand Down
12 changes: 12 additions & 0 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,15 @@ The initial baseline build is done in a single job, with no parallelism at the c
If the baseline test completes successfully, its build directory is copied to make a total of one per parallel job. Unlike the initial copy from the source directory, this includes the `target` directory, since it should now be up to date for the compiler options that cargo-mutants will use.

We then launch the appropriate number of threads, each of which has its own build directory. They each build and test new mutants until everything is done, or until there's an error that stops all processing.

## `RUSTFLAGS`

Cargo has a somewhat complex system for controlling flags passed to `rustc`. cargo-mutants uses this to pass `--cap-lints` to avoid failures due to strict lints.

However, for other settings, we want to keep using whatever flags the user has configured in their environment, in the source tree, or in their per-user configuration. Unfortunately the Cargo `RUSTFLAGS` or `CARGO_ENCODED_RUSTFLAGS` variables entirely replace, instead of appending to, the user's flags.

This seems to require us to reimplement some of Cargo's logic from <https://doc.rust-lang.org/cargo/reference/config.html#buildrustflags> to first determine the user's flags, and then append our own.

Reading the configuration in turn requires knowing the right target triple.

In testing this we should probably override `CARGO_HOME` to avoid picking up any user configuration.
125 changes: 116 additions & 9 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ use std::sync::Arc;
use std::thread::sleep;
use std::time::{Duration, Instant};

use anyhow::bail;
#[allow(unused_imports)]
use anyhow::{anyhow, Context, Result};
use camino::Utf8Path;
use anyhow::{anyhow, bail, Context, Result};
use camino::{Utf8Path, Utf8PathBuf};
use globset::GlobSet;
use serde_json::Value;
#[allow(unused_imports)]
Expand All @@ -28,24 +27,26 @@ const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50);

/// Run one `cargo` subprocess, with a timeout, and with appropriate handling of interrupts.
pub fn run_cargo(
build_dir: &BuildDir,
argv: &[String],
in_dir: &Utf8Path,
log_file: &mut LogFile,
timeout: Duration,
console: &Console,
rustflags: &str,
) -> Result<ProcessStatus> {
let start = Instant::now();

// See <https://doc.rust-lang.org/cargo/reference/environment-variables.html>
// <https://doc.rust-lang.org/rustc/lints/levels.html#capping-lints>
//
// The tests might use Insta <https://insta.rs>, and we don't want it to write
// updates to the source tree, and we *certainly* don't want it to write
// updates and then let the test pass.

let env = [("RUSTFLAGS", "--cap-lints=allow"), ("INSTA_UPDATE", "no")];
let env = [
("CARGO_ENCODED_RUSTFLAGS", rustflags),
("INSTA_UPDATE", "no"),
];
debug!(?env);

let mut child = Process::start(argv, &env, in_dir, timeout, log_file)?;
let mut child = Process::start(argv, &env, build_dir.path(), timeout, log_file)?;

let process_status = loop {
if let Some(exit_status) = child.poll()? {
Expand Down Expand Up @@ -113,11 +114,45 @@ impl CargoSourceTree {
.expect("cargo_toml_path has a parent")
.to_owned();
assert!(root.is_dir());

Ok(CargoSourceTree {
root,
cargo_toml_path,
})
}

/// Return appropriate CARGO_ENCODED_RUSTFLAGS for building this tree, including any changes to cap-lints.
///
/// See <https://doc.rust-lang.org/cargo/reference/environment-variables.html>
/// <https://doc.rust-lang.org/rustc/lints/levels.html#capping-lints>
pub fn rustflags(&self) -> String {
let mut rustflags: Vec<String> =
if let Some(rustflags) = env::var_os("CARGO_ENCODED_RUSTFLAGS") {
rustflags
.to_str()
.expect("CARGO_ENCODED_RUSTFLAGS is not valid UTF-8")
.split(|c| c == '\x1f')
.map(|s| s.to_owned())
.collect()
} else if let Some(rustflags) = env::var_os("RUSTFLAGS") {
rustflags
.to_str()
.expect("RUSTFLAGS is not valid UTF-8")
.split(' ')
.map(|s| s.to_owned())
.collect()
} else {
// TODO: Determine the right target triple and profile?
let config_paths = enclosing_config_files(&self.root);
debug!("search config files {config_paths:?}");
// TODO: All matching target.<triple>.rustflags and target.<cfg>.rustflags config entries joined together.
// TODO: build.rustflags config value.
Vec::new()
};
rustflags.push("--cap-lints=allow".to_owned());
debug!("adjusted rustflags: {:?}", rustflags);
rustflags.join("\x1f")
}
}

/// Run `cargo locate-project` to find the path of the `Cargo.toml` enclosing this path.
Expand Down Expand Up @@ -261,6 +296,58 @@ fn should_mutate_target(target: &cargo_metadata::Target) -> bool {
target.kind.iter().any(|k| k.ends_with("lib") || k == "bin")
}

/// Return a list of cargo config.toml files enclosing a directory, and in the
/// cargo home directory.
///
/// Only actually existing files are returned.
fn enclosing_config_files(path: &Utf8Path) -> Result<Vec<Utf8PathBuf>> {
// https://doc.rust-lang.org/cargo/reference/config.html#hierarchical-structure
// NOTE: The docs are ambiguous on what order the arrays are joined; but it
// seems to make sense to put the most-specific (first-searched) one *last*
// so that it can override earlier values.
// TODO: Unit test this walking up some directory tree?
let mut path = path.canonicalize_utf8().context("canonicalize path")?;
let mut r: Vec<Utf8PathBuf> = Vec::new();
loop {
for suffix in &[".cargo/config.toml", ".cargo/config"] {
let config_path = path.join(suffix);
if config_path.exists() {
r.push(config_path);
break;
}
}
if let Some(parent) = path.parent() {
path = parent.to_owned();
} else {
break;
}
}
if let Some(cargo_home) = cargo_home() {
for filename in ["config.toml", "config"] {
let config_path = cargo_home.join(filename);
if config_path.exists() {
if !r.contains(&config_path) {
r.push(config_path);
}
break;
}
}
}
Ok(r)
}

fn cargo_home() -> Option<Utf8PathBuf> {
if let Some(home) = env::var_os("CARGO_HOME") {
let home = home.to_str().expect("CARGO_HOME is not valid UTF-8");
Some(Utf8PathBuf::from(home))
} else if let Some(home) = dirs::home_dir() {
let home: Utf8PathBuf = home.try_into().expect("home_dir is not valid UTF-8");
Some(home.join(".cargo"))
} else {
None
}
}

#[cfg(test)]
mod test {
use std::ffi::OsStr;
Expand Down Expand Up @@ -366,4 +453,24 @@ mod test {
assert!(path.join("src/bin/factorial.rs").is_file());
assert_eq!(path.file_name().unwrap(), OsStr::new("factorial"));
}

/// Either CARGO_HOME is set, or at least it can be found in HOME.
#[test]
fn cargo_home_is_found_in_test_environment() {
assert!(super::cargo_home().is_some());
}

/// In the common case where the source is inside HOME, we still don't get duplicated config paths.
#[test]
fn enclosing_config_files_has_no_duplicates() {
let paths = enclosing_config_files("testdata/tree/small_well_tested".into()).unwrap();
for i in 0..paths.len() {
for j in (i + 1)..(paths.len()) {
assert_ne!(
paths[i], paths[j],
"duplicate config file found in {paths:?}"
);
}
}
}
}
8 changes: 7 additions & 1 deletion src/lab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ pub fn test_unmutated_then_all_mutants(
let output_dir = OutputDir::new(output_in_dir)?;
console.set_debug_log(output_dir.open_debug_log()?);

let rustflags = source_tree.rustflags();

let mut mutants = source_tree.mutants(&options)?;
if options.shuffle {
mutants.shuffle(&mut rand::thread_rng());
Expand Down Expand Up @@ -63,6 +65,7 @@ pub fn test_unmutated_then_all_mutants(
phases,
options.test_timeout.unwrap_or(Duration::MAX),
console,
&rustflags,
)?
};
if !baseline_outcome.success() {
Expand Down Expand Up @@ -128,6 +131,7 @@ pub fn test_unmutated_then_all_mutants(
phases,
mutated_test_timeout,
console,
&rustflags,
)
.expect("scenario test");
} else {
Expand Down Expand Up @@ -163,6 +167,7 @@ fn test_scenario(
phases: &[Phase],
test_timeout: Duration,
console: &Console,
rustflags: &str,
) -> Result<ScenarioOutcome> {
let mut log_file = output_mutex
.lock()
Expand All @@ -185,11 +190,12 @@ fn test_scenario(
_ => Duration::MAX,
};
let cargo_result = run_cargo(
&build_dir,
&cargo_argv,
build_dir.path(),
&mut log_file,
timeout,
console,
rustflags,
)?;
outcome.add_phase_result(phase, phase_start.elapsed(), cargo_result, &cargo_argv);
console.scenario_phase_finished(scenario, phase);
Expand Down

0 comments on commit d1c173c

Please sign in to comment.