Skip to content
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 patterns for package/target selection #8752

Merged
merged 22 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
be31989
test: minimal shell quote supoort for cargo-test-support
weihanglo Oct 4, 2020
42696ae
feat: glob support for target selection
weihanglo Oct 4, 2020
633e5f0
test: glob support for target selection
weihanglo Oct 4, 2020
2361fb0
feat: glob support for package selection
weihanglo Oct 4, 2020
ab88c48
test(tree): glob support for package selection
weihanglo Oct 4, 2020
db313e5
test(rustc): glob support for package selection
weihanglo Oct 4, 2020
570aea7
test(rustdoc): glob support for package selection
weihanglo Oct 4, 2020
f1de239
test(doc): glob support for package selection
weihanglo Oct 4, 2020
a06ec88
test(bench): glob support for package selection
weihanglo Oct 4, 2020
33d883c
test(run): glob support for package selection
weihanglo Oct 4, 2020
667a5ae
test(build): glob support for package selection
weihanglo Oct 4, 2020
8d5e11a
test(check): glob support for package selection
weihanglo Oct 4, 2020
6fb6b94
test(test): glob support for package selection
weihanglo Oct 4, 2020
b3441f9
doc: glob pattern support for package/target selection
weihanglo Oct 4, 2020
4a61d8a
doc: build-man.sh for glob pattern support
weihanglo Oct 4, 2020
5e3dc46
test: be consistent on error message styles
weihanglo Oct 9, 2020
0f1534c
refactor: simplify match -> if let
weihanglo Oct 9, 2020
8f0664f
test: normalize raw string indentation.
weihanglo Oct 9, 2020
e5007de
test: use `with_stderr_unordered`
weihanglo Oct 10, 2020
e4a1794
fix: emit errors instead of warnings when glob patterns not found
weihanglo Oct 10, 2020
3d04268
style: cargo fmt
weihanglo Oct 10, 2020
d613cd6
Merge branch 'master' into feat/glob-pattern
weihanglo Oct 18, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions crates/cargo-test-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1638,8 +1638,12 @@ impl ChannelChanger for cargo::util::ProcessBuilder {
}

