Skip to content

Commit dd83ae5

Browse files
borsbors
committed
Auto merge of #8752 - weihanglo:feat/glob-pattern, r=ehuss
Support glob patterns for package/target selection Resolves #8582 ## What Commands that supports glob patterns on package/target selection: - `build` - `test` - `bench` - `doc` - `check` - `fix` (run `check` underneath the hood) - `tree` (no target selection functionalit as same as before) Commands that supports glob patterns on target selection only: - `rustdoc` (can only build one package via `-p/--package` option) - `rustc` (can only build one package via `-p/--package` option) Command that _does not_ support any glob patterns for package/target selection: - `run` (you can only run at most one executable) ## How By using existing [`glob`](https://crates.io/crates/glob) crate to 1. check whether a target filter is a glob pattern, and then 1. compare if the target matches the filter. Also, I loosed some shell-style restriction in cargo-test-support (by adding simple quote arguments support). ## Breaking Changes Suppose no. - No public API introduced. - All valid glob pattern chars (`*`, `?`, `[`, `]`) characters [are restricted by Cargo](https://github.com/rust-lang/cargo/blob/75615f8e69f748d7ef0df7bc0b064a9b1f5c78b2/src/cargo/util/restricted_names.rs#L70-L82 ) due to not containing in`XID_Continue` character set in [UAX#31](https://docs.rs/unicode-xid/0.2.1/unicode_xid/trait.UnicodeXID.html) and are also restricted ## Performance Regression Definitely gets some. However, assumed no one would pass lots of package/target selection with glob patterns, so the extra overhead for ordinary use cases are glob pattern existence checks on each target filter. The current implementation is somewhat naive, so if you have any better algorithm, please help me improve it. ## Documentation I have done my best effort to write the documentation, but as a non-native English speaker I need helps to polish these sentences. Besides, I am not quite sure should we mention glob pattern in CLI help text, which is a bit lengthy at the moment....
2 parents 9fb208d + d613cd6 commit dd83ae5

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+2016
-238
lines changed

‎crates/cargo-test-support/src/lib.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -1638,8 +1638,12 @@ impl ChannelChanger for cargo::util::ProcessBuilder {
16381638
}
16391639

