From e6d90b4c1fb78155beb712a6f8f6a8225cefa415 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 16 Oct 2022 06:11:39 +0800 Subject: [PATCH 1/5] refactor: extract `Packages` & `CompileFilter` to separate modules --- src/cargo/ops/cargo_compile.rs | 524 +----------------- src/cargo/ops/cargo_compile/compile_filter.rs | 294 ++++++++++ src/cargo/ops/cargo_compile/packages.rs | 218 ++++++++ 3 files changed, 521 insertions(+), 515 deletions(-) create mode 100644 src/cargo/ops/cargo_compile/compile_filter.rs create mode 100644 src/cargo/ops/cargo_compile/packages.rs diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 78264de58dc..75f1bc89c85 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -29,7 +29,7 @@ //! [`Layout`]: crate::core::compiler::Layout //! ["Cargo Target"]: https://doc.rust-lang.org/nightly/cargo/reference/cargo-targets.html -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{HashMap, HashSet}; use std::fmt::Write; use std::hash::{Hash, Hasher}; use std::sync::Arc; @@ -44,7 +44,7 @@ use crate::core::profiles::{Profiles, UnitFor}; use crate::core::resolver::features::{self, CliFeatures, FeaturesFor}; use crate::core::resolver::{HasDevUnits, Resolve}; use crate::core::{FeatureValue, Package, PackageSet, Shell, Summary, Target}; -use crate::core::{PackageId, PackageIdSpec, SourceId, TargetKind, Workspace}; +use crate::core::{PackageId, SourceId, TargetKind, Workspace}; use crate::drop_println; use crate::ops; use crate::ops::resolve::WorkspaceResolve; @@ -53,7 +53,12 @@ use crate::util::interning::InternedString; use crate::util::restricted_names::is_glob_pattern; use crate::util::{closest_msg, profile, CargoResult, StableHasher}; -use anyhow::{bail, Context as _}; +mod compile_filter; +pub use compile_filter::{CompileFilter, FilterRule, LibRule}; + +mod packages; +use packages::build_glob; +pub use packages::Packages; /// Contains information about how a package should be compiled. /// @@ -112,188 +117,6 @@ impl CompileOptions { } } -/// Represents the selected pacakges that will be built. -/// -/// Generally, it represents the combination of all `-p` flag. When working within -/// a workspace, `--exclude` and `--workspace` flags also contribute to it. -#[derive(PartialEq, Eq, Debug)] -pub enum Packages { - /// Pacakges selected by default. Ususally means no flag provided. - Default, - /// Opt in all packages. - /// - /// As of the time of this writing, it only works on opting in all workspace members. - All, - /// Opt out of packages passed in. - /// - /// As of the time of this writing, it only works on opting out workspace members. - OptOut(Vec), - /// A sequence of hand-picked packages that will be built. Normally done by `-p` flag. - Packages(Vec), -} - -impl Packages { - /// Creates a `Packages` from flags which are generally equivalent to command line flags. - pub fn from_flags(all: bool, exclude: Vec, package: Vec) -> CargoResult { - Ok(match (all, exclude.len(), package.len()) { - (false, 0, 0) => Packages::Default, - (false, 0, _) => Packages::Packages(package), - (false, _, _) => anyhow::bail!("--exclude can only be used together with --workspace"), - (true, 0, _) => Packages::All, - (true, _, _) => Packages::OptOut(exclude), - }) - } - - /// Converts selected packages to [`PackageIdSpec`]s. - pub fn to_package_id_specs(&self, ws: &Workspace<'_>) -> CargoResult> { - let specs = match self { - Packages::All => ws - .members() - .map(Package::package_id) - .map(PackageIdSpec::from_package_id) - .collect(), - Packages::OptOut(opt_out) => { - let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?; - let specs = ws - .members() - .filter(|pkg| { - !names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns) - }) - .map(Package::package_id) - .map(PackageIdSpec::from_package_id) - .collect(); - 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(opt_in) => { - let (mut patterns, packages) = opt_patterns_and_names(opt_in)?; - let mut specs = packages - .iter() - .map(|p| PackageIdSpec::parse(p)) - .collect::>>()?; - 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) - .map(PackageIdSpec::from_package_id) - .collect(), - }; - if specs.is_empty() { - if ws.is_virtual() { - bail!( - "manifest path `{}` contains no package: The manifest is virtual, \ - and the workspace has no members.", - ws.root().display() - ) - } - bail!("no packages to compile") - } - Ok(specs) - } - - /// Gets a list of selected [`Package`]s. - pub fn get_packages<'ws>(&self, ws: &'ws Workspace<'_>) -> CargoResult> { - let packages: Vec<_> = match self { - Packages::Default => ws.default_members().collect(), - Packages::All => ws.members().collect(), - 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) - } - - /// Returns whether or not the user needs to pass a `-p` flag to target a - /// specific package in the workspace. - pub fn needs_spec_flag(&self, ws: &Workspace<'_>) -> bool { - match self { - Packages::Default => ws.default_members().count() > 1, - Packages::All => ws.members().count() > 1, - Packages::Packages(_) => true, - Packages::OptOut(_) => true, - } - } -} - -#[derive(Debug, PartialEq, Eq)] -/// Indicates whether or not the library target gets included. -pub enum LibRule { - /// Include the library, fail if not present - True, - /// Include the library if present - Default, - /// Exclude the library - False, -} - -#[derive(Debug)] -/// Indicates which Cargo targets will be selected to be built. -pub enum FilterRule { - /// All included. - All, - /// Just a subset of Cargo targets based on names given. - Just(Vec), -} - -/// Filter to apply to the root package to select which Cargo targets will be built. -/// (examples, bins, benches, tests, ...) -/// -/// The actual filter process happens inside [`generate_targets`]. -#[derive(Debug)] -pub enum CompileFilter { - /// The default set of Cargo targets. - Default { - /// Flag whether targets can be safely skipped when required-features are not satisfied. - required_features_filterable: bool, - }, - /// Only includes a subset of all Cargo targets. - Only { - /// Include all Cargo targets. - all_targets: bool, - lib: LibRule, - bins: FilterRule, - examples: FilterRule, - tests: FilterRule, - benches: FilterRule, - }, -} - /// Compiles! /// /// This uses the [`DefaultExecutor`]. To use a custom [`Executor`], see [`compile_with_exec`]. @@ -557,7 +380,7 @@ pub fn create_bcx<'a, 'cfg>( FilterRule::none(), ), _ => { - bail!( + anyhow::bail!( r#"-Z rustdoc-scrape-examples must take "all" or "examples" as an argument"# ) } @@ -753,252 +576,6 @@ pub fn create_bcx<'a, 'cfg>( Ok(bcx) } -impl FilterRule { - pub fn new(targets: Vec, all: bool) -> FilterRule { - if all { - FilterRule::All - } else { - FilterRule::Just(targets) - } - } - - pub fn none() -> FilterRule { - FilterRule::Just(Vec::new()) - } - - fn matches(&self, target: &Target) -> bool { - match *self { - FilterRule::All => true, - FilterRule::Just(ref targets) => targets.iter().any(|x| *x == target.name()), - } - } - - fn is_specific(&self) -> bool { - match *self { - FilterRule::All => true, - FilterRule::Just(ref targets) => !targets.is_empty(), - } - } - - pub fn try_collect(&self) -> Option> { - match *self { - FilterRule::All => None, - 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 { - /// Constructs a filter from raw command line arguments. - pub fn from_raw_arguments( - lib_only: bool, - bins: Vec, - all_bins: bool, - tsts: Vec, - all_tsts: bool, - exms: Vec, - all_exms: bool, - bens: Vec, - all_bens: bool, - all_targets: bool, - ) -> CompileFilter { - if all_targets { - return CompileFilter::new_all_targets(); - } - let rule_lib = if lib_only { - LibRule::True - } else { - LibRule::False - }; - let rule_bins = FilterRule::new(bins, all_bins); - let rule_tsts = FilterRule::new(tsts, all_tsts); - let rule_exms = FilterRule::new(exms, all_exms); - let rule_bens = FilterRule::new(bens, all_bens); - - CompileFilter::new(rule_lib, rule_bins, rule_tsts, rule_exms, rule_bens) - } - - /// Constructs a filter from underlying primitives. - pub fn new( - rule_lib: LibRule, - rule_bins: FilterRule, - rule_tsts: FilterRule, - rule_exms: FilterRule, - rule_bens: FilterRule, - ) -> CompileFilter { - if rule_lib == LibRule::True - || rule_bins.is_specific() - || rule_tsts.is_specific() - || rule_exms.is_specific() - || rule_bens.is_specific() - { - CompileFilter::Only { - all_targets: false, - lib: rule_lib, - bins: rule_bins, - examples: rule_exms, - benches: rule_bens, - tests: rule_tsts, - } - } else { - CompileFilter::Default { - required_features_filterable: true, - } - } - } - - /// Constructs a filter that includes all targets. - pub fn new_all_targets() -> CompileFilter { - CompileFilter::Only { - all_targets: true, - lib: LibRule::Default, - bins: FilterRule::All, - examples: FilterRule::All, - benches: FilterRule::All, - tests: FilterRule::All, - } - } - - /// Constructs a filter that includes all test targets. - /// - /// Being different from the behavior of [`CompileFilter::Default`], this - /// function only recognizes test targets, which means cargo might compile - /// all targets with `tested` flag on, whereas [`CompileFilter::Default`] - /// may include additional example targets to ensure they can be compiled. - /// - /// Note that the actual behavior is subject to `filter_default_targets` - /// and `generate_targets` though. - pub fn all_test_targets() -> Self { - Self::Only { - all_targets: false, - lib: LibRule::Default, - bins: FilterRule::none(), - examples: FilterRule::none(), - tests: FilterRule::All, - benches: FilterRule::none(), - } - } - - /// Constructs a filter that includes lib target only. - pub fn lib_only() -> Self { - Self::Only { - all_targets: false, - lib: LibRule::True, - bins: FilterRule::none(), - examples: FilterRule::none(), - tests: FilterRule::none(), - benches: FilterRule::none(), - } - } - - /// Constructs a filter that includes the given binary. No more. No less. - pub fn single_bin(bin: String) -> Self { - Self::Only { - all_targets: false, - lib: LibRule::False, - bins: FilterRule::new(vec![bin], false), - examples: FilterRule::none(), - tests: FilterRule::none(), - benches: FilterRule::none(), - } - } - - /// Indicates if Cargo needs to build any dev dependency. - pub fn need_dev_deps(&self, mode: CompileMode) -> bool { - match mode { - CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, - CompileMode::Check { test: true } => true, - CompileMode::Build - | CompileMode::Doc { .. } - | CompileMode::Docscrape - | CompileMode::Check { test: false } => match *self { - CompileFilter::Default { .. } => false, - CompileFilter::Only { - ref examples, - ref tests, - ref benches, - .. - } => examples.is_specific() || tests.is_specific() || benches.is_specific(), - }, - CompileMode::RunCustomBuild => panic!("Invalid mode"), - } - } - - /// Selects targets for "cargo run". for logic to select targets for other - /// subcommands, see `generate_targets` and `filter_default_targets`. - pub fn target_run(&self, target: &Target) -> bool { - match *self { - CompileFilter::Default { .. } => true, - CompileFilter::Only { - ref lib, - ref bins, - ref examples, - ref tests, - ref benches, - .. - } => { - let rule = match *target.kind() { - TargetKind::Bin => bins, - TargetKind::Test => tests, - TargetKind::Bench => benches, - TargetKind::ExampleBin | TargetKind::ExampleLib(..) => examples, - TargetKind::Lib(..) => { - return match *lib { - LibRule::True => true, - LibRule::Default => true, - LibRule::False => false, - }; - } - TargetKind::CustomBuild => return false, - }; - rule.matches(target) - } - } - } - - pub fn is_specific(&self) -> bool { - match *self { - CompileFilter::Default { .. } => false, - CompileFilter::Only { .. } => true, - } - } - - pub fn is_all_targets(&self) -> bool { - matches!( - *self, - CompileFilter::Only { - all_targets: 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. /// /// Proposed targets are later filtered into actual `Unit`s based on whether or @@ -1725,89 +1302,6 @@ fn traverse_and_share( new_unit } -/// Build `glob::Pattern` with informative context. -fn build_glob(pat: &str) -> CargoResult { - 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::>().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::>(); - 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)) -} - /// Removes duplicate CompileMode::Doc units that would cause problems with /// filename collisions. /// diff --git a/src/cargo/ops/cargo_compile/compile_filter.rs b/src/cargo/ops/cargo_compile/compile_filter.rs new file mode 100644 index 00000000000..1b2915b7d82 --- /dev/null +++ b/src/cargo/ops/cargo_compile/compile_filter.rs @@ -0,0 +1,294 @@ +use crate::core::compiler::CompileMode; +use crate::core::{Target, TargetKind}; +use crate::util::restricted_names::is_glob_pattern; + +#[derive(Debug, PartialEq, Eq)] +/// Indicates whether or not the library target gets included. +pub enum LibRule { + /// Include the library, fail if not present + True, + /// Include the library if present + Default, + /// Exclude the library + False, +} + +#[derive(Debug)] +/// Indicates which Cargo targets will be selected to be built. +pub enum FilterRule { + /// All included. + All, + /// Just a subset of Cargo targets based on names given. + Just(Vec), +} + +/// Filter to apply to the root package to select which Cargo targets will be built. +/// (examples, bins, benches, tests, ...) +/// +/// The actual filter process happens inside [`generate_targets`]. +/// +/// [`generate_targets`]: super::generate_targets +#[derive(Debug)] +pub enum CompileFilter { + /// The default set of Cargo targets. + Default { + /// Flag whether targets can be safely skipped when required-features are not satisfied. + required_features_filterable: bool, + }, + /// Only includes a subset of all Cargo targets. + Only { + /// Include all Cargo targets. + all_targets: bool, + lib: LibRule, + bins: FilterRule, + examples: FilterRule, + tests: FilterRule, + benches: FilterRule, + }, +} + +impl FilterRule { + pub fn new(targets: Vec, all: bool) -> FilterRule { + if all { + FilterRule::All + } else { + FilterRule::Just(targets) + } + } + + pub fn none() -> FilterRule { + FilterRule::Just(Vec::new()) + } + + fn matches(&self, target: &Target) -> bool { + match *self { + FilterRule::All => true, + FilterRule::Just(ref targets) => targets.iter().any(|x| *x == target.name()), + } + } + + fn is_specific(&self) -> bool { + match *self { + FilterRule::All => true, + FilterRule::Just(ref targets) => !targets.is_empty(), + } + } + + pub fn try_collect(&self) -> Option> { + match *self { + FilterRule::All => None, + 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 { + /// Constructs a filter from raw command line arguments. + pub fn from_raw_arguments( + lib_only: bool, + bins: Vec, + all_bins: bool, + tsts: Vec, + all_tsts: bool, + exms: Vec, + all_exms: bool, + bens: Vec, + all_bens: bool, + all_targets: bool, + ) -> CompileFilter { + if all_targets { + return CompileFilter::new_all_targets(); + } + let rule_lib = if lib_only { + LibRule::True + } else { + LibRule::False + }; + let rule_bins = FilterRule::new(bins, all_bins); + let rule_tsts = FilterRule::new(tsts, all_tsts); + let rule_exms = FilterRule::new(exms, all_exms); + let rule_bens = FilterRule::new(bens, all_bens); + + CompileFilter::new(rule_lib, rule_bins, rule_tsts, rule_exms, rule_bens) + } + + /// Constructs a filter from underlying primitives. + pub fn new( + rule_lib: LibRule, + rule_bins: FilterRule, + rule_tsts: FilterRule, + rule_exms: FilterRule, + rule_bens: FilterRule, + ) -> CompileFilter { + if rule_lib == LibRule::True + || rule_bins.is_specific() + || rule_tsts.is_specific() + || rule_exms.is_specific() + || rule_bens.is_specific() + { + CompileFilter::Only { + all_targets: false, + lib: rule_lib, + bins: rule_bins, + examples: rule_exms, + benches: rule_bens, + tests: rule_tsts, + } + } else { + CompileFilter::Default { + required_features_filterable: true, + } + } + } + + /// Constructs a filter that includes all targets. + pub fn new_all_targets() -> CompileFilter { + CompileFilter::Only { + all_targets: true, + lib: LibRule::Default, + bins: FilterRule::All, + examples: FilterRule::All, + benches: FilterRule::All, + tests: FilterRule::All, + } + } + + /// Constructs a filter that includes all test targets. + /// + /// Being different from the behavior of [`CompileFilter::Default`], this + /// function only recognizes test targets, which means cargo might compile + /// all targets with `tested` flag on, whereas [`CompileFilter::Default`] + /// may include additional example targets to ensure they can be compiled. + /// + /// Note that the actual behavior is subject to `filter_default_targets` + /// and `generate_targets` though. + pub fn all_test_targets() -> Self { + Self::Only { + all_targets: false, + lib: LibRule::Default, + bins: FilterRule::none(), + examples: FilterRule::none(), + tests: FilterRule::All, + benches: FilterRule::none(), + } + } + + /// Constructs a filter that includes lib target only. + pub fn lib_only() -> Self { + Self::Only { + all_targets: false, + lib: LibRule::True, + bins: FilterRule::none(), + examples: FilterRule::none(), + tests: FilterRule::none(), + benches: FilterRule::none(), + } + } + + /// Constructs a filter that includes the given binary. No more. No less. + pub fn single_bin(bin: String) -> Self { + Self::Only { + all_targets: false, + lib: LibRule::False, + bins: FilterRule::new(vec![bin], false), + examples: FilterRule::none(), + tests: FilterRule::none(), + benches: FilterRule::none(), + } + } + + /// Indicates if Cargo needs to build any dev dependency. + pub fn need_dev_deps(&self, mode: CompileMode) -> bool { + match mode { + CompileMode::Test | CompileMode::Doctest | CompileMode::Bench => true, + CompileMode::Check { test: true } => true, + CompileMode::Build + | CompileMode::Doc { .. } + | CompileMode::Docscrape + | CompileMode::Check { test: false } => match *self { + CompileFilter::Default { .. } => false, + CompileFilter::Only { + ref examples, + ref tests, + ref benches, + .. + } => examples.is_specific() || tests.is_specific() || benches.is_specific(), + }, + CompileMode::RunCustomBuild => panic!("Invalid mode"), + } + } + + /// Selects targets for "cargo run". for logic to select targets for other + /// subcommands, see `generate_targets` and `filter_default_targets`. + pub fn target_run(&self, target: &Target) -> bool { + match *self { + CompileFilter::Default { .. } => true, + CompileFilter::Only { + ref lib, + ref bins, + ref examples, + ref tests, + ref benches, + .. + } => { + let rule = match *target.kind() { + TargetKind::Bin => bins, + TargetKind::Test => tests, + TargetKind::Bench => benches, + TargetKind::ExampleBin | TargetKind::ExampleLib(..) => examples, + TargetKind::Lib(..) => { + return match *lib { + LibRule::True => true, + LibRule::Default => true, + LibRule::False => false, + }; + } + TargetKind::CustomBuild => return false, + }; + rule.matches(target) + } + } + } + + pub fn is_specific(&self) -> bool { + match *self { + CompileFilter::Default { .. } => false, + CompileFilter::Only { .. } => true, + } + } + + pub fn is_all_targets(&self) -> bool { + matches!( + *self, + CompileFilter::Only { + all_targets: 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() + } + } + } +} diff --git a/src/cargo/ops/cargo_compile/packages.rs b/src/cargo/ops/cargo_compile/packages.rs new file mode 100644 index 00000000000..e16dd445f46 --- /dev/null +++ b/src/cargo/ops/cargo_compile/packages.rs @@ -0,0 +1,218 @@ +use std::collections::BTreeSet; + +use crate::core::Package; +use crate::core::{PackageIdSpec, Workspace}; +use crate::util::restricted_names::is_glob_pattern; +use crate::util::CargoResult; + +use anyhow::{bail, Context as _}; + +/// Represents the selected pacakges that will be built. +/// +/// Generally, it represents the combination of all `-p` flag. When working within +/// a workspace, `--exclude` and `--workspace` flags also contribute to it. +#[derive(PartialEq, Eq, Debug)] +pub enum Packages { + /// Pacakges selected by default. Ususally means no flag provided. + Default, + /// Opt in all packages. + /// + /// As of the time of this writing, it only works on opting in all workspace members. + All, + /// Opt out of packages passed in. + /// + /// As of the time of this writing, it only works on opting out workspace members. + OptOut(Vec), + /// A sequence of hand-picked packages that will be built. Normally done by `-p` flag. + Packages(Vec), +} + +impl Packages { + /// Creates a `Packages` from flags which are generally equivalent to command line flags. + pub fn from_flags(all: bool, exclude: Vec, package: Vec) -> CargoResult { + Ok(match (all, exclude.len(), package.len()) { + (false, 0, 0) => Packages::Default, + (false, 0, _) => Packages::Packages(package), + (false, _, _) => anyhow::bail!("--exclude can only be used together with --workspace"), + (true, 0, _) => Packages::All, + (true, _, _) => Packages::OptOut(exclude), + }) + } + + /// Converts selected packages to [`PackageIdSpec`]s. + pub fn to_package_id_specs(&self, ws: &Workspace<'_>) -> CargoResult> { + let specs = match self { + Packages::All => ws + .members() + .map(Package::package_id) + .map(PackageIdSpec::from_package_id) + .collect(), + Packages::OptOut(opt_out) => { + let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?; + let specs = ws + .members() + .filter(|pkg| { + !names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns) + }) + .map(Package::package_id) + .map(PackageIdSpec::from_package_id) + .collect(); + 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(opt_in) => { + let (mut patterns, packages) = opt_patterns_and_names(opt_in)?; + let mut specs = packages + .iter() + .map(|p| PackageIdSpec::parse(p)) + .collect::>>()?; + 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) + .map(PackageIdSpec::from_package_id) + .collect(), + }; + if specs.is_empty() { + if ws.is_virtual() { + bail!( + "manifest path `{}` contains no package: The manifest is virtual, \ + and the workspace has no members.", + ws.root().display() + ) + } + bail!("no packages to compile") + } + Ok(specs) + } + + /// Gets a list of selected [`Package`]s. + pub fn get_packages<'ws>(&self, ws: &'ws Workspace<'_>) -> CargoResult> { + let packages: Vec<_> = match self { + Packages::Default => ws.default_members().collect(), + Packages::All => ws.members().collect(), + 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) + } + + /// Returns whether or not the user needs to pass a `-p` flag to target a + /// specific package in the workspace. + pub fn needs_spec_flag(&self, ws: &Workspace<'_>) -> bool { + match self { + Packages::Default => ws.default_members().count() > 1, + Packages::All => ws.members().count() > 1, + Packages::Packages(_) => true, + Packages::OptOut(_) => true, + } + } +} + +/// Emits "package not found" error. +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::>().join(", "), + ws.root().display(), + ) + } + Ok(()) +} + +/// Emits "glob pattern not found" error. +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::>(); + 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(()) +} + +/// Given a list opt-in or opt-out package selection strings, generates two +/// collections that represent glob patterns and package names respectively. +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)) +} + +/// Checks whether a package matches any of a list of glob patterns generated +/// from `opt_patterns_and_names`. +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 + }) +} + +/// Build [`glob::Pattern`] with informative context. +pub fn build_glob(pat: &str) -> CargoResult { + glob::Pattern::new(pat).with_context(|| format!("cannot build glob pattern from `{}`", pat)) +} From f4da514ddcf609b2d63dbda127bf18cfe3b110d8 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 16 Oct 2022 06:42:55 +0800 Subject: [PATCH 2/5] doc(cargo_compile): more on compile filter --- src/cargo/ops/cargo_compile/compile_filter.rs | 15 +++++++++++++++ src/cargo/ops/cargo_compile/packages.rs | 2 ++ 2 files changed, 17 insertions(+) diff --git a/src/cargo/ops/cargo_compile/compile_filter.rs b/src/cargo/ops/cargo_compile/compile_filter.rs index 1b2915b7d82..508c96df4b1 100644 --- a/src/cargo/ops/cargo_compile/compile_filter.rs +++ b/src/cargo/ops/cargo_compile/compile_filter.rs @@ -1,3 +1,5 @@ +//! Filters and their rules to select which Cargo targets will be built. + use crate::core::compiler::CompileMode; use crate::core::{Target, TargetKind}; use crate::util::restricted_names::is_glob_pattern; @@ -27,7 +29,10 @@ pub enum FilterRule { /// /// The actual filter process happens inside [`generate_targets`]. /// +/// Not to be confused with [`Packages`], which opts in packages to be built. +/// /// [`generate_targets`]: super::generate_targets +/// [`Packages`]: crate::ops::Packages #[derive(Debug)] pub enum CompileFilter { /// The default set of Cargo targets. @@ -56,10 +61,15 @@ impl FilterRule { } } + /// Creates a filter with no rule. + /// + /// In the current Cargo implementation, filter without a rule implies + /// Cargo will follows the default behaviour to filter targets. pub fn none() -> FilterRule { FilterRule::Just(Vec::new()) } + /// Checks if a target definition matches this filter rule. fn matches(&self, target: &Target) -> bool { match *self { FilterRule::All => true, @@ -67,6 +77,9 @@ impl FilterRule { } } + /// Check if a filter is specific. + /// + /// Only filters without rules are considered as not specific. fn is_specific(&self) -> bool { match *self { FilterRule::All => true, @@ -81,6 +94,7 @@ impl FilterRule { } } + /// Checks if any specified target name contains glob patterns. pub(crate) fn contains_glob_patterns(&self) -> bool { match self { FilterRule::All => false, @@ -274,6 +288,7 @@ impl CompileFilter { ) } + /// Checks if any specified target name contains glob patterns. pub(crate) fn contains_glob_patterns(&self) -> bool { match self { CompileFilter::Default { .. } => false, diff --git a/src/cargo/ops/cargo_compile/packages.rs b/src/cargo/ops/cargo_compile/packages.rs index e16dd445f46..39f627b5db5 100644 --- a/src/cargo/ops/cargo_compile/packages.rs +++ b/src/cargo/ops/cargo_compile/packages.rs @@ -1,3 +1,5 @@ +//! See [`Packages`]. + use std::collections::BTreeSet; use crate::core::Package; From 3fd7580c5b3ce838bda06e34ae47f45769748940 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 16 Oct 2022 07:31:26 +0800 Subject: [PATCH 3/5] remove unnecessary specialiized `FilterRule::try_collect` method --- src/cargo/ops/cargo_compile/compile_filter.rs | 7 ------ .../ops/common_for_install_and_uninstall.rs | 25 +++++++++---------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/cargo/ops/cargo_compile/compile_filter.rs b/src/cargo/ops/cargo_compile/compile_filter.rs index 508c96df4b1..bf6dbbd7bf5 100644 --- a/src/cargo/ops/cargo_compile/compile_filter.rs +++ b/src/cargo/ops/cargo_compile/compile_filter.rs @@ -87,13 +87,6 @@ impl FilterRule { } } - pub fn try_collect(&self) -> Option> { - match *self { - FilterRule::All => None, - FilterRule::Just(ref targets) => Some(targets.clone()), - } - } - /// Checks if any specified target name contains glob patterns. pub(crate) fn contains_glob_patterns(&self) -> bool { match self { diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index 08d5c908979..6b8e8d792aa 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -7,10 +7,12 @@ use std::rc::Rc; use std::task::Poll; use anyhow::{bail, format_err, Context as _}; +use ops::FilterRule; use serde::{Deserialize, Serialize}; use toml_edit::easy as toml; use crate::core::compiler::Freshness; +use crate::core::Target; use crate::core::{Dependency, FeatureValue, Package, PackageId, QueryKind, Source, SourceId}; use crate::ops::{self, CompileFilter, CompileOptions}; use crate::sources::PathSource; @@ -690,20 +692,17 @@ pub fn exe_names(pkg: &Package, filter: &ops::CompileFilter) -> BTreeSet ref examples, .. } => { - let all_bins: Vec = bins.try_collect().unwrap_or_else(|| { - pkg.targets() + let collect = |rule: &_, f: fn(&Target) -> _| match rule { + FilterRule::All => pkg + .targets() .iter() - .filter(|t| t.is_bin()) - .map(|t| t.name().to_string()) - .collect() - }); - let all_examples: Vec = examples.try_collect().unwrap_or_else(|| { - pkg.targets() - .iter() - .filter(|t| t.is_exe_example()) - .map(|t| t.name().to_string()) - .collect() - }); + .filter(|t| f(t)) + .map(|t| t.name().into()) + .collect(), + FilterRule::Just(targets) => targets.clone(), + }; + let all_bins = collect(bins, Target::is_bin); + let all_examples = collect(examples, Target::is_exe_example); all_bins .iter() From 0195423233c826c8387f4420233fa233ee86d30b Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Sun, 16 Oct 2022 08:03:48 +0800 Subject: [PATCH 4/5] Remove unused `CompileOptions.local_rustdoc_args` --- src/cargo/ops/cargo_compile.rs | 47 ++++++++++++------------------- src/cargo/ops/cargo_package.rs | 1 - src/cargo/util/command_prelude.rs | 1 - 3 files changed, 18 insertions(+), 31 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 75f1bc89c85..b7bf1dd94f4 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -86,8 +86,6 @@ pub struct CompileOptions { pub target_rustc_args: Option>, /// Crate types to be passed to rustc (single target only) pub target_rustc_crate_types: Option>, - /// Extra arguments passed to all selected targets for rustdoc. - pub local_rustdoc_args: Option>, /// Whether the `--document-private-items` flags was specified and should /// be forwarded to `rustdoc`. pub rustdoc_document_private_items: bool, @@ -110,7 +108,6 @@ impl CompileOptions { target_rustdoc_args: None, target_rustc_args: None, target_rustc_crate_types: None, - local_rustdoc_args: None, rustdoc_document_private_items: false, honor_rust_version: true, }) @@ -206,7 +203,6 @@ pub fn create_bcx<'a, 'cfg>( ref target_rustdoc_args, ref target_rustc_args, ref target_rustc_crate_types, - ref local_rustdoc_args, rustdoc_document_private_items, honor_rust_version, } = *options; @@ -481,32 +477,25 @@ pub fn create_bcx<'a, 'cfg>( extra_compiler_args.insert(units[0].clone(), args); } - for unit in &units { - if unit.mode.is_doc() || unit.mode.is_doc_test() { - let mut extra_args = local_rustdoc_args.clone(); - - // Add `--document-private-items` rustdoc flag if requested or if - // the target is a binary. Binary crates get their private items - // documented by default. - if rustdoc_document_private_items || unit.target.is_bin() { - let mut args = extra_args.take().unwrap_or_default(); - args.push("--document-private-items".into()); - if unit.target.is_bin() { - // This warning only makes sense if it's possible to document private items - // sometimes and ignore them at other times. But cargo consistently passes - // `--document-private-items`, so the warning isn't useful. - args.push("-Arustdoc::private-intra-doc-links".into()); - } - extra_args = Some(args); - } - - if let Some(args) = extra_args { - extra_compiler_args - .entry(unit.clone()) - .or_default() - .extend(args); - } + for unit in units + .iter() + .filter(|unit| unit.mode.is_doc() || unit.mode.is_doc_test()) + .filter(|unit| rustdoc_document_private_items || unit.target.is_bin()) + { + // Add `--document-private-items` rustdoc flag if requested or if + // the target is a binary. Binary crates get their private items + // documented by default. + let mut args = vec!["--document-private-items".into()]; + if unit.target.is_bin() { + // This warning only makes sense if it's possible to document private items + // sometimes and ignore them at other times. But cargo consistently passes + // `--document-private-items`, so the warning isn't useful. + args.push("-Arustdoc::private-intra-doc-links".into()); } + extra_compiler_args + .entry(unit.clone()) + .or_default() + .extend(args); } if honor_rust_version { diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 270e1d70d3a..d3fd85af377 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -829,7 +829,6 @@ fn run_verify( target_rustdoc_args: None, target_rustc_args: rustc_args, target_rustc_crate_types: None, - local_rustdoc_args: None, rustdoc_document_private_items: false, honor_rust_version: true, }, diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 4e0a5dbffad..54466f6a0e5 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -600,7 +600,6 @@ pub trait ArgMatchesExt { target_rustdoc_args: None, target_rustc_args: None, target_rustc_crate_types: None, - local_rustdoc_args: None, rustdoc_document_private_items: false, honor_rust_version: !self.flag("ignore-rust-version"), }; From 886c9d2153013f9944de8e4308a0d5a6061bdc65 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 18 Oct 2022 01:55:15 +0800 Subject: [PATCH 5/5] refactor(cargo_compile): move to mod.rs --- src/cargo/ops/{cargo_compile.rs => cargo_compile/mod.rs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/cargo/ops/{cargo_compile.rs => cargo_compile/mod.rs} (100%) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile/mod.rs similarity index 100% rename from src/cargo/ops/cargo_compile.rs rename to src/cargo/ops/cargo_compile/mod.rs