fn split_and_add_args(p: &mut ProcessBuilder, s: &str) {
for arg in s.split_whitespace() {
if arg.contains('"') || arg.contains('\'') {
for mut arg in s.split_whitespace() {
if (arg.starts_with('"') && arg.ends_with('"'))
|| (arg.starts_with('\'') && arg.ends_with('\''))
{
arg = &arg[1..(arg.len() - 1).max(1)];
} else if arg.contains(&['"', '\''][..]) {
panic!("shell-style argument parsing is not supported")
}
p.arg(arg);
Expand Down
14 changes: 13 additions & 1 deletion src/bin/cargo/commands/run.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::command_prelude::*;
use crate::util::restricted_names::is_glob_pattern;
use crate::util::ProcessError;
use cargo::core::Verbosity;
use cargo::ops::{self, CompileFilter};
use cargo::ops::{self, CompileFilter, Packages};

pub fn cli() -> App {
subcommand("run")
Expand Down Expand Up @@ -38,6 +39,17 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
ProfileChecking::Checked,
)?;

// Disallow `spec` to be an glob pattern
if let Packages::Packages(opt_in) = &compile_opts.spec {
if let Some(pattern) = opt_in.iter().find(|s| is_glob_pattern(s)) {
return Err(anyhow::anyhow!(
"`cargo run` does not support glob pattern `{}` on package selection",
pattern,
)
.into());
}
}

if !args.is_present("example") && !args.is_present("bin") {
let default_runs: Vec<_> = compile_opts
.spec
Expand Down
215 changes: 175 additions & 40 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

use std::collections::{BTreeSet, HashMap, HashSet};
use std::hash::{Hash, Hasher};
use std::iter::FromIterator;
use std::sync::Arc;

use crate::core::compiler::standard_lib;
Expand All @@ -41,8 +40,11 @@ use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace};
use crate::ops;
use crate::ops::resolve::WorkspaceResolve;
use crate::util::config::Config;
use crate::util::restricted_names::is_glob_pattern;
use crate::util::{closest_msg, profile, CargoResult, StableHasher};

use anyhow::Context as _;

/// Contains information about how a package should be compiled.
///
/// Note on distinction between `CompileOptions` and `BuildConfig`:
Expand Down Expand Up @@ -116,6 +118,7 @@ impl Packages {
})
}

/// Converts selected packages from a workspace to `PackageIdSpec`s.
pub fn to_package_id_specs(&self, ws: &Workspace<'_>) -> CargoResult<Vec<PackageIdSpec>> {
let specs = match self {
Packages::All => ws
Expand All @@ -124,33 +127,40 @@ impl Packages {
.map(PackageIdSpec::from_package_id)
.collect(),
Packages::OptOut(opt_out) => {
let mut opt_out = BTreeSet::from_iter(opt_out.iter().cloned());
let packages = ws
let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?;
let specs = ws
.members()
.filter(|pkg| !opt_out.remove(pkg.name().as_str()))
.filter(|pkg| {
!names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns)
})
.map(Package::package_id)
.map(PackageIdSpec::from_package_id)
.collect();
if !opt_out.is_empty() {
ws.config().shell().warn(format!(
"excluded package(s) {} not found in workspace `{}`",
opt_out
.iter()
.map(|x| x.as_ref())
.collect::<Vec<_>>()
.join(", "),
ws.root().display(),
))?;
}
packages
let warn = |e| ws.config().shell().warn(e);
emit_package_not_found(ws, names, true).or_else(warn)?;
emit_pattern_not_found(ws, patterns, true).or_else(warn)?;
specs
}
Packages::Packages(packages) if packages.is_empty() => {
vec![PackageIdSpec::from_package_id(ws.current()?.package_id())]
}
Packages::Packages(packages) => packages
.iter()
.map(|p| PackageIdSpec::parse(p))
.collect::<CargoResult<Vec<_>>>()?,
Packages::Packages(opt_in) => {
let (mut patterns, packages) = opt_patterns_and_names(opt_in)?;
let mut specs = packages
.iter()
.map(|p| PackageIdSpec::parse(p))
.collect::<CargoResult<Vec<_>>>()?;
if !patterns.is_empty() {
let matched_pkgs = ws
.members()
.filter(|pkg| match_patterns(pkg, &mut patterns))
.map(Package::package_id)
.map(PackageIdSpec::from_package_id);
specs.extend(matched_pkgs);
}
emit_pattern_not_found(ws, patterns, false)?;
specs
}
Packages::Default => ws
.default_members()
.map(Package::package_id)
Expand All @@ -170,27 +180,35 @@ impl Packages {
Ok(specs)
}

/// Gets a list of selected packages from a workspace.
pub fn get_packages<'ws>(&self, ws: &'ws Workspace<'_>) -> CargoResult<Vec<&'ws Package>> {
let packages: Vec<_> = match self {
Packages::Default => ws.default_members().collect(),
Packages::All => ws.members().collect(),
Packages::OptOut(opt_out) => ws
.members()
.filter(|pkg| !opt_out.iter().any(|name| pkg.name().as_str() == name))
.collect(),
Packages::Packages(packages) => packages
.iter()
.map(|name| {
ws.members()
.find(|pkg| pkg.name().as_str() == name)
.ok_or_else(|| {
anyhow::format_err!(
"package `{}` is not a member of the workspace",
name
)
})
})
.collect::<CargoResult<Vec<_>>>()?,
Packages::OptOut(opt_out) => {
let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?;
let packages = ws
.members()
.filter(|pkg| {
!names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns)
})
.collect();
emit_package_not_found(ws, names, true)?;
emit_pattern_not_found(ws, patterns, true)?;
packages
}
Packages::Packages(opt_in) => {
let (mut patterns, mut names) = opt_patterns_and_names(opt_in)?;
let packages = ws
.members()
.filter(|pkg| {
names.remove(pkg.name().as_str()) || match_patterns(pkg, &mut patterns)
})
.collect();
emit_package_not_found(ws, names, false)?;
emit_pattern_not_found(ws, patterns, false)?;
packages
}
};
Ok(packages)
}
Expand Down Expand Up @@ -577,6 +595,13 @@ impl FilterRule {
FilterRule::Just(ref targets) => Some(targets.clone()),
}
}

pub(crate) fn contains_glob_patterns(&self) -> bool {
match self {
FilterRule::All => false,
FilterRule::Just(targets) => targets.iter().any(is_glob_pattern),
}
}
}

impl CompileFilter {
Expand Down Expand Up @@ -706,6 +731,24 @@ impl CompileFilter {
CompileFilter::Only { .. } => true,
}
}

pub(crate) fn contains_glob_patterns(&self) -> bool {
match self {
CompileFilter::Default { .. } => false,
CompileFilter::Only {
bins,
examples,
tests,
benches,
..
} => {
bins.contains_glob_patterns()
|| examples.contains_glob_patterns()
|| tests.contains_glob_patterns()
|| benches.contains_glob_patterns()
}
}
}
}

