Skip to content

Commit 58b75b4

Browse files
committed
Remove dyn SourceTree
Squashed commit of the following: commit 654e6f546870d03e61527e84f02f76a59e2792c3 Author: Martin Pool <mbp@sourcefrog.net> Date: Sun Jan 15 19:00:37 2023 -0800 More refactors of the tool-process-run area commit df400c5c17eece0e3de60e57e3fd6e2260e68a6b Author: Martin Pool <mbp@sourcefrog.net> Date: Sun Jan 15 18:34:02 2023 -0800 Adjust rustflags only once commit 80256f2e6a4798a914c710d779a18a54fda01aaf Author: Martin Pool <mbp@sourcefrog.net> Date: Sun Jan 15 18:31:41 2023 -0800 More separation of concerns in running Cargo commit c0d6a04fe17b7e7eb37176d2283563868430995d Author: Martin Pool <mbp@sourcefrog.net> Date: Sun Jan 15 17:59:40 2023 -0800 Move more cargo-specific details into its tool Rename PhaseResult fields for consistency. commit 23142170271b23cdb069447d33826a3ef8b6a456 Author: Martin Pool <mbp@sourcefrog.net> Date: Sat Jan 14 09:27:27 2023 -0800 Discover files and mutants through Tool commit 8aaee6ea371c208d6fb8762be712fffa12aacf6e Author: Martin Pool <mbp@sourcefrog.net> Date: Sat Jan 14 09:16:46 2023 -0800 Add CargoTool::new commit b9606b3f2b23c674a0262be2336cd8597045bbf1 Author: Martin Pool <mbp@sourcefrog.net> Date: Sat Jan 14 09:16:39 2023 -0800 Doc commit 93d551c5a76df3146a683ef05048357f4eaac4e5 Author: Martin Pool <mbp@sourcefrog.net> Date: Sat Jan 14 09:11:16 2023 -0800 Remove dyn SourceTree Instead, a simpler(?) structure of passing TreeRoots as just Utf8Paths, and a Tool interface that could be Cargo or potentially something else. commit 68fb8d6343f2ca5b53e7074068ace5b9767ed8de Author: Martin Pool <mbp@sourcefrog.net> Date: Fri Jan 6 07:27:57 2023 -0800 WIP refactoring away from per-tool source trees
1 parent 6d5e1cf commit 58b75b4

File tree

14 files changed

+280
-239
lines changed

14 files changed

+280
-239
lines changed

NEWS.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
join existing environment variable `CARGO_MUTANTS_MINIMUM_TEST_TIMEOUT`, to allow
1616
boosting the minimum, especially for test environments with poor or uneven throughput.
1717

18+
- Changed: Renamed fields in `outcomes.json` from `cargo_result` to `process_status` and from `command` to `argv`.
19+
1820
## 1.2.1
1921

2022
Released 2023-01-05
@@ -71,7 +73,7 @@ Released 2022-09-24
7173

7274
- New: `cargo mutants --completions SHELL` to generate shell completions using `clap_complete`.
7375

74-
- Changed: `carg-mutants` no longer builds in the source directory, and no longer copies the `target/` directory to the scratch directory. Since `cargo-mutants` now sets `RUSTFLAGS` to avoid false failures from warnings, it is unlikely to match the existing build products in the source directory `target/`, and in fact building there is just likely to cause rebuilds in the source. The behavior now is as if `--no-copy-target` was always passed. That option is still accepted, but it has no effect.
76+
- Changed: `cargo-mutants` no longer builds in the source directory, and no longer copies the `target/` directory to the scratch directory. Since `cargo-mutants` now sets `RUSTFLAGS` to avoid false failures from warnings, it is unlikely to match the existing build products in the source directory `target/`, and in fact building there is just likely to cause rebuilds in the source. The behavior now is as if `--no-copy-target` was always passed. That option is still accepted, but it has no effect.
7577

7678
- Changed: `cargo-mutants` finds all possible mutations before doing the baseline test, so that you can see earlier how many there will be.
7779

src/build_dir.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2021, 2022 Martin Pool
1+
// Copyright 2021-2023 Martin Pool
22

