Skip to content

Commit

Permalink
feat: Support passing workspace to --manifest-path
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
NobodyXu committed Aug 3, 2023
1 parent b4cf580 commit afbb8b9
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 31 deletions.
24 changes: 16 additions & 8 deletions crates/binstalk/src/helpers/cargo_toml_workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Path>,
crate_name: impl AsRef<str>,
Expand Down Expand Up @@ -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::<Meta>::from_path_with_metadata(&p)?;
let mut manifest_paths = vec![manifest_path];

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);
debug!(
"Loading from {}, manifest.package.name = {:#?}",
p.display(),
manifest_path.display(),
name
);

Expand All @@ -102,13 +108,15 @@ fn load_manifest_from_workspace_inner(
.map(|pat| Pattern::new(&pat))
.collect::<Result<Vec<_>, _>>()?;

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"));
}
}
}
Expand Down
43 changes: 23 additions & 20 deletions crates/binstalk/src/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
cargo_toml_workspace::load_manifest_from_workspace, download::ExtractedFiles, git,
remote::Client, target_triple::TargetTriple,
},
manifests::cargo_toml_binstall::{Meta, PkgMeta, PkgOverride},
ops::{CargoTomlFetchOverride, Options},
};
Expand Down Expand Up @@ -363,17 +366,22 @@ 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();
let name = name.clone();

spawn_blocking(move || {
let dir = TempDir::new()?;
helpers::git::Repository::shallow_clone(git_url, dir.as_ref())?;
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??
}
Expand Down Expand Up @@ -448,29 +456,24 @@ impl PackageInfo {
}

/// 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,
name: N,
) -> Result<Manifest<Meta>, 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<Manifest<Meta>, 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::<Meta>::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())
}
8 changes: 5 additions & 3 deletions crates/binstalk/tests/parse-meta.rs
Original file line number Diff line number Diff line change
@@ -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();

Expand Down

0 comments on commit afbb8b9

Please sign in to comment.