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 <Jiahao_XU@outlook.com>
This commit is contained in:
Jiahao XU 2023-08-04 07:12:06 +10:00 committed by GitHub
parent b4cf580e4f
commit aeacebcf83
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 43 additions and 30 deletions

View file

@ -16,7 +16,8 @@ use crate::{errors::BinstallError, manifests::cargo_toml_binstall::Meta};
/// ///
/// WARNING: This is a blocking operation. /// 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( pub fn load_manifest_from_workspace(
workspace_path: impl AsRef<Path>, workspace_path: impl AsRef<Path>,
crate_name: impl AsRef<str>, crate_name: impl AsRef<str>,
@ -72,16 +73,21 @@ fn load_manifest_from_workspace_inner(
workspace_path.display() 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 mut manifest_paths = vec![manifest_path];
let p = workspace_path.join("Cargo.toml");
let manifest = Manifest::<Meta>::from_path_with_metadata(&p)?; while let Some(manifest_path) = manifest_paths.pop() {
let manifest = Manifest::<Meta>::from_path_with_metadata(&manifest_path)?;
let name = manifest.package.as_ref().map(|p| &*p.name); let name = manifest.package.as_ref().map(|p| &*p.name);
debug!( debug!(
"Loading from {}, manifest.package.name = {:#?}", "Loading from {}, manifest.package.name = {:#?}",
p.display(), manifest_path.display(),
name name
); );
@ -102,13 +108,15 @@ fn load_manifest_from_workspace_inner(
.map(|pat| Pattern::new(&pat)) .map(|pat| Pattern::new(&pat))
.collect::<Result<Vec<_>, _>>()?; .collect::<Result<Vec<_>, _>>()?;
let workspace_path = manifest_path.parent().unwrap();
for member in members { 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 if !exclude_patterns
.iter() .iter()
.any(|exclude| exclude.matches_with_trailing(&path)) .any(|exclude| exclude.matches_with_trailing(&path))
{ {
workspace_paths.push(workspace_path.join(path)); manifest_paths.push(workspace_path.join(path).join("Cargo.toml"));
} }
} }
} }

View file

@ -14,14 +14,17 @@ use leon::Template;
use maybe_owned::MaybeOwned; use maybe_owned::MaybeOwned;
use semver::{Version, VersionReq}; use semver::{Version, VersionReq};
use tempfile::TempDir; use tempfile::TempDir;
use tokio::task::{block_in_place, spawn_blocking}; use tokio::task::spawn_blocking;
use tracing::{debug, info, instrument, warn}; use tracing::{debug, info, instrument, warn};
use crate::{ use crate::{
bins, bins,
errors::{BinstallError, VersionParseError}, errors::{BinstallError, VersionParseError},
fetchers::{Data, Fetcher, TargetData}, 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}, manifests::cargo_toml_binstall::{Meta, PkgMeta, PkgOverride},
ops::{CargoTomlFetchOverride, Options}, ops::{CargoTomlFetchOverride, Options},
}; };
@ -363,7 +366,12 @@ impl PackageInfo {
// Fetch crate via crates.io, git, or use a local manifest path // Fetch crate via crates.io, git, or use a local manifest path
let manifest = match opts.cargo_toml_fetch_override.as_ref() { 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")] #[cfg(feature = "git")]
Some(Git(git_url)) => { Some(Git(git_url)) => {
let git_url = git_url.clone(); let git_url = git_url.clone();
@ -373,7 +381,7 @@ impl PackageInfo {
let dir = TempDir::new()?; let dir = TempDir::new()?;
helpers::git::Repository::shallow_clone(git_url, dir.as_ref())?; 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?? .await??
} }
@ -448,29 +456,24 @@ impl PackageInfo {
} }
/// Load binstall metadata from the crate `Cargo.toml` at the provided path /// Load binstall metadata from the crate `Cargo.toml` at the provided path
pub fn load_manifest_path<P: AsRef<Path>>( ///
/// This is a blocking function.
pub fn load_manifest_path<P: AsRef<Path>, N: AsRef<str>>(
manifest_path: P, manifest_path: P,
name: N,
) -> Result<Manifest<Meta>, BinstallError> { ) -> Result<Manifest<Meta>, BinstallError> {
let manifest_path = manifest_path.as_ref(); fn inner(manifest_path: &Path, crate_name: &str) -> Result<Manifest<Meta>, BinstallError> {
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);
};
debug!( debug!(
"Reading manifest at local path: {}", "Reading crate {crate_name} manifest at local path: {}",
manifest_path.display() manifest_path.display()
); );
// Load and parse manifest (this checks file system for binary output names) // Load and parse manifest (this checks file system for binary output names)
let manifest = Manifest::<Meta>::from_path_with_metadata(manifest_path)?; let manifest = load_manifest_from_workspace(manifest_path, crate_name)?;
// Return metadata // Return metadata
Ok(manifest) Ok(manifest)
}) }
inner(manifest_path.as_ref(), name.as_ref())
} }

View file

@ -1,12 +1,14 @@
use binstalk::ops::resolve::load_manifest_path; use binstalk::ops::resolve::load_manifest_path;
use cargo_toml::Product; use cargo_toml::Product;
use std::path::PathBuf;
#[test] #[test]
fn parse_meta() { fn parse_meta() {
let mut manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); let mut manifest_dir = PathBuf::from(std::env::var_os("CARGO_MANIFEST_DIR").unwrap());
manifest_dir.push_str("/tests/parse-meta.Cargo.toml"); 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 package = manifest.package.unwrap();
let meta = package.metadata.and_then(|m| m.binstall).unwrap(); let meta = package.metadata.and_then(|m| m.binstall).unwrap();