16401640
fn split_and_add_args(p: &mut ProcessBuilder, s: &str) {
1641-
for arg in s.split_whitespace() {
1642-
if arg.contains('"') || arg.contains('\'') {
1641+
for mut arg in s.split_whitespace() {
1642+
if (arg.starts_with('"') && arg.ends_with('"'))
1643+
|| (arg.starts_with('\'') && arg.ends_with('\''))
1644+
{
1645+
arg = &arg[1..(arg.len() - 1).max(1)];
1646+
} else if arg.contains(&['"', '\''][..]) {
16431647
panic!("shell-style argument parsing is not supported")
16441648
}
16451649
p.arg(arg);

‎src/bin/cargo/commands/run.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
use crate::command_prelude::*;
2+
use crate::util::restricted_names::is_glob_pattern;
23
use crate::util::ProcessError;
34
use cargo::core::Verbosity;
4-
use cargo::ops::{self, CompileFilter};
5+
use cargo::ops::{self, CompileFilter, Packages};
56

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

42+
// Disallow `spec` to be an glob pattern
43+
if let Packages::Packages(opt_in) = &compile_opts.spec {
44+
if let Some(pattern) = opt_in.iter().find(|s| is_glob_pattern(s)) {
45+
return Err(anyhow::anyhow!(
46+
"`cargo run` does not support glob pattern `{}` on package selection",
47+
pattern,
48+
)
49+
.into());
50+
}
51+
}
52+
4153
if !args.is_present("example") && !args.is_present("bin") {
4254
let default_runs: Vec<_> = compile_opts
4355
.spec

‎src/cargo/ops/cargo_compile.rs

+175-40
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
2525
use std::collections::{BTreeSet, HashMap, HashSet};
2626
use std::hash::{Hash, Hasher};
27-
use std::iter::FromIterator;
2827
use std::sync::Arc;
2928

3029
use crate::core::compiler::standard_lib;
@@ -41,8 +40,11 @@ use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace};
4140
use crate::ops;
4241
use crate::ops::resolve::WorkspaceResolve;
4342
use crate::util::config::Config;
43+
use crate::util::restricted_names::is_glob_pattern;
4444
use crate::util::{closest_msg, profile, CargoResult, StableHasher};
4545

46+
use anyhow::Context as _;
47+
4648
/// Contains information about how a package should be compiled.
4749
///
4850
/// Note on distinction between `CompileOptions` and `BuildConfig`:
@@ -116,6 +118,7 @@ impl Packages {
116118
})
117119
}
118120

121+
/// Converts selected packages from a workspace to `PackageIdSpec`s.
119122
pub fn to_package_id_specs(&self, ws: &Workspace<'_>) -> CargoResult<Vec<PackageIdSpec>> {
120123
let specs = match self {
121124
Packages::All => ws
@@ -124,33 +127,40 @@ impl Packages {
124127
.map(PackageIdSpec::from_package_id)
125128
.collect(),
126129
Packages::OptOut(opt_out) => {
127-
let mut opt_out = BTreeSet::from_iter(opt_out.iter().cloned());
128-
let packages = ws
130+
let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?;
131+
let specs = ws
129132
.members()
130-
.filter(|pkg| !opt_out.remove(pkg.name().as_str()))
133+
.filter(|pkg| {
134+
!names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns)
135+
})
131136
.map(Package::package_id)
132137
.map(PackageIdSpec::from_package_id)
133138
.collect();
134-
if !opt_out.is_empty() {
135-
ws.config().shell().warn(format!(
136-
"excluded package(s) {} not found in workspace `{}`",
137-
opt_out
138-
.iter()
139-
.map(|x| x.as_ref())
140-
.collect::<Vec<_>>()
141-
.join(", "),
142-
ws.root().display(),
143-
))?;
144-
}
145-
packages
139+
let warn = |e| ws.config().shell().warn(e);
140+
emit_package_not_found(ws, names, true).or_else(warn)?;
141+
emit_pattern_not_found(ws, patterns, true).or_else(warn)?;
142+
specs
146143
}
147144
Packages::Packages(packages) if packages.is_empty() => {
148145
vec![PackageIdSpec::from_package_id(ws.current()?.package_id())]
149146
}
150-
Packages::Packages(packages) => packages
151-
.iter()
152-
.map(|p| PackageIdSpec::parse(p))
153-
.collect::<CargoResult<Vec<_>>>()?,
147+
Packages::Packages(opt_in) => {
148+
let (mut patterns, packages) = opt_patterns_and_names(opt_in)?;
149+
let mut specs = packages
150+
.iter()
151+
.map(|p| PackageIdSpec::parse(p))
152+
.collect::<CargoResult<Vec<_>>>()?;
153+
if !patterns.is_empty() {
154+
let matched_pkgs = ws
155+
.members()
156+
.filter(|pkg| match_patterns(pkg, &mut patterns))
157+
.map(Package::package_id)
158+
.map(PackageIdSpec::from_package_id);
159+
specs.extend(matched_pkgs);
160+
}
161+
emit_pattern_not_found(ws, patterns, false)?;
162+
specs
163+
}
154164
Packages::Default => ws
155165
.default_members()
156166
.map(Package::package_id)
@@ -170,27 +180,35 @@ impl Packages {
170180
Ok(specs)
171181
}
172182

183+
/// Gets a list of selected packages from a workspace.
173184
pub fn get_packages<'ws>(&self, ws: &'ws Workspace<'_>) -> CargoResult<Vec<&'ws Package>> {
174185
let packages: Vec<_> = match self {
175186
Packages::Default => ws.default_members().collect(),
176187
Packages::All => ws.members().collect(),
177-
Packages::OptOut(opt_out) => ws
178-
.members()
179-
.filter(|pkg| !opt_out.iter().any(|name| pkg.name().as_str() == name))
180-
.collect(),
181-
Packages::Packages(packages) => packages
182-
.iter()
183-
.map(|name| {
184-
ws.members()
185-
.find(|pkg| pkg.name().as_str() == name)
186-
.ok_or_else(|| {
187-
anyhow::format_err!(
188-
"package `{}` is not a member of the workspace",
189-
name
190-
)
191-
})
192-
})
193-
.collect::<CargoResult<Vec<_>>>()?,
188+
Packages::OptOut(opt_out) => {
189+
let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?;
190+
let packages = ws
191+
.members()
192+
.filter(|pkg| {
193+
!names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns)
194+
})
195+
.collect();
196+
emit_package_not_found(ws, names, true)?;
197+
emit_pattern_not_found(ws, patterns, true)?;
198+
packages
199+
}
200+
Packages::Packages(opt_in) => {
201+
let (mut patterns, mut names) = opt_patterns_and_names(opt_in)?;
202+
let packages = ws
203+
.members()
204+
.filter(|pkg| {
205+
names.remove(pkg.name().as_str()) || match_patterns(pkg, &mut patterns)
206+
})
207+
.collect();
208+
emit_package_not_found(ws, names, false)?;
209+
emit_pattern_not_found(ws, patterns, false)?;
210+
packages
211+
}
194212
};
195213
Ok(packages)
196214
}
@@ -577,6 +595,13 @@ impl FilterRule {
577595
FilterRule::Just(ref targets) => Some(targets.clone()),
578596
}
579597
}
598+
599+
pub(crate) fn contains_glob_patterns(&self) -> bool {
600+
match self {
601+
FilterRule::All => false,
602+
FilterRule::Just(targets) => targets.iter().any(is_glob_pattern),
603+
}
604+
}
580605
}
581606

582607
impl CompileFilter {
@@ -706,6 +731,24 @@ impl CompileFilter {
706731
CompileFilter::Only { .. } => true,
707732
}
708733
}
734+
735+
pub(crate) fn contains_glob_patterns(&self) -> bool {
736+
match self {
737+
CompileFilter::Default { .. } => false,
738+
CompileFilter::Only {
739+
bins,
740+
examples,
741+
tests,
742+
benches,
743+
..
744+
} => {
745+
bins.contains_glob_patterns()
746+
|| examples.contains_glob_patterns()
747+
|| tests.contains_glob_patterns()
748+
|| benches.contains_glob_patterns()
749+
}
750+
}
751+
}
709752
}
710753

