Skip to content

Commit

Permalink
Pass around Arc<Package> rather than cloning them
Browse files Browse the repository at this point in the history
  • Loading branch information
sourcefrog committed Jan 4, 2025
1 parent 7e905c7 commit da26d15
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 29 deletions.
15 changes: 6 additions & 9 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,10 @@ fn encoded_rustflags(options: &Options) -> Option<String> {

#[cfg(test)]
mod test {
use camino::Utf8PathBuf;
use clap::Parser;
use pretty_assertions::assert_eq;
use rusty_fork::rusty_fork_test;

use crate::package::Package;
use crate::Args;

use super::*;
Expand All @@ -227,18 +225,17 @@ mod test {
#[test]
fn generate_cargo_args_with_additional_cargo_test_args_and_package() {
let mut options = Options::default();
let package = Package {
name: "cargo-mutants-testdata-something".to_owned(),
relative_dir: Utf8PathBuf::new(),
top_sources: vec!["src/lib.rs".into()],
version: "0.1.0".to_owned(),
};
options
.additional_cargo_test_args
.extend(["--lib", "--no-fail-fast"].iter().map(ToString::to_string));
assert_eq!(
cargo_argv(
&PackageSelection::Explicit(vec![package]),
&PackageSelection::one(
"cargo-mutants-testdata-something",
"0.1.0",
"",
"src/lib.rs"
),
Phase::Check,
&options
)[1..],
Expand Down
8 changes: 4 additions & 4 deletions src/lab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

use std::cmp::{max, min};
use std::panic::resume_unwind;
use std::sync::Mutex;
use std::sync::{Arc, Mutex};
use std::time::Instant;
use std::{thread, vec};

Expand Down Expand Up @@ -174,9 +174,9 @@ impl Lab<'_> {
///
/// If it succeeds, return the timeouts to be used for the other scenarios.
fn run_baseline(&self, build_dir: &BuildDir, mutants: &[Mutant]) -> Result<ScenarioOutcome> {
let all_mutated_packages = mutants
let all_mutated_packages: Vec<Arc<Package>> = mutants
.iter()
.map(|m| m.source_file.package.clone())
.map(|m| Arc::clone(&m.source_file.package))
.sorted_by_key(|p| p.name.clone())
.unique()
.collect_vec();
Expand Down Expand Up @@ -331,7 +331,7 @@ pub enum TestsForMutant {
/// Test only the package that was mutated
Mutated,
/// Test specific packages
Explicit(Vec<Package>),
Explicit(Vec<Arc<Package>>),
}

impl TestsForMutant {
Expand Down
24 changes: 22 additions & 2 deletions src/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

//! Discover and represent cargo packages within a workspace.
use std::sync::Arc;

use camino::{Utf8Path, Utf8PathBuf};
use cargo_metadata::TargetKind;
use itertools::Itertools;
Expand Down Expand Up @@ -31,12 +33,13 @@ pub struct Package {
}

/// Read `cargo-metadata` parsed output, and produce our package representation.
pub fn packages_from_metadata(metadata: &cargo_metadata::Metadata) -> Vec<Package> {
pub fn packages_from_metadata(metadata: &cargo_metadata::Metadata) -> Vec<Arc<Package>> {
metadata
.workspace_packages()
.into_iter()
.sorted_by_key(|p| &p.name)
.filter_map(|p| Package::from_cargo_metadata(p, &metadata.workspace_root))
.map(Arc::new)
.collect()
}

Expand Down Expand Up @@ -133,5 +136,22 @@ pub enum PackageSelection {
/// All packages in the workspace.
All,
/// Explicitly selected packages.
Explicit(Vec<Package>),
Explicit(Vec<Arc<Package>>),
}

impl PackageSelection {
#[cfg(test)]
pub fn one<P: Into<Utf8PathBuf>>(
name: &str,
version: &str,
relative_dir: P,
top_source: &str,
) -> Self {
Self::Explicit(vec![Arc::new(Package {
name: name.to_string(),
version: version.to_string(),
relative_dir: relative_dir.into(),
top_sources: vec![top_source.into()],
})])
}
}
1 change: 0 additions & 1 deletion src/scenario.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use std::fmt;
use crate::Mutant;

/// A scenario is either a freshening build in the source tree, a baseline test with no mutations, or a mutation test.
#[allow(clippy::large_enum_variant)] // baselines are uncommon, size doesn't matter much?
#[derive(Clone, Eq, PartialEq, Debug, Serialize)]
pub enum Scenario {
/// Build in a copy of the source tree but with no mutations applied.
Expand Down
12 changes: 6 additions & 6 deletions src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::span::LineColumn;
#[allow(clippy::module_name_repetitions)]
pub struct SourceFile {
/// What package includes this file?
pub package: Package,
pub package: Arc<Package>,

/// Path of this source file relative to workspace.
pub tree_relative_path: Utf8PathBuf,
Expand Down Expand Up @@ -70,7 +70,7 @@ impl SourceFile {
Ok(Some(SourceFile {
tree_relative_path: tree_relative_path.to_owned(),
code,
package: package.clone(),
package: Arc::new(package.clone()),
is_top,
}))
}
Expand All @@ -92,12 +92,12 @@ impl SourceFile {
SourceFile {
tree_relative_path,
code: Arc::new(code.to_owned()),
package: Package {
package: Arc::new(Package {
name: package_name.to_owned(),
relative_dir: Utf8PathBuf::new(),
top_sources,
version: "0.1.0".to_owned(),
},
}),
is_top,
}
}
Expand Down Expand Up @@ -155,12 +155,12 @@ mod test {

#[test]
fn skips_files_outside_of_workspace() {
let package = Package {
let package = Arc::new(Package {
name: "imaginary-package".to_owned(),
relative_dir: Utf8PathBuf::from(""),
top_sources: vec!["src/lib.rs".into()],
version: "0.1.0".to_owned(),
};
});
let source_file = SourceFile::load(
Utf8Path::new("unimportant"),
Utf8Path::new("../outside_workspace.rs"),
Expand Down
2 changes: 1 addition & 1 deletion src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl Discovered {
/// they generated mutants or not).
pub fn walk_tree(
workspace_dir: &Utf8Path,
packages: &[Package],
packages: &[Arc<Package>],
options: &Options,
console: &Console,
) -> Result<Discovered> {
Expand Down
14 changes: 8 additions & 6 deletions src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::fmt;
use std::panic::catch_unwind;
use std::path::Path;
use std::process::Command;
use std::sync::Arc;

use anyhow::{anyhow, bail, ensure, Context};
use camino::{Utf8Path, Utf8PathBuf};
Expand Down Expand Up @@ -67,7 +68,7 @@ impl PackageFilter {
/// A cargo workspace.
pub struct Workspace {
metadata: cargo_metadata::Metadata,
packages: Vec<Package>,
packages: Vec<Arc<Package>>,
}

impl fmt::Debug for Workspace {
Expand Down Expand Up @@ -110,17 +111,18 @@ impl Workspace {
Ok(Workspace { metadata, packages })
}

pub fn packages_by_name<S: AsRef<str>>(&self, names: &[S]) -> Vec<Package> {
pub fn packages_by_name<S: AsRef<str>>(&self, names: &[S]) -> Vec<Arc<Package>> {
names
.iter()
.map(AsRef::as_ref)
.sorted()
.filter_map(|name| {
let p = self.packages.iter().find(|p| p.name == name);
if p.is_none() {
if let Some(p) = self.packages.iter().find(|p| p.name == name) {
Some(Arc::clone(p))
} else {
warn!("Package {name:?} not found in source tree");
None
}
p.cloned()
})
.collect()
}
Expand Down Expand Up @@ -166,7 +168,7 @@ impl Workspace {
}
}

fn expand_selection(&self, selection: PackageSelection) -> Vec<Package> {
fn expand_selection(&self, selection: PackageSelection) -> Vec<Arc<Package>> {
match selection {
PackageSelection::All => self.packages.clone(),
PackageSelection::Explicit(packages) => packages,
Expand Down

0 comments on commit da26d15

Please sign in to comment.