Skip to content

Commit

Permalink
cargo-insta: fix --manifest-path <virtual-workspace> (#409)
Browse files Browse the repository at this point in the history 
Prior to this change --manifest-path only worked properly if it pointed
to the root of a non-virtual workspace.

Rewrite all the cargo interaction to use cargo-metadata which crucially
provides `Metadata::resolve::root` which is used to determine which
package --manifest-path was pointing to.

https://doc.rust-lang.org/cargo/reference/workspaces.html#virtual-workspace
  • Loading branch information
@tamird
tamird authored Oct 7, 2023
1 parent 9623b3a commit 2e8ceba
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 186 deletions.
42 changes: 42 additions & 0 deletions cargo-insta/Cargo.lock
View file

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

1 change: 1 addition & 0 deletions cargo-insta/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
220 changes: 53 additions & 167 deletions cargo-insta/src/cargo.rs
View file
Original file line number Diff line number Diff line change
@@ -1,190 +1,76 @@
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, MetadataCommand, Package};

use crate::utils::err_msg;
pub(crate) fn find_snapshot_roots(package: &Package) -> Vec<PathBuf> {
let mut roots = Vec::new();

#[derive(Deserialize, Clone, Debug)]
struct Target {
src_path: PathBuf,
kind: HashSet<String>,
}

#[derive(Deserialize, Clone, Debug)]
pub(crate) struct Package {
name: String,
version: String,
id: String,
manifest_path: PathBuf,
targets: Vec<Target>,
}

#[derive(Deserialize, Debug)]
pub(crate) struct Metadata {
packages: Vec<Package>,
workspace_members: Vec<String>,
workspace_root: String,
}

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<PathBuf> {
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(
pub(crate) fn get_metadata(
manifest_path: Option<&Path>,
all: bool,
) -> Result<Metadata, Box<dyn Error>> {
let mut cmd = process::Command::new(get_cargo());
cmd.arg("metadata")
.arg("--no-deps")
.arg("--format-version=1");
let mut cmd = 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());
}
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()
)));
cmd.manifest_path(manifest_path);
}
Ok(serde_json::from_slice(&output.stdout)?)
}

fn get_default_manifest() -> Result<Option<PathBuf>, Box<dyn Error>> {
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<Package> {
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<Vec<Package>, Box<dyn Error>> {
if all {
Ok(find_all_packages(metadata))
} 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)
cmd.no_deps();
}
let mut metadata = cmd.exec()?;
let Metadata {
packages,
workspace_members,
resolve,
..
} = &mut metadata;
match resolve
.as_ref()
.and_then(|cargo_metadata::Resolve { root, .. }| root.as_ref())
{
Some(root) => packages.retain(|Package { id, .. }| id == root),
None => {
packages.retain(|Package { id, .. }| workspace_members.contains(id));
}
}
Ok(metadata)
}
Loading

0 comments on commit 2e8ceba

Please sign in to comment.