33
//! A temporary directory containing mutated source to run cargo builds and tests.
44
@@ -42,13 +42,12 @@ impl BuildDir {
4242
/// Make a new build dir, copying from a source directory.
4343
///
4444
/// [SOURCE_EXCLUDE] is excluded.
45-
pub fn new(source: &dyn SourceTree, console: &Console) -> Result<BuildDir> {
46-
let name_base = format!("cargo-mutants-{}-", source.path().file_name().unwrap_or(""));
45+
pub fn new(source: &Utf8Path, console: &Console) -> Result<BuildDir> {
46+
let name_base = format!("cargo-mutants-{}-", source.file_name().unwrap_or(""));
4747
let source_abs = source
48-
.path()
4948
.canonicalize_utf8()
5049
.expect("canonicalize source path");
51-
let temp_dir = copy_tree(source.path(), &name_base, SOURCE_EXCLUDE, console)?;
50+
let temp_dir = copy_tree(source, &name_base, SOURCE_EXCLUDE, console)?;
5251
let path: Utf8PathBuf = temp_dir.path().to_owned().try_into().unwrap();
5352
fix_manifest(&path.join("Cargo.toml"), &source_abs)?;
5453
fix_cargo_config(&path, &source_abs)?;
@@ -139,8 +138,10 @@ mod test {
139138

140139
#[test]
141140
fn build_dir_debug_form() {
142-
let source_tree = CargoSourceTree::open("testdata/tree/factorial".into()).unwrap();
143-
let build_dir = BuildDir::new(&source_tree, &Console::new()).unwrap();
141+
let root = CargoTool::new()
142+
.find_root("testdata/tree/factorial".into())
143+
.unwrap();
144+
let build_dir = BuildDir::new(&root, &Console::new()).unwrap();
144145
let debug_form = format!("{build_dir:?}");
145146
assert!(
146147
Regex::new(r#"^BuildDir \{ path: "[^"]*[/\\]cargo-mutants-factorial[^"]*" \}$"#)

src/cargo.rs

Lines changed: 80 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
// Copyright 2021, 2022 Martin Pool
1+
// Copyright 2021-2023 Martin Pool
22

33
//! Run Cargo as a subprocess, including timeouts and propagating signals.
44
55
use std::env;
66
use std::sync::Arc;
7-
use std::thread::sleep;
8-
use std::time::{Duration, Instant};
97

108
#[allow(unused_imports)]
119
use anyhow::{anyhow, bail, Context, Result};
@@ -14,56 +12,84 @@ use serde_json::Value;
1412
#[allow(unused_imports)]
1513
use tracing::{debug, error, info, span, trace, warn, Level};
1614

17-
use crate::console::Console;
18-
use crate::log_file::LogFile;
1915
use crate::path::TreeRelativePathBuf;
20-
use crate::process::{get_command_output, Process, ProcessStatus};
16+
use crate::process::get_command_output;
17+
use crate::tool::Tool;
2118
use crate::*;
2219

23-
/// How frequently to check if cargo finished.
24-
const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50);
25-
26-
/// Run one `cargo` subprocess, with a timeout, and with appropriate handling of interrupts.
27-
pub fn run_cargo(
28-
build_dir: &BuildDir,
29-
argv: &[String],
30-
log_file: &mut LogFile,
31-
timeout: Duration,
32-
console: &Console,
33-
rustflags: &str,
34-
) -> Result<ProcessStatus> {
35-
let start = Instant::now();
20+
#[derive(Debug)]
21+
pub struct CargoTool {
22+
// environment is currently constant across all invocations.
23+
env: Vec<(String, String)>,
24+
}
3625

37-
// The tests might use Insta <https://insta.rs>, and we don't want it to write
38-
// updates to the source tree, and we *certainly* don't want it to write
39-
// updates and then let the test pass.
26+
impl CargoTool {
27+
pub fn new() -> CargoTool {
28+
let env = vec![
29+
("CARGO_ENCODED_RUSTFLAGS".to_owned(), rustflags()),
30+
// The tests might use Insta <https://insta.rs>, and we don't want it to write
31+
// updates to the source tree, and we *certainly* don't want it to write
32+
// updates and then let the test pass.
33+
("INSTA_UPDATE".to_owned(), "no".to_owned()),
34+
];
35+
CargoTool { env }
36+
}
37+
}
4038

41-
let env = [
42-
("CARGO_ENCODED_RUSTFLAGS", rustflags),
43-
("INSTA_UPDATE", "no"),
44-
];
45-
debug!(?env);
39+
impl Tool for CargoTool {
40+
fn find_root(&self, path: &Utf8Path) -> Result<Utf8PathBuf> {
41+
let cargo_toml_path = locate_cargo_toml(path)?;
42+
let root = cargo_toml_path
43+
.parent()
44+
.expect("cargo_toml_path has a parent")
45+
.to_owned();
46+
assert!(root.is_dir());
47+
Ok(root)
48+
}
4649

47-
let mut child = Process::start(argv, &env, build_dir.path(), timeout, log_file)?;
50+
fn root_files(&self, source_root_path: &Utf8Path) -> Result<Vec<Arc<SourceFile>>> {
51+
let cargo_toml_path = source_root_path.join("Cargo.toml");
52+
debug!("cargo_toml_path = {}", cargo_toml_path);
53+
check_interrupted()?;
54+
let metadata = cargo_metadata::MetadataCommand::new()
55+
.manifest_path(&cargo_toml_path)
56+
.exec()
57+
.context("run cargo metadata")?;
58+
check_interrupted()?;
4859

49-
let process_status = loop {
50-
if let Some(exit_status) = child.poll()? {
51-
break exit_status;
52-
} else {
53-
console.tick();
54-
sleep(WAIT_POLL_INTERVAL);
60+
let mut r = Vec::new();
61+
for package_metadata in &metadata.workspace_packages() {
62+
debug!("walk package {:?}", package_metadata.manifest_path);
63+
let package_name = Arc::new(package_metadata.name.to_string());
64+
for source_path in direct_package_sources(source_root_path, package_metadata)? {
65+
check_interrupted()?;
66+
r.push(Arc::new(SourceFile::new(
67+
source_root_path,
68+
source_path,
69+
package_name.clone(),
70+
)?));
71+
}
5572
}
56-
};
73+
Ok(r)
74+
}
5775

58-
let message = format!(
59-
"cargo result: {:?} in {:.3}s",
60-
process_status,
61-
start.elapsed().as_secs_f64()
62-
);
63-
log_file.message(&message);
64-
debug!(cargo_result = ?process_status, elapsed = ?start.elapsed());
65-
check_interrupted()?;
66-
Ok(process_status)
76+
fn compose_argv(
77+
&self,
78+
scenario: &Scenario,
79+
phase: Phase,
80+
options: &Options,
81+
) -> Result<Vec<String>> {
82+
Ok(cargo_argv(scenario.package_name(), phase, options))
83+
}
84+
85+
fn compose_env(
86+
&self,
87+
_scenario: &Scenario,
88+
_phase: Phase,
89+
_options: &Options,
90+
) -> Result<Vec<(String, String)>> {
91+
Ok(self.env.clone())
92+
}
6793
}
6894

6995
/// Return the name of the cargo binary.
@@ -76,7 +102,7 @@ fn cargo_bin() -> String {
76102

77103
/// Make up the argv for a cargo check/build/test invocation, including argv[0] as the
78104
/// cargo binary itself.
79-
pub fn cargo_argv(package_name: Option<&str>, phase: Phase, options: &Options) -> Vec<String> {
105+
fn cargo_argv(package_name: Option<&str>, phase: Phase, options: &Options) -> Vec<String> {
80106
let mut cargo_args = vec![cargo_bin(), phase.name().to_string()];
81107
if phase == Phase::Check || phase == Phase::Build {
82108
cargo_args.push("--tests".to_string());
@@ -94,39 +120,13 @@ pub fn cargo_argv(package_name: Option<&str>, phase: Phase, options: &Options) -
94120
cargo_args
95121
}
96122

97-
/// A source tree where we can run cargo commands.
98-
#[derive(Debug)]
99-
pub struct CargoSourceTree {
100-
pub root: Utf8PathBuf,
101-
cargo_toml_path: Utf8PathBuf,
102-
}
103-
104-
impl CargoSourceTree {
105-
/// Open the source tree enclosing the given path.
106-
///
107-
/// Returns an error if it's not found.
108-
pub fn open(path: &Utf8Path) -> Result<CargoSourceTree> {
109-
let cargo_toml_path = locate_cargo_toml(path)?;
110-
let root = cargo_toml_path
111-
.parent()
112-
.expect("cargo_toml_path has a parent")
113-
.to_owned();
114-
assert!(root.is_dir());
115-
116-
Ok(CargoSourceTree {
117-
root,
118-
cargo_toml_path,
119-
})
120-
}
121-
}
122-
123123
/// Return adjusted CARGO_ENCODED_RUSTFLAGS, including any changes to cap-lints.
124124
///
125125
/// This does not currently read config files; it's too complicated.
126126
///
127127
/// See <https://doc.rust-lang.org/cargo/reference/environment-variables.html>
128128
/// <https://doc.rust-lang.org/rustc/lints/levels.html#capping-lints>
129-
pub fn rustflags() -> String {
129+
fn rustflags() -> String {
130130
let mut rustflags: Vec<String> = if let Some(rustflags) = env::var_os("CARGO_ENCODED_RUSTFLAGS")
131131
{
132132
rustflags
@@ -150,7 +150,7 @@ pub fn rustflags() -> String {
150150
Vec::new()
151151
};
152152
rustflags.push("--cap-lints=allow".to_owned());
153-
debug!("adjusted rustflags: {:?}", rustflags);
153+
// debug!("adjusted rustflags: {:?}", rustflags);
154154
rustflags.join("\x1f")
155155
}
156156

@@ -173,37 +173,6 @@ fn locate_cargo_toml(path: &Utf8Path) -> Result<Utf8PathBuf> {
173173
Ok(cargo_toml_path)
174174
}
175175

176-
impl SourceTree for CargoSourceTree {
177-
fn path(&self) -> &Utf8Path {
178-
&self.root
179-
}
180-
181-
fn root_files(&self, _options: &Options) -> Result<Vec<Arc<SourceFile>>> {
182-
debug!("cargo_toml_path = {}", self.cargo_toml_path);
183-
check_interrupted()?;
184-
let metadata = cargo_metadata::MetadataCommand::new()
185-
.manifest_path(&self.cargo_toml_path)
186-
.exec()
187-
.context("run cargo metadata")?;
188-
check_interrupted()?;
189-
190-
let mut r = Vec::new();
191-
for package_metadata in &metadata.workspace_packages() {
192-
debug!("walk package {:?}", package_metadata.manifest_path);
193-
let package_name = Arc::new(package_metadata.name.to_string());
194-
for source_path in direct_package_sources(&self.root, package_metadata)? {
195-
check_interrupted()?;
196-
r.push(Arc::new(SourceFile::new(
197-
&self.root,
198-
source_path,
199-
package_name.clone(),
200-
)?));
201-
}
202-
}
203-
Ok(r)
204-
}
205-
}
206-
207176
/// Find all the files that are named in the `path` of targets in a Cargo manifest that should be tested.
208177
///
209178
/// These are the starting points for discovering source files.
@@ -320,17 +289,17 @@ mod test {
320289

321290
#[test]
322291
fn error_opening_outside_of_crate() {
323-
CargoSourceTree::open(Utf8Path::new("/")).unwrap_err();
292+
CargoTool::new().find_root(Utf8Path::new("/")).unwrap_err();
324293
}
325294

326295
#[test]
327296
fn open_subdirectory_of_crate_opens_the_crate() {
328-
let source_tree = CargoSourceTree::open(Utf8Path::new("testdata/tree/factorial/src"))
297+
let root = CargoTool::new()
298+
.find_root(Utf8Path::new("testdata/tree/factorial/src"))
329299
.expect("open source tree from subdirectory");
330-
let path = source_tree.path();
331-
assert!(path.is_dir());
332-
assert!(path.join("Cargo.toml").is_file());
333-
assert!(path.join("src/bin/factorial.rs").is_file());
334-
assert_eq!(path.file_name().unwrap(), OsStr::new("factorial"));
300+
assert!(root.is_dir());
301+
assert!(root.join("Cargo.toml").is_file());
302+
assert!(root.join("src/bin/factorial.rs").is_file());
303+
assert_eq!(root.file_name().unwrap(), OsStr::new("factorial"));
335304
}
336305
}

src/config.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use anyhow::Context;
1515
use camino::Utf8Path;
1616
use serde::Deserialize;
1717

18-
use crate::source::SourceTree;
1918
use crate::Result;
2019

2120
/// Configuration read from a config file.
@@ -49,8 +48,8 @@ impl Config {
4948

5049
/// Read the config from a tree's `.cargo/mutants.toml`, and return a default (empty)
5150
/// Config is the file does not exist.
52-
pub fn read_tree_config(source_tree: &dyn SourceTree) -> Result<Config> {
53-
let path = source_tree.path().join(".cargo").join("mutants.toml");
51+
pub fn read_tree_config(source_tree_root: &Utf8Path) -> Result<Config> {
52+
let path = source_tree_root.join(".cargo").join("mutants.toml");
5453
if path.exists() {
5554
Config::read_file(&path)
5655
} else {

0 commit comments

Comments
 (0)