From da26d1542444c5ccf282384395dda85a11fc5453 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 3 Jan 2025 17:23:59 -0800 Subject: [PATCH] Pass around Arc rather than cloning them --- src/cargo.rs | 15 ++++++--------- src/lab.rs | 8 ++++---- src/package.rs | 24 ++++++++++++++++++++++-- src/scenario.rs | 1 - src/source.rs | 12 ++++++------ src/visit.rs | 2 +- src/workspace.rs | 14 ++++++++------ 7 files changed, 47 insertions(+), 29 deletions(-) diff --git a/src/cargo.rs b/src/cargo.rs index cb607735..e8367699 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -197,12 +197,10 @@ fn encoded_rustflags(options: &Options) -> Option { #[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::*; @@ -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..], diff --git a/src/lab.rs b/src/lab.rs index ba34abb2..793ce021 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -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}; @@ -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 { - let all_mutated_packages = mutants + let all_mutated_packages: Vec> = 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(); @@ -331,7 +331,7 @@ pub enum TestsForMutant { /// Test only the package that was mutated Mutated, /// Test specific packages - Explicit(Vec), + Explicit(Vec>), } impl TestsForMutant { diff --git a/src/package.rs b/src/package.rs index 41bccbec..45fe370a 100644 --- a/src/package.rs +++ b/src/package.rs @@ -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; @@ -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 { +pub fn packages_from_metadata(metadata: &cargo_metadata::Metadata) -> Vec> { 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() } @@ -133,5 +136,22 @@ pub enum PackageSelection { /// All packages in the workspace. All, /// Explicitly selected packages. - Explicit(Vec), + Explicit(Vec>), +} + +impl PackageSelection { + #[cfg(test)] + pub fn one>( + 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()], + })]) + } } diff --git a/src/scenario.rs b/src/scenario.rs index f599aeeb..36fce116 100644 --- a/src/scenario.rs +++ b/src/scenario.rs @@ -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. diff --git a/src/source.rs b/src/source.rs index fe288042..9dc353e1 100644 --- a/src/source.rs +++ b/src/source.rs @@ -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, /// Path of this source file relative to workspace. pub tree_relative_path: Utf8PathBuf, @@ -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, })) } @@ -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, } } @@ -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"), diff --git a/src/visit.rs b/src/visit.rs index 898f96a8..852a9cd6 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -59,7 +59,7 @@ impl Discovered { /// they generated mutants or not). pub fn walk_tree( workspace_dir: &Utf8Path, - packages: &[Package], + packages: &[Arc], options: &Options, console: &Console, ) -> Result { diff --git a/src/workspace.rs b/src/workspace.rs index a4459fd5..33594003 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -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}; @@ -67,7 +68,7 @@ impl PackageFilter { /// A cargo workspace. pub struct Workspace { metadata: cargo_metadata::Metadata, - packages: Vec, + packages: Vec>, } impl fmt::Debug for Workspace { @@ -110,17 +111,18 @@ impl Workspace { Ok(Workspace { metadata, packages }) } - pub fn packages_by_name>(&self, names: &[S]) -> Vec { + pub fn packages_by_name>(&self, names: &[S]) -> Vec> { 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() } @@ -166,7 +168,7 @@ impl Workspace { } } - fn expand_selection(&self, selection: PackageSelection) -> Vec { + fn expand_selection(&self, selection: PackageSelection) -> Vec> { match selection { PackageSelection::All => self.packages.clone(), PackageSelection::Explicit(packages) => packages,