diff --git a/cargo-insta/Cargo.lock b/cargo-insta/Cargo.lock index 80ff5ae9..3422c5a4 100644 --- a/cargo-insta/Cargo.lock +++ b/cargo-insta/Cargo.lock @@ -35,10 +35,20 @@ dependencies = [ "memchr", ] +[[package]] +name = "camino" +version = "1.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c59e92b5a388f549b863a7bea62612c09f24c8393560709a54558a9abdfb3b9c" +dependencies = [ + "serde", +] + [[package]] name = "cargo-insta" version = "1.33.0" dependencies = [ + "cargo_metadata", "console", "ignore", "insta", @@ -51,6 +61,29 @@ dependencies = [ "uuid", ] +[[package]] +name = "cargo-platform" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2cfa25e60aea747ec7e1124f238816749faa93759c6ff5b31f1ccdda137f4479" +dependencies = [ + "serde", +] + +[[package]] +name = "cargo_metadata" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb9ac64500cc83ce4b9f8dafa78186aa008c8dea77a09b94cd307fd0cd5022a8" +dependencies = [ + "camino", + "cargo-platform", + "semver", + "serde", + "serde_json", + "thiserror", +] + [[package]] name = "cc" version = "1.0.79" @@ -461,6 +494,15 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "semver" +version = "1.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ad977052201c6de01a8ef2aa3378c4bd23217a056337d1d6da40468d267a4fb0" +dependencies = [ + "serde", +] + [[package]] name = "serde" version = "1.0.144" diff --git a/cargo-insta/Cargo.toml b/cargo-insta/Cargo.toml index edd9f68d..3eff7ae4 100644 --- a/cargo-insta/Cargo.toml +++ b/cargo-insta/Cargo.toml @@ -13,6 +13,7 @@ readme = "README.md" [dependencies] insta = { version = "=1.33.0", path = "..", features = ["json", "yaml", "redactions", "_cargo_insta_internal"] } +cargo_metadata = { version = "0.18.0", default-features = false } console = "0.15.4" structopt = { version = "0.3.26", default-features = false } serde = { version = "1.0.117", features = ["derive"] } diff --git a/cargo-insta/src/cargo.rs b/cargo-insta/src/cargo.rs index 9f8c713a..3e12b5e2 100644 --- a/cargo-insta/src/cargo.rs +++ b/cargo-insta/src/cargo.rs @@ -1,190 +1,78 @@ -use std::collections::HashSet; -use std::env; use std::error::Error; -use std::fs; use std::path::{Path, PathBuf}; -use std::process; -use serde::Deserialize; +pub(crate) use cargo_metadata::{Metadata, Package}; use crate::utils::err_msg; -#[derive(Deserialize, Clone, Debug)] -struct Target { - src_path: PathBuf, - kind: HashSet, -} - -#[derive(Deserialize, Clone, Debug)] -pub(crate) struct Package { - name: String, - version: String, - id: String, - manifest_path: PathBuf, - targets: Vec, -} - -#[derive(Deserialize, Debug)] -pub(crate) struct Metadata { - packages: Vec, - workspace_members: Vec, - workspace_root: String, -} +pub(crate) fn find_snapshot_roots(package: &Package) -> Vec { + let mut roots = Vec::new(); -impl Metadata { - pub(crate) fn workspace_root(&self) -> &Path { - Path::new(&self.workspace_root) + // the manifest path's parent is always a snapshot container. For + // a rationale see GH-70. But generally a user would expect to be + // able to put a snapshot into foo/snapshots instead of foo/src/snapshots. + if let Some(manifest) = package.manifest_path.parent() { + roots.push(manifest.as_std_path().to_path_buf()); } -} - -#[derive(Deserialize, Debug)] -struct ProjectLocation { - root: PathBuf, -} - -impl Package { - pub(crate) fn manifest_path(&self) -> &Path { - &self.manifest_path - } - - pub(crate) fn name(&self) -> &str { - &self.name - } - - pub(crate) fn version(&self) -> &str { - &self.version - } - - pub(crate) fn find_snapshot_roots(&self) -> Vec { - let mut roots = Vec::new(); - // the manifest path's parent is always a snapshot container. For - // a rationale see GH-70. But generally a user would expect to be - // able to put a snapshot into foo/snapshots instead of foo/src/snapshots. - if let Some(manifest) = self.manifest_path.parent() { - roots.push(manifest.to_path_buf()); + // additionally check all targets. + for target in &package.targets { + // custom build scripts we can safely skip over. In the past this + // caused issues with duplicate paths but that's resolved in other + // ways now. We do not want to pick up snapshots in such places + // though. + if target.kind.iter().any(|kind| kind == "custom-build") { + continue; } - // additionally check all targets. - for target in &self.targets { - // custom build scripts we can safely skip over. In the past this - // caused issues with duplicate paths but that's resolved in other - // ways now. We do not want to pick up snapshots in such places - // though. - if target.kind.contains("custom-build") { - continue; - } - - // this gives us the containing source folder. Typically this is - // something like crate/src. - let root = target.src_path.parent().unwrap(); - roots.push(root.to_path_buf()); - } + // this gives us the containing source folder. Typically this is + // something like crate/src. + let root = target.src_path.parent().unwrap().as_std_path(); + roots.push(root.to_path_buf()); + } - // reduce roots to avoid traversing into paths twice. If we have both - // /foo and /foo/bar as roots we would only walk into /foo. Otherwise - // we would encounter paths twice. If we don't skip them here we run - // into issues where the existence of a build script causes a snapshot - // to be picked up twice since the same path is determined. (GH-15) - roots.sort_by_key(|x| x.as_os_str().len()); - let mut reduced_roots = vec![]; - for root in roots { - if !reduced_roots.iter().any(|x| root.starts_with(x)) { - reduced_roots.push(root); - } + // reduce roots to avoid traversing into paths twice. If we have both + // /foo and /foo/bar as roots we would only walk into /foo. Otherwise + // we would encounter paths twice. If we don't skip them here we run + // into issues where the existence of a build script causes a snapshot + // to be picked up twice since the same path is determined. (GH-15) + roots.sort_by_key(|x| x.as_os_str().len()); + let mut reduced_roots = vec![]; + for root in roots { + if !reduced_roots.iter().any(|x| root.starts_with(x)) { + reduced_roots.push(root); } - - reduced_roots } -} -pub(crate) fn get_cargo() -> String { - env::var("CARGO") - .ok() - .unwrap_or_else(|| "cargo".to_string()) + reduced_roots } pub(crate) fn get_package_metadata( manifest_path: Option<&Path>, -) -> Result> { - let mut cmd = process::Command::new(get_cargo()); - cmd.arg("metadata") - .arg("--no-deps") - .arg("--format-version=1"); +) -> cargo_metadata::Result { + let mut cmd = cargo_metadata::MetadataCommand::new(); if let Some(manifest_path) = manifest_path { - if !fs::metadata(manifest_path) - .ok() - .map_or(false, |x| x.is_file()) - { - return Err(err_msg( - "the manifest-path must be a path to a Cargo.toml file", - )); - } - cmd.arg("--manifest-path").arg(manifest_path.as_os_str()); + cmd.manifest_path(manifest_path); } - let output = cmd.output()?; - if !output.status.success() { - let msg = String::from_utf8_lossy(&output.stderr); - return Err(err_msg(format!( - "cargo erroried getting metadata: {}", - msg.trim() - ))); - } - Ok(serde_json::from_slice(&output.stdout)?) + cmd.exec() } -fn get_default_manifest() -> Result, Box> { - let output = process::Command::new(get_cargo()) - .arg("locate-project") - .output()?; - if output.status.success() { - let loc: ProjectLocation = serde_json::from_slice(&output.stdout)?; - Ok(Some(loc.root)) - } else { - Ok(None) - } -} - -fn find_all_packages(metadata: &Metadata) -> Vec { - metadata - .packages - .iter() - .filter_map(|package| { - if metadata.workspace_members.contains(&package.id) { - Some(package.clone()) - } else { - None - } - }) - .collect() -} - -pub(crate) fn find_packages( - metadata: &Metadata, - all: bool, -) -> Result, Box> { +pub(crate) fn find_packages(metadata: Metadata, all: bool) -> Result, Box> { + let Metadata { + packages, + workspace_members, + resolve, + .. + } = metadata; if all { - Ok(find_all_packages(metadata)) + let mut packages = packages; + packages.retain(|Package { id, .. }| workspace_members.contains(id)); + Ok(packages) } else { - let default_manifest = get_default_manifest()? - .ok_or_else(|| { - err_msg( - "Could not find `Cargo.toml` in the current folder or any parent directory.", - ) - })? - .canonicalize()?; - let mut rv = vec![]; - for package in &metadata.packages { - if package.manifest_path.canonicalize()? == default_manifest { - rv.push(package.clone()); - } - } - if rv.is_empty() { - // if we don't find anything we're in a workspace root that has no - // root member in which case --all is implied. - Ok(find_all_packages(metadata)) - } else { - Ok(rv) - } + let root = resolve + .and_then(|cargo_metadata::Resolve { root, .. }| root) + .and_then(|root| packages.into_iter().find(|Package { id, .. }| *id == root)) + .ok_or_else(|| err_msg("could not find root package. Try running with --all"))?; + Ok(vec![root]) } } diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 67fe8fc6..a53357be 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -1,6 +1,7 @@ use std::borrow::{Borrow, Cow}; use std::collections::HashSet; use std::error::Error; +use std::fmt::Display; use std::path::{Path, PathBuf}; use std::{env, fs}; use std::{io, process}; @@ -15,7 +16,7 @@ use structopt::clap::AppSettings; use structopt::StructOpt; use uuid::Uuid; -use crate::cargo::{find_packages, get_cargo, get_package_metadata, Package}; +use crate::cargo::{find_packages, find_snapshot_roots, get_package_metadata, Package}; use crate::container::{Operation, SnapshotContainer}; use crate::utils::{err_msg, QuietExit}; use crate::walk::{find_snapshots, make_deletion_walker, make_snapshot_walker, FindFlags}; @@ -240,10 +241,10 @@ fn query_snapshot( ) -> Result> { loop { term.clear_screen()?; - let (pkg_name, pkg_version) = if let Some(pkg) = pkg { - (pkg.name(), pkg.version()) + let (pkg_name, pkg_version): (_, &dyn Display) = if let Some(pkg) = pkg { + (pkg.name.as_str(), &pkg.version) } else { - ("unknown package", "unknown version") + ("unknown package", &"unknown version") }; println!( @@ -396,10 +397,11 @@ fn handle_target_args(target_args: &TargetArgs) -> Result, Box< }) } else { let metadata = get_package_metadata(manifest_path.as_ref().map(|x| x.as_path()))?; - let packages = find_packages(&metadata, target_args.all || target_args.workspace)?; - let tool_config = ToolConfig::from_workspace(metadata.workspace_root())?; + let workspace_root = metadata.workspace_root.as_std_path().to_path_buf(); + let packages = find_packages(metadata, target_args.all || target_args.workspace)?; + let tool_config = ToolConfig::from_workspace(&workspace_root)?; Ok(LocationInfo { - workspace_root: metadata.workspace_root().to_path_buf(), + workspace_root, packages: Some(packages), exts, find_flags: get_find_flags(&tool_config, target_args), @@ -421,7 +423,7 @@ fn load_snapshot_containers<'a>( let mut snapshot_containers = vec![]; if let Some(ref packages) = loc.packages { for package in packages.iter() { - for root in package.find_snapshot_roots() { + for root in find_snapshot_roots(package) { roots.insert(root.clone()); for snapshot_container in find_snapshots(&root, &loc.exts, loc.find_flags) { snapshot_containers.push((snapshot_container?, Some(package))); @@ -776,14 +778,18 @@ fn prepare_test_runner<'snapshot_ref>( extra_args: &[&str], snapshot_ref_file: Option<&'snapshot_ref Path>, ) -> Result<(process::Command, Option>, bool), Box> { + let cargo = env::var_os("CARGO"); + let cargo = cargo + .as_deref() + .unwrap_or_else(|| std::ffi::OsStr::new("cargo")); let mut proc = match test_runner { TestRunner::CargoTest | TestRunner::Auto => { - let mut proc = process::Command::new(get_cargo()); + let mut proc = process::Command::new(cargo); proc.arg("test"); proc } TestRunner::Nextest => { - let mut proc = process::Command::new(get_cargo()); + let mut proc = process::Command::new(cargo); proc.arg("nextest"); proc.arg("run"); proc diff --git a/cargo-insta/src/walk.rs b/cargo-insta/src/walk.rs index d11964ce..08b3a0ac 100644 --- a/cargo-insta/src/walk.rs +++ b/cargo-insta/src/walk.rs @@ -95,11 +95,11 @@ pub(crate) fn make_deletion_walker( .filter_map(|x| { // filter out packages we did not ask for. if let Some(only_package) = selected_package { - if x.name() != only_package { + if x.name != only_package { return None; } } - x.manifest_path().parent().unwrap().canonicalize().ok() + x.manifest_path.parent().unwrap().canonicalize().ok() }) .collect() } else {