diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index d8529034d73..6dc6091ef33 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1,7 +1,8 @@ use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::path::{Path, PathBuf}; +use std::rc::Rc; use std::slice; use glob::glob; @@ -11,7 +12,7 @@ use url::Url; use crate::core::features::Features; use crate::core::registry::PackageRegistry; use crate::core::resolver::features::RequestedFeatures; -use crate::core::{Dependency, PackageId, PackageIdSpec}; +use crate::core::{Dependency, InternedString, PackageId, PackageIdSpec}; use crate::core::{EitherManifest, Package, SourceId, VirtualManifest}; use crate::ops; use crate::sources::PathSource; @@ -878,59 +879,143 @@ impl<'cfg> Workspace<'cfg> { .collect()); } if self.config().cli_unstable().package_features { - if specs.len() > 1 && !requested_features.features.is_empty() { - anyhow::bail!("cannot specify features for more than one package"); + self.members_with_features_pf(specs, requested_features) + } else { + self.members_with_features_stable(specs, requested_features) + } + } + + /// New command-line feature selection with -Zpackage-features. + fn members_with_features_pf( + &self, + specs: &[PackageIdSpec], + requested_features: &RequestedFeatures, + ) -> CargoResult> { + // Keep track of which features matched *any* member, to produce an error + // if any of them did not match anywhere. + let mut found: BTreeSet = BTreeSet::new(); + + // Returns the requested features for the given member. + // This filters out any named features that the member does not have. + let mut matching_features = |member: &Package| -> RequestedFeatures { + if requested_features.features.is_empty() || requested_features.all_features { + return requested_features.clone(); + } + // Only include features this member defines. + let summary = member.summary(); + let member_features = summary.features(); + let mut features = BTreeSet::new(); + + // Checks if a member contains the given feature. + let contains = |feature: InternedString| -> bool { + member_features.contains_key(&feature) + || summary + .dependencies() + .iter() + .any(|dep| dep.is_optional() && dep.name_in_toml() == feature) + }; + + for feature in requested_features.features.iter() { + let mut split = feature.splitn(2, '/'); + let split = (split.next().unwrap(), split.next()); + if let (pkg, Some(pkg_feature)) = split { + let pkg = InternedString::new(pkg); + let pkg_feature = InternedString::new(pkg_feature); + if summary + .dependencies() + .iter() + .any(|dep| dep.name_in_toml() == pkg) + { + // pkg/feat for a dependency. + // Will rely on the dependency resolver to validate `feat`. + features.insert(*feature); + found.insert(*feature); + } else if pkg == member.name() && contains(pkg_feature) { + // member/feat where "feat" is a feature in member. + features.insert(pkg_feature); + found.insert(*feature); + } + } else if contains(*feature) { + // feature exists in this member. + features.insert(*feature); + found.insert(*feature); + } + } + RequestedFeatures { + features: Rc::new(features), + all_features: false, + uses_default_features: requested_features.uses_default_features, + } + }; + + let members: Vec<(&Package, RequestedFeatures)> = self + .members() + .filter(|m| specs.iter().any(|spec| spec.matches(m.package_id()))) + .map(|m| (m, matching_features(m))) + .collect(); + if members.is_empty() { + // `cargo build -p foo`, where `foo` is not a member. + // Do not allow any command-line flags (defaults only). + if !(requested_features.features.is_empty() + && !requested_features.all_features + && requested_features.uses_default_features) + { + anyhow::bail!("cannot specify features for packages outside of workspace"); } - let members: Vec<(&Package, RequestedFeatures)> = self + // Add all members from the workspace so we can ensure `-p nonmember` + // is in the resolve graph. + return Ok(self .members() - .filter(|m| specs.iter().any(|spec| spec.matches(m.package_id()))) - .map(|m| (m, requested_features.clone())) + .map(|m| (m, RequestedFeatures::new_all(false))) + .collect()); + } + if *requested_features.features != found { + let missing: Vec<_> = requested_features + .features + .difference(&found) + .copied() .collect(); - if members.is_empty() { - // `cargo build -p foo`, where `foo` is not a member. - // Do not allow any command-line flags (defaults only). - if !(requested_features.features.is_empty() - && !requested_features.all_features - && requested_features.uses_default_features) - { - anyhow::bail!("cannot specify features for packages outside of workspace"); + // TODO: typo suggestions would be good here. + anyhow::bail!( + "none of the selected packages contains these features: {}", + missing.join(", ") + ); + } + Ok(members) + } + + /// This is the current "stable" behavior for command-line feature selection. + fn members_with_features_stable( + &self, + specs: &[PackageIdSpec], + requested_features: &RequestedFeatures, + ) -> CargoResult> { + let ms = self.members().filter_map(|member| { + let member_id = member.package_id(); + match self.current_opt() { + // The features passed on the command-line only apply to + // the "current" package (determined by the cwd). + Some(current) if member_id == current.package_id() => { + Some((member, requested_features.clone())) } - // Add all members from the workspace so we can ensure `-p nonmember` - // is in the resolve graph. - return Ok(self - .members() - .map(|m| (m, RequestedFeatures::new_all(false))) - .collect()); - } - Ok(members) - } else { - let ms = self.members().filter_map(|member| { - let member_id = member.package_id(); - match self.current_opt() { - // The features passed on the command-line only apply to - // the "current" package (determined by the cwd). - Some(current) if member_id == current.package_id() => { - Some((member, requested_features.clone())) - } - _ => { - // Ignore members that are not enabled on the command-line. - if specs.iter().any(|spec| spec.matches(member_id)) { - // -p for a workspace member that is not the - // "current" one, don't use the local - // `--features`, only allow `--all-features`. - Some(( - member, - RequestedFeatures::new_all(requested_features.all_features), - )) - } else { - // This member was not requested on the command-line, skip. - None - } + _ => { + // Ignore members that are not enabled on the command-line. + if specs.iter().any(|spec| spec.matches(member_id)) { + // -p for a workspace member that is not the + // "current" one, don't use the local + // `--features`, only allow `--all-features`. + Some(( + member, + RequestedFeatures::new_all(requested_features.all_features), + )) + } else { + // This member was not requested on the command-line, skip. + None } } - }); - Ok(ms.collect()) - } + } + }); + Ok(ms.collect()) } } diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index b548ad5ae03..bf7e0599871 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -62,7 +62,14 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { let mut build_config = BuildConfig::new(config, Some(1), &opts.target, CompileMode::Build)?; build_config.requested_profile = opts.requested_profile; let target_data = RustcTargetData::new(ws, build_config.requested_kind)?; - let resolve_opts = ResolveOpts::everything(); + // Resolve for default features. In the future, `cargo clean` should be rewritten + // so that it doesn't need to guess filename hashes. + let resolve_opts = ResolveOpts::new( + /*dev_deps*/ true, + &[], + /*all features*/ false, + /*default*/ true, + ); let specs = opts .spec .iter() diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 9e05d271fdd..fe00ed57e3e 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -474,7 +474,7 @@ cargo +nightly -Zunstable-options -Zconfig-include --config somefile.toml build CLI paths are relative to the current working directory. -## Features +### Features * Tracking Issues: * [itarget #7914](https://github.com/rust-lang/cargo/issues/7914) * [build_dep #7915](https://github.com/rust-lang/cargo/issues/7915) @@ -549,6 +549,31 @@ The available options are: * `compare` — This option compares the resolved features to the old resolver, and will print any differences. +### package-features +* Tracking Issue: [#5364](https://github.com/rust-lang/cargo/issues/5364) + +The `-Zpackage-features` flag changes the way features can be passed on the +command-line for a workspace. The normal behavior can be confusing, as the +features passed are always enabled on the package in the current directory, +even if that package is not selected with a `-p` flag. Feature flags also do +not work in the root of a virtual workspace. `-Zpackage-features` tries to +make feature flags behave in a more intuitive manner. + +* `cargo build -p other_member --features …` — This now only enables the given + features as defined in `other_member` (ignores whatever is in the current + directory). +* `cargo build -p a -p b --features …` — This now enables the given features + on both `a` and `b`. Not all packages need to define every feature, it only + enables matching features. It is still an error if none of the packages + define a given feature. +* `--features` and `--no-default-features` are now allowed in the root of a + virtual workspace. +* `member_name/feature_name` syntax may now be used on the command-line to + enable features for a specific member. + +The ability to set features for non-workspace members is no longer allowed, as +the resolver fundamentally does not support that ability. + ### crate-versions * Tracking Issue: [#7907](https://github.com/rust-lang/cargo/issues/7907) diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index e5487610647..ddc289b3b69 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -1358,80 +1358,6 @@ fn many_cli_features_comma_and_space_delimited() { .run(); } -#[cargo_test] -fn combining_features_and_package() { - Package::new("dep", "1.0.0").publish(); - - let p = project() - .file( - "Cargo.toml", - r#" - [project] - name = "foo" - version = "0.0.1" - authors = [] - - [workspace] - members = ["bar"] - - [dependencies] - dep = "1" - "#, - ) - .file("src/lib.rs", "") - .file( - "bar/Cargo.toml", - r#" - [package] - name = "bar" - version = "0.0.1" - authors = [] - [features] - main = [] - "#, - ) - .file( - "bar/src/main.rs", - r#" - #[cfg(feature = "main")] - fn main() {} - "#, - ) - .build(); - - p.cargo("build -Z package-features --workspace --features main") - .masquerade_as_nightly_cargo() - .with_status(101) - .with_stderr_contains("[ERROR] cannot specify features for more than one package") - .run(); - - p.cargo("build -Z package-features --package dep --features main") - .masquerade_as_nightly_cargo() - .with_status(101) - .with_stderr_contains("[ERROR] cannot specify features for packages outside of workspace") - .run(); - p.cargo("build -Z package-features --package dep --all-features") - .masquerade_as_nightly_cargo() - .with_status(101) - .with_stderr_contains("[ERROR] cannot specify features for packages outside of workspace") - .run(); - p.cargo("build -Z package-features --package dep --no-default-features") - .masquerade_as_nightly_cargo() - .with_status(101) - .with_stderr_contains("[ERROR] cannot specify features for packages outside of workspace") - .run(); - - p.cargo("build -Z package-features --workspace --all-features") - .masquerade_as_nightly_cargo() - .run(); - p.cargo("run -Z package-features --package bar --features main") - .masquerade_as_nightly_cargo() - .run(); - p.cargo("build -Z package-features --package dep") - .masquerade_as_nightly_cargo() - .run(); -} - #[cargo_test] fn namespaced_invalid_feature() { let p = project() diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 57a145430af..10f2401fd7c 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -73,6 +73,7 @@ mod offline; mod out_dir; mod owner; mod package; +mod package_features; mod patch; mod path; mod paths; diff --git a/tests/testsuite/package_features.rs b/tests/testsuite/package_features.rs new file mode 100644 index 00000000000..50196ceb523 --- /dev/null +++ b/tests/testsuite/package_features.rs @@ -0,0 +1,472 @@ +//! Tests for -Zpackage-features + +use cargo_test_support::registry::Package; +use cargo_test_support::{basic_manifest, project}; + +#[cargo_test] +fn virtual_no_default_features() { + // --no-default-features in root of virtual workspace. + Package::new("dep1", "1.0.0").publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b"] + "#, + ) + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.1.0" + + [dependencies] + dep1 = {version = "1.0", optional = true} + + [features] + default = ["dep1"] + "#, + ) + .file("a/src/lib.rs", "") + .file( + "b/Cargo.toml", + r#" + [package] + name = "b" + version = "0.1.0" + + [features] + default = ["f1"] + f1 = [] + "#, + ) + .file( + "b/src/lib.rs", + r#" + #[cfg(feature = "f1")] + compile_error!{"expected f1 off"} + "#, + ) + .build(); + + p.cargo("check --no-default-features") + .with_status(101) + .with_stderr( + "\ +[ERROR] --no-default-features is not allowed in the root of a virtual workspace +[NOTE] while this was previously accepted, it didn't actually do anything +", + ) + .run(); + + p.cargo("check --no-default-features -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_stderr_unordered( + "\ +[UPDATING] [..] +[CHECKING] a v0.1.0 [..] +[CHECKING] b v0.1.0 [..] +[FINISHED] [..] +", + ) + .run(); + + p.cargo("check --features foo -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr("[ERROR] none of the selected packages contains these features: foo") + .run(); + + p.cargo("check --features a/dep1,b/f1,b/f2,f2 -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr("[ERROR] none of the selected packages contains these features: b/f2, f2") + .run(); +} + +#[cargo_test] +fn virtual_features() { + // --features in root of virtual workspace. + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b"] + "#, + ) + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.1.0" + + [features] + f1 = [] + "#, + ) + .file( + "a/src/lib.rs", + r#" + #[cfg(not(feature = "f1"))] + compile_error!{"f1 is missing"} + "#, + ) + .file("b/Cargo.toml", &basic_manifest("b", "0.1.0")) + .file("b/src/lib.rs", "") + .build(); + + p.cargo("check --features f1") + .with_status(101) + .with_stderr( + "\ +[ERROR] --features is not allowed in the root of a virtual workspace +[NOTE] while this was previously accepted, it didn't actually do anything +", + ) + .run(); + + p.cargo("check --features f1 -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_stderr_unordered( + "\ +[CHECKING] a [..] +[CHECKING] b [..] +[FINISHED] [..] +", + ) + .run(); +} + +#[cargo_test] +fn virtual_with_specific() { + // -p flags with --features in root of virtual. + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b"] + "#, + ) + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.1.0" + + [features] + f1 = [] + f2 = [] + "#, + ) + .file( + "a/src/lib.rs", + r#" + #[cfg(not_feature = "f1")] + compile_error!{"f1 is missing"} + #[cfg(not_feature = "f2")] + compile_error!{"f2 is missing"} + "#, + ) + .file( + "b/Cargo.toml", + r#" + [package] + name = "b" + version = "0.1.0" + + [features] + f2 = [] + f3 = [] + "#, + ) + .file( + "b/src/lib.rs", + r#" + #[cfg(not_feature = "f2")] + compile_error!{"f2 is missing"} + #[cfg(not_feature = "f3")] + compile_error!{"f3 is missing"} + "#, + ) + .build(); + + p.cargo("check -p a -p b --features f1,f2,f3") + .with_status(101) + .with_stderr( + "\ +[ERROR] --features is not allowed in the root of a virtual workspace +[NOTE] while this was previously accepted, it didn't actually do anything +", + ) + .run(); + + p.cargo("check -p a -p b --features f1,f2,f3 -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_stderr_unordered( + "\ +[CHECKING] a [..] +[CHECKING] b [..] +[FINISHED] [..] +", + ) + .run(); +} + +#[cargo_test] +fn other_member_from_current() { + // -p for another member while in the current directory. + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + bar = { path="bar", features=["f3"] } + + [features] + f1 = ["bar/f4"] + "#, + ) + .file("src/lib.rs", "") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.1.0" + + [features] + f1 = [] + f2 = [] + f3 = [] + f4 = [] + "#, + ) + .file("bar/src/lib.rs", "") + .file( + "bar/src/main.rs", + r#" + fn main() { + if cfg!(feature = "f1") { + print!("f1"); + } + if cfg!(feature = "f2") { + print!("f2"); + } + if cfg!(feature = "f3") { + print!("f3"); + } + if cfg!(feature = "f4") { + print!("f4"); + } + println!(); + } + "#, + ) + .build(); + + p.cargo("run -p bar --features f1") + .with_stdout("f3f4") + .run(); + + p.cargo("run -p bar --features f1 -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_stdout("f1") + .run(); + + p.cargo("run -p bar --features f1,f2") + .with_status(101) + .with_stderr("[ERROR] Package `foo[..]` does not have these features: `f2`") + .run(); + + p.cargo("run -p bar --features f1,f2 -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_stdout("f1f2") + .run(); + + p.cargo("run -p bar --features bar/f1") + .with_stdout("f1f3") + .run(); + + p.cargo("run -p bar --features bar/f1 -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_stdout("f1") + .run(); +} + +#[cargo_test] +fn virtual_member_slash() { + // member slash feature syntax + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["a"] + "#, + ) + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.1.0" + + [dependencies] + b = {path="../b", optional=true} + + [features] + default = ["f1"] + f1 = [] + f2 = [] + "#, + ) + .file( + "a/src/lib.rs", + r#" + #[cfg(feature = "f1")] + compile_error!{"f1 is set"} + + #[cfg(feature = "f2")] + compile_error!{"f2 is set"} + + #[cfg(feature = "b")] + compile_error!{"b is set"} + "#, + ) + .file( + "b/Cargo.toml", + r#" + [package] + name = "b" + version = "0.1.0" + + [features] + bfeat = [] + "#, + ) + .file( + "b/src/lib.rs", + r#" + #[cfg(feature = "bfeat")] + compile_error!{"bfeat is set"} + "#, + ) + .build(); + + p.cargo("check --features a/f1") + .with_status(101) + .with_stderr( + "\ +[ERROR] --features is not allowed in the root of a virtual workspace +[NOTE] while this was previously accepted, it didn't actually do anything +", + ) + .run(); + + p.cargo("check -p a -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains("[..]f1 is set[..]") + .with_stderr_does_not_contain("[..]f2 is set[..]") + .with_stderr_does_not_contain("[..]b is set[..]") + .run(); + + p.cargo("check -p a --features a/f1 -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains("[..]f1 is set[..]") + .with_stderr_does_not_contain("[..]f2 is set[..]") + .with_stderr_does_not_contain("[..]b is set[..]") + .run(); + + p.cargo("check -p a --features a/f2 -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains("[..]f1 is set[..]") + .with_stderr_contains("[..]f2 is set[..]") + .with_stderr_does_not_contain("[..]b is set[..]") + .run(); + + p.cargo("check -p a --features b/bfeat -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains("[..]bfeat is set[..]") + .run(); + + p.cargo("check -p a --no-default-features -Zpackage-features") + .masquerade_as_nightly_cargo() + .run(); + + p.cargo("check -p a --no-default-features --features b -Zpackage-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr_contains("[..]b is set[..]") + .run(); +} + +#[cargo_test] +fn non_member() { + // -p for a non-member + Package::new("dep", "1.0.0").publish(); + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [dependencies] + dep = "1.0" + + [features] + f1 = [] + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build -Zpackage-features -p dep --features f1") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "[UPDATING][..]\n[ERROR] cannot specify features for packages outside of workspace", + ) + .run(); + + p.cargo("build -Zpackage-features -p dep --all-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr("[ERROR] cannot specify features for packages outside of workspace") + .run(); + + p.cargo("build -Zpackage-features -p dep --no-default-features") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr("[ERROR] cannot specify features for packages outside of workspace") + .run(); + + p.cargo("build -Zpackage-features -p dep") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[DOWNLOADING] [..] +[DOWNLOADED] [..] +[COMPILING] dep [..] +[FINISHED] [..] +", + ) + .run(); +}