711754
/// A proposed target.
@@ -1163,8 +1206,16 @@ fn find_named_targets<'a>(
11631206
is_expected_kind: fn(&Target) -> bool,
11641207
mode: CompileMode,
11651208
) -> CargoResult<Vec<Proposal<'a>>> {
1166-
let filter = |t: &Target| t.name() == target_name && is_expected_kind(t);
1167-
let proposals = filter_targets(packages, filter, true, mode);
1209+
let is_glob = is_glob_pattern(target_name);
1210+
let proposals = if is_glob {
1211+
let pattern = build_glob(target_name)?;
1212+
let filter = |t: &Target| is_expected_kind(t) && pattern.matches(t.name());
1213+
filter_targets(packages, filter, true, mode)
1214+
} else {
1215+
let filter = |t: &Target| t.name() == target_name && is_expected_kind(t);
1216+
filter_targets(packages, filter, true, mode)
1217+
};
1218+
11681219
if proposals.is_empty() {
11691220
let targets = packages.iter().flat_map(|pkg| {
11701221
pkg.targets()
@@ -1173,8 +1224,9 @@ fn find_named_targets<'a>(
11731224
});
11741225
let suggestion = closest_msg(target_name, targets, |t| t.name());
11751226
anyhow::bail!(
1176-
"no {} target named `{}`{}",
1227+
"no {} target {} `{}`{}",
11771228
target_desc,
1229+
if is_glob { "matches pattern" } else { "named" },
11781230
target_name,
11791231
suggestion
11801232
);
@@ -1291,3 +1343,86 @@ fn traverse_and_share(
12911343
new_graph.entry(new_unit.clone()).or_insert(new_deps);
12921344
new_unit
12931345
}
1346+
1347+
/// Build `glob::Pattern` with informative context.
1348+
fn build_glob(pat: &str) -> CargoResult<glob::Pattern> {
1349+
glob::Pattern::new(pat).with_context(|| format!("cannot build glob pattern from `{}`", pat))
1350+
}
1351+
1352+
/// Emits "package not found" error.
1353+
///
1354+
/// > This function should be used only in package selection processes such like
1355+
/// `Packages::to_package_id_specs` and `Packages::get_packages`.
1356+
fn emit_package_not_found(
1357+
ws: &Workspace<'_>,
1358+
opt_names: BTreeSet<&str>,
1359+
opt_out: bool,
1360+
) -> CargoResult<()> {
1361+
if !opt_names.is_empty() {
1362+
anyhow::bail!(
1363+
"{}package(s) `{}` not found in workspace `{}`",
1364+
if opt_out { "excluded " } else { "" },
1365+
opt_names.into_iter().collect::<Vec<_>>().join(", "),
1366+
ws.root().display(),
1367+
)
1368+
}
1369+
Ok(())
1370+
}
1371+
1372+
/// Emits "glob pattern not found" error.
1373+
///
1374+
/// > This function should be used only in package selection processes such like
1375+
/// `Packages::to_package_id_specs` and `Packages::get_packages`.
1376+
fn emit_pattern_not_found(
1377+
ws: &Workspace<'_>,
1378+
opt_patterns: Vec<(glob::Pattern, bool)>,
1379+
opt_out: bool,
1380+
) -> CargoResult<()> {
1381+
let not_matched = opt_patterns
1382+
.iter()
1383+
.filter(|(_, matched)| !*matched)
1384+
.map(|(pat, _)| pat.as_str())
1385+
.collect::<Vec<_>>();
1386+
if !not_matched.is_empty() {
1387+
anyhow::bail!(
1388+
"{}package pattern(s) `{}` not found in workspace `{}`",
1389+
if opt_out { "excluded " } else { "" },
1390+
not_matched.join(", "),
1391+
ws.root().display(),
1392+
)
1393+
}
1394+
Ok(())
1395+
}
1396+
1397+
/// Checks whether a package matches any of a list of glob patterns generated
1398+
/// from `opt_patterns_and_names`.
1399+
///
1400+
/// > This function should be used only in package selection processes such like
1401+
/// `Packages::to_package_id_specs` and `Packages::get_packages`.
1402+
fn match_patterns(pkg: &Package, patterns: &mut Vec<(glob::Pattern, bool)>) -> bool {
1403+
patterns.iter_mut().any(|(m, matched)| {
1404+
let is_matched = m.matches(pkg.name().as_str());
1405+
*matched |= is_matched;
1406+
is_matched
1407+
})
1408+
}
1409+
1410+
/// Given a list opt-in or opt-out package selection strings, generates two
1411+
/// collections that represent glob patterns and package names respectively.
1412+
///
1413+
/// > This function should be used only in package selection processes such like
1414+
/// `Packages::to_package_id_specs` and `Packages::get_packages`.
1415+
fn opt_patterns_and_names(
1416+
opt: &[String],
1417+
) -> CargoResult<(Vec<(glob::Pattern, bool)>, BTreeSet<&str>)> {
1418+
let mut opt_patterns = Vec::new();
1419+
let mut opt_names = BTreeSet::new();
1420+
for x in opt.iter() {
1421+
if is_glob_pattern(x) {
1422+
opt_patterns.push((build_glob(x)?, false));
1423+
} else {
1424+
opt_names.insert(String::as_str(x));
1425+
}
1426+
}
1427+
Ok((opt_patterns, opt_names))
1428+
}

‎src/cargo/ops/cargo_run.rs

+4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ pub fn run(
1313
) -> CargoResult<()> {
1414
let config = ws.config();
1515

16+
if options.filter.contains_glob_patterns() {
17+
anyhow::bail!("`cargo run` does not support glob patterns on target selection")
18+
}
19+
1620
// We compute the `bins` here *just for diagnosis*. The actual set of
1721
// packages to be run is determined by the `ops::compile` call below.
1822
let packages = options.spec.get_packages(ws)?;

‎src/cargo/util/command_prelude.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use crate::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionCon
44
use crate::sources::CRATES_IO_REGISTRY;
55
use crate::util::important_paths::find_root_manifest_for_wd;
66
use crate::util::interning::InternedString;
7+
use crate::util::restricted_names::is_glob_pattern;
78
use crate::util::{paths, toml::TomlProfile, validate_package_name};
89
use crate::util::{
910
print_available_benches, print_available_binaries, print_available_examples,
@@ -510,7 +511,11 @@ pub trait ArgMatchesExt {
510511
profile_checking: ProfileChecking,
511512
) -> CargoResult<CompileOptions> {
512513
let mut compile_opts = self.compile_options(config, mode, workspace, profile_checking)?;
513-
compile_opts.spec = Packages::Packages(self._values_of("package"));
514+
let spec = self._values_of("package");
515+
if spec.iter().any(is_glob_pattern) {
516+
anyhow::bail!("Glob patterns on package selection are not supported.")
517+
}
518+
compile_opts.spec = Packages::Packages(spec);
514519
Ok(compile_opts)
515520
}
516521

‎src/cargo/util/restricted_names.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ pub fn validate_package_name(name: &str, what: &str, help: &str) -> CargoResult<
8383
Ok(())
8484
}
8585

86-
// Check the entire path for names reserved in Windows.
86+
/// Check the entire path for names reserved in Windows.
8787
pub fn is_windows_reserved_path(path: &Path) -> bool {
8888
path.iter()
8989
.filter_map(|component| component.to_str())
@@ -92,3 +92,8 @@ pub fn is_windows_reserved_path(path: &Path) -> bool {
9292
is_windows_reserved(stem)
9393
})
9494
}
95+
96+
/// Returns `true` if the name contains any glob pattern wildcards.
97+
pub fn is_glob_pattern<T: AsRef<str>>(name: T) -> bool {
98+
name.as_ref().contains(&['*', '?', '[', ']'][..])
99+
}

0 commit comments

Comments
 (0)