From aeacebcf83c85e3aa9bdd766363b20145b3fd86b Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 4 Aug 2023 07:12:06 +1000 Subject: [PATCH] feat: Support passing workspace to `--manifest-path` (#1246) Previously it will load the root `Cargo.toml` and treat it as the manifest for the crate, now it will check its `package.name` and would search for the workspace if the `package.name` does not match the crate name. Signed-off-by: Jiahao XU --- .../src/helpers/cargo_toml_workspace.rs | 24 +++++++---- crates/binstalk/src/ops/resolve.rs | 41 ++++++++++--------- crates/binstalk/tests/parse-meta.rs | 8 ++-- 3 files changed, 43 insertions(+), 30 deletions(-) diff --git a/crates/binstalk/src/helpers/cargo_toml_workspace.rs b/crates/binstalk/src/helpers/cargo_toml_workspace.rs index 3360e55e..61f9d293 100644 --- a/crates/binstalk/src/helpers/cargo_toml_workspace.rs +++ b/crates/binstalk/src/helpers/cargo_toml_workspace.rs @@ -16,7 +16,8 @@ use crate::{errors::BinstallError, manifests::cargo_toml_binstall::Meta}; /// /// WARNING: This is a blocking operation. /// -/// * `workspace_path` - should be a directory +/// * `workspace_path` - can be a directory (path to workspace) or +/// a file (path to `Cargo.toml`). pub fn load_manifest_from_workspace( workspace_path: impl AsRef, crate_name: impl AsRef, @@ -72,16 +73,21 @@ fn load_manifest_from_workspace_inner( workspace_path.display() ); - let mut workspace_paths = vec![workspace_path.to_owned()]; + let manifest_path = if workspace_path.is_file() { + workspace_path.to_owned() + } else { + workspace_path.join("Cargo.toml") + }; - while let Some(workspace_path) = workspace_paths.pop() { - let p = workspace_path.join("Cargo.toml"); - let manifest = Manifest::::from_path_with_metadata(&p)?; + let mut manifest_paths = vec![manifest_path]; + + while let Some(manifest_path) = manifest_paths.pop() { + let manifest = Manifest::::from_path_with_metadata(&manifest_path)?; let name = manifest.package.as_ref().map(|p| &*p.name); debug!( "Loading from {}, manifest.package.name = {:#?}", - p.display(), + manifest_path.display(), name ); @@ -102,13 +108,15 @@ fn load_manifest_from_workspace_inner( .map(|pat| Pattern::new(&pat)) .collect::, _>>()?; + let workspace_path = manifest_path.parent().unwrap(); + for member in members { - for path in Pattern::new(&member)?.glob_dirs(&workspace_path)? { + for path in Pattern::new(&member)?.glob_dirs(workspace_path)? { if !exclude_patterns .iter() .any(|exclude| exclude.matches_with_trailing(&path)) { - workspace_paths.push(workspace_path.join(path)); + manifest_paths.push(workspace_path.join(path).join("Cargo.toml")); } } } diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs index 47b1d359..84abca11 100644 --- a/crates/binstalk/src/ops/resolve.rs +++ b/crates/binstalk/src/ops/resolve.rs @@ -14,14 +14,17 @@ use leon::Template; use maybe_owned::MaybeOwned; use semver::{Version, VersionReq}; use tempfile::TempDir; -use tokio::task::{block_in_place, spawn_blocking}; +use tokio::task::spawn_blocking; use tracing::{debug, info, instrument, warn}; use crate::{ bins, errors::{BinstallError, VersionParseError}, fetchers::{Data, Fetcher, TargetData}, - helpers::{self, download::ExtractedFiles, remote::Client, target_triple::TargetTriple}, + helpers::{ + self, cargo_toml_workspace::load_manifest_from_workspace, download::ExtractedFiles, + remote::Client, target_triple::TargetTriple, + }, manifests::cargo_toml_binstall::{Meta, PkgMeta, PkgOverride}, ops::{CargoTomlFetchOverride, Options}, }; @@ -363,7 +366,12 @@ impl PackageInfo { // Fetch crate via crates.io, git, or use a local manifest path let manifest = match opts.cargo_toml_fetch_override.as_ref() { - Some(Path(manifest_path)) => load_manifest_path(manifest_path)?, + Some(Path(manifest_path)) => { + let manifest_path = manifest_path.clone(); + let name = name.clone(); + + spawn_blocking(move || load_manifest_path(manifest_path, &name)).await?? + } #[cfg(feature = "git")] Some(Git(git_url)) => { let git_url = git_url.clone(); @@ -373,7 +381,7 @@ impl PackageInfo { let dir = TempDir::new()?; helpers::git::Repository::shallow_clone(git_url, dir.as_ref())?; - helpers::cargo_toml_workspace::load_manifest_from_workspace(dir.as_ref(), &name) + load_manifest_from_workspace(dir.as_ref(), &name) }) .await?? } @@ -448,29 +456,24 @@ impl PackageInfo { } /// Load binstall metadata from the crate `Cargo.toml` at the provided path -pub fn load_manifest_path>( +/// +/// This is a blocking function. +pub fn load_manifest_path, N: AsRef>( manifest_path: P, + name: N, ) -> Result, BinstallError> { - let manifest_path = manifest_path.as_ref(); - - block_in_place(|| { - let manifest_path = if manifest_path.is_dir() { - Cow::Owned(manifest_path.join("Cargo.toml")) - } else if manifest_path.is_file() { - Cow::Borrowed(manifest_path) - } else { - return Err(BinstallError::CargoManifestPath); - }; - + fn inner(manifest_path: &Path, crate_name: &str) -> Result, BinstallError> { debug!( - "Reading manifest at local path: {}", + "Reading crate {crate_name} manifest at local path: {}", manifest_path.display() ); // Load and parse manifest (this checks file system for binary output names) - let manifest = Manifest::::from_path_with_metadata(manifest_path)?; + let manifest = load_manifest_from_workspace(manifest_path, crate_name)?; // Return metadata Ok(manifest) - }) + } + + inner(manifest_path.as_ref(), name.as_ref()) } diff --git a/crates/binstalk/tests/parse-meta.rs b/crates/binstalk/tests/parse-meta.rs index a76bedf5..4443bd57 100644 --- a/crates/binstalk/tests/parse-meta.rs +++ b/crates/binstalk/tests/parse-meta.rs @@ -1,12 +1,14 @@ use binstalk::ops::resolve::load_manifest_path; use cargo_toml::Product; +use std::path::PathBuf; #[test] fn parse_meta() { - let mut manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); - manifest_dir.push_str("/tests/parse-meta.Cargo.toml"); + let mut manifest_dir = PathBuf::from(std::env::var_os("CARGO_MANIFEST_DIR").unwrap()); + manifest_dir.push("tests/parse-meta.Cargo.toml"); - let manifest = load_manifest_path(&manifest_dir).expect("Error parsing metadata"); + let manifest = + load_manifest_path(&manifest_dir, "cargo-binstall-test").expect("Error parsing metadata"); let package = manifest.package.unwrap(); let meta = package.metadata.and_then(|m| m.binstall).unwrap();