/// A proposed target.
Expand Down Expand Up @@ -1163,8 +1206,16 @@ fn find_named_targets<'a>(
is_expected_kind: fn(&Target) -> bool,
mode: CompileMode,
) -> CargoResult<Vec<Proposal<'a>>> {
let filter = |t: &Target| t.name() == target_name && is_expected_kind(t);
let proposals = filter_targets(packages, filter, true, mode);
let is_glob = is_glob_pattern(target_name);
let proposals = if is_glob {
let pattern = build_glob(target_name)?;
let filter = |t: &Target| is_expected_kind(t) && pattern.matches(t.name());
filter_targets(packages, filter, true, mode)
} else {
let filter = |t: &Target| t.name() == target_name && is_expected_kind(t);
filter_targets(packages, filter, true, mode)
};

if proposals.is_empty() {
let targets = packages.iter().flat_map(|pkg| {
pkg.targets()
Expand All @@ -1173,8 +1224,9 @@ fn find_named_targets<'a>(
});
let suggestion = closest_msg(target_name, targets, |t| t.name());
anyhow::bail!(
"no {} target named `{}`{}",
"no {} target {} `{}`{}",
target_desc,
if is_glob { "matches pattern" } else { "named" },
target_name,
suggestion
);
Expand Down Expand Up @@ -1291,3 +1343,86 @@ fn traverse_and_share(
new_graph.entry(new_unit.clone()).or_insert(new_deps);
new_unit
}

/// Build `glob::Pattern` with informative context.
fn build_glob(pat: &str) -> CargoResult<glob::Pattern> {
glob::Pattern::new(pat).with_context(|| format!("cannot build glob pattern from `{}`", pat))
}

/// Emits "package not found" error.
///
/// > This function should be used only in package selection processes such like
/// `Packages::to_package_id_specs` and `Packages::get_packages`.
fn emit_package_not_found(
ws: &Workspace<'_>,
opt_names: BTreeSet<&str>,
opt_out: bool,
) -> CargoResult<()> {
if !opt_names.is_empty() {
anyhow::bail!(
"{}package(s) `{}` not found in workspace `{}`",
if opt_out { "excluded " } else { "" },
opt_names.into_iter().collect::<Vec<_>>().join(", "),
ws.root().display(),
)
}
Ok(())
}

/// Emits "glob pattern not found" error.
///
/// > This function should be used only in package selection processes such like
/// `Packages::to_package_id_specs` and `Packages::get_packages`.
fn emit_pattern_not_found(
ws: &Workspace<'_>,
opt_patterns: Vec<(glob::Pattern, bool)>,
opt_out: bool,
) -> CargoResult<()> {
let not_matched = opt_patterns
.iter()
.filter(|(_, matched)| !*matched)
.map(|(pat, _)| pat.as_str())
.collect::<Vec<_>>();
if !not_matched.is_empty() {
anyhow::bail!(
"{}package pattern(s) `{}` not found in workspace `{}`",
if opt_out { "excluded " } else { "" },
not_matched.join(", "),
ws.root().display(),
)
}
Ok(())
}

/// Checks whether a package matches any of a list of glob patterns generated
/// from `opt_patterns_and_names`.
///
/// > This function should be used only in package selection processes such like
/// `Packages::to_package_id_specs` and `Packages::get_packages`.
fn match_patterns(pkg: &Package, patterns: &mut Vec<(glob::Pattern, bool)>) -> bool {
patterns.iter_mut().any(|(m, matched)| {
let is_matched = m.matches(pkg.name().as_str());
*matched |= is_matched;
is_matched
})
}

/// Given a list opt-in or opt-out package selection strings, generates two
/// collections that represent glob patterns and package names respectively.
///
/// > This function should be used only in package selection processes such like
/// `Packages::to_package_id_specs` and `Packages::get_packages`.
fn opt_patterns_and_names(
opt: &[String],
) -> CargoResult<(Vec<(glob::Pattern, bool)>, BTreeSet<&str>)> {
let mut opt_patterns = Vec::new();
let mut opt_names = BTreeSet::new();
for x in opt.iter() {
if is_glob_pattern(x) {
opt_patterns.push((build_glob(x)?, false));
} else {
opt_names.insert(String::as_str(x));
}
}
Ok((opt_patterns, opt_names))
}
4 changes: 4 additions & 0 deletions src/cargo/ops/cargo_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ pub fn run(
) -> CargoResult<()> {
let config = ws.config();

if options.filter.contains_glob_patterns() {
anyhow::bail!("`cargo run` does not support glob patterns on target selection")
}

// We compute the `bins` here *just for diagnosis*. The actual set of
// packages to be run is determined by the `ops::compile` call below.
let packages = options.spec.get_packages(ws)?;
Expand Down
7 changes: 6 additions & 1 deletion src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionCon
use crate::sources::CRATES_IO_REGISTRY;
use crate::util::important_paths::find_root_manifest_for_wd;
use crate::util::interning::InternedString;
use crate::util::restricted_names::is_glob_pattern;
use crate::util::{paths, toml::TomlProfile, validate_package_name};
use crate::util::{
print_available_benches, print_available_binaries, print_available_examples,
Expand Down Expand Up @@ -510,7 +511,11 @@ pub trait ArgMatchesExt {
profile_checking: ProfileChecking,
) -> CargoResult<CompileOptions> {
let mut compile_opts = self.compile_options(config, mode, workspace, profile_checking)?;
compile_opts.spec = Packages::Packages(self._values_of("package"));
let spec = self._values_of("package");
if spec.iter().any(is_glob_pattern) {
anyhow::bail!("Glob patterns on package selection are not supported.")
}
compile_opts.spec = Packages::Packages(spec);
Ok(compile_opts)
}

Expand Down
7 changes: 6 additions & 1 deletion src/cargo/util/restricted_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult<
Ok(())
}

// Check the entire path for names reserved in Windows.
/// Check the entire path for names reserved in Windows.
pub fn is_windows_reserved_path(path: &Path) -> bool {
path.iter()
.filter_map(|component| component.to_str())
Expand All @@ -92,3 +92,8 @@ pub fn is_windows_reserved_path(path: &Path) -> bool {
is_windows_reserved(stem)
})
}

/// Returns `true` if the name contains any glob pattern wildcards.
pub fn is_glob_pattern<T: AsRef<str>>(name: T) -> bool {
name.as_ref().contains(&['*', '?', '[', ']'][..])
}