-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support glob syntax in workspace members #3979
Changes from 4 commits
b3a747c
229627a
06a1371
9817033
ea2df45
f00c223
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ use std::collections::BTreeMap; | |
use std::path::{Path, PathBuf}; | ||
use std::slice; | ||
|
||
use glob::glob; | ||
|
||
use core::{Package, VirtualManifest, EitherManifest, SourceId}; | ||
use core::{PackageIdSpec, Dependency, Profile, Profiles}; | ||
use ops; | ||
|
@@ -316,9 +318,24 @@ impl<'cfg> Workspace<'cfg> { | |
}; | ||
|
||
if let Some(list) = members { | ||
let root = root_manifest.parent().unwrap(); | ||
|
||
let mut expanded_list = Vec::new(); | ||
for path in list { | ||
let root = root_manifest.parent().unwrap(); | ||
let manifest_path = root.join(path).join("Cargo.toml"); | ||
let pathbuf = root.join(path); | ||
let expanded_paths = expand_member_path(&pathbuf)?; | ||
|
||
// If glob does not find any valid paths, then put the original | ||
// path in the expanded list to maintain backwards compatibility. | ||
if expanded_paths.is_empty() { | ||
expanded_list.push(pathbuf); | ||
} else { | ||
expanded_list.extend(expanded_paths); | ||
} | ||
} | ||
|
||
for path in expanded_list { | ||
let manifest_path = path.join("Cargo.toml"); | ||
self.find_path_deps(&manifest_path, &root_manifest, false)?; | ||
} | ||
} | ||
|
@@ -527,6 +544,18 @@ impl<'cfg> Workspace<'cfg> { | |
} | ||
} | ||
|
||
fn expand_member_path(path: &Path) -> CargoResult<Vec<PathBuf>> { | ||
let path = path.to_str().unwrap(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
let res = glob(path).map_err(|e| { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I may be missing something here, but I don't see how |
||
human(format!("could not parse pattern `{}`: {}", &path, e)) | ||
})?; | ||
res.map(|p| { | ||
p.or_else(|e| { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, could this use |
||
Err(human(format!("unable to match path to pattern `{}`: {}", &path, e))) | ||
}) | ||
}).collect() | ||
} | ||
|
||
fn is_excluded(members: &Option<Vec<String>>, | ||
exclude: &[String], | ||
root_path: &Path, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this error if there's a path without
Cargo.toml
matched by the glob? I would say that it's OK to bail in this case, but that just skipping that path would be probably more useful: I often have various assorted folders alongside the workspace packages.In any case, I think a test case for this would be useful :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call out. I will get on it.