Skip to content

Commit

Permalink
Merge -Zpackage-features2 and -Zpackage-features.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Apr 8, 2020
1 parent c17bfcb commit 54ace8a
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 112 deletions.
2 changes: 0 additions & 2 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,6 @@ pub struct CliUnstable {
pub avoid_dev_deps: bool,
pub minimal_versions: bool,
pub package_features: bool,
pub package_features2: bool,
pub advanced_env: bool,
pub config_include: bool,
pub dual_proc_macros: bool,
Expand Down Expand Up @@ -405,7 +404,6 @@ impl CliUnstable {
"avoid-dev-deps" => self.avoid_dev_deps = parse_empty(k, v)?,
"minimal-versions" => self.minimal_versions = parse_empty(k, v)?,
"package-features" => self.package_features = parse_empty(k, v)?,
"package-features2" => self.package_features2 = parse_empty(k, v)?,
"advanced-env" => self.advanced_env = parse_empty(k, v)?,
"config-include" => self.config_include = parse_empty(k, v)?,
"dual-proc-macros" => self.dual_proc_macros = parse_empty(k, v)?,
Expand Down
16 changes: 3 additions & 13 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,36 +878,26 @@ impl<'cfg> Workspace<'cfg> {
.map(|m| (m, RequestedFeatures::new_all(true)))
.collect());
}
if self.config().cli_unstable().package_features
|| self.config().cli_unstable().package_features2
{
if self.config().cli_unstable().package_features {
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 or -Zpackage-features2.
/// New command-line feature selection with -Zpackage-features.
fn members_with_features_pf(
&self,
specs: &[PackageIdSpec],
requested_features: &RequestedFeatures,
) -> CargoResult<Vec<(&Package, RequestedFeatures)>> {
let pf2 = self.config().cli_unstable().package_features2;
if specs.len() > 1 && !requested_features.features.is_empty() && !pf2 {
anyhow::bail!("cannot specify features for more than one package");
}
// Keep track of which features matched *any* member, to produce an error
// if any of them did not match anywhere.
let mut found: BTreeSet<InternedString> = 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 {
// This new behavior is only enabled for -Zpackage-features2
if !pf2 {
return requested_features.clone();
}
if requested_features.features.is_empty() || requested_features.all_features {
return requested_features.clone();
}
Expand Down Expand Up @@ -979,7 +969,7 @@ impl<'cfg> Workspace<'cfg> {
.map(|m| (m, RequestedFeatures::new_all(false)))
.collect());
}
if pf2 && *requested_features.features != found {
if *requested_features.features != found {
let missing: Vec<_> = requested_features
.features
.difference(&found)
Expand Down
4 changes: 1 addition & 3 deletions src/cargo/util/command_prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,7 @@ pub trait ArgMatchesExt {
if config.cli_unstable().avoid_dev_deps {
ws.set_require_optional_deps(false);
}
let unstable =
config.cli_unstable().package_features || config.cli_unstable().package_features2;
if ws.is_virtual() && !unstable {
if ws.is_virtual() && !config.cli_unstable().package_features {
// --all-features is actually honored. In general, workspaces and
// feature flags are a bit of a mess right now.
for flag in &["features", "no-default-features"] {
Expand Down
8 changes: 4 additions & 4 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -549,14 +549,14 @@ The available options are:
* `compare` — This option compares the resolved features to the old resolver,
and will print any differences.

### package-features2
* Tracking Issue: TODO
### package-features
* Tracking Issue: [#5364](https:/rust-lang/cargo/issues/5364)

The `-Zpackage-features2` flag changes the way features can be passed on the
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-features2` tries to
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
Expand Down
74 changes: 0 additions & 74 deletions tests/testsuite/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ mod offline;
mod out_dir;
mod owner;
mod package;
mod package_features2;
mod package_features;
mod patch;
mod path;
mod paths;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Tests for -Zpackage-features2
//! Tests for -Zpackage-features

use cargo_test_support::registry::Package;
use cargo_test_support::{basic_manifest, project};
Expand Down Expand Up @@ -61,7 +61,7 @@ fn virtual_no_default_features() {
)
.run();

p.cargo("check --no-default-features -Zpackage-features2")
p.cargo("check --no-default-features -Zpackage-features")
.masquerade_as_nightly_cargo()
.with_stderr_unordered(
"\
Expand All @@ -73,13 +73,13 @@ fn virtual_no_default_features() {
)
.run();

p.cargo("check --features foo -Zpackage-features2")
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-features2")
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")
Expand Down Expand Up @@ -129,7 +129,7 @@ fn virtual_features() {
)
.run();

p.cargo("check --features f1 -Zpackage-features2")
p.cargo("check --features f1 -Zpackage-features")
.masquerade_as_nightly_cargo()
.with_stderr_unordered(
"\
Expand Down Expand Up @@ -206,7 +206,7 @@ fn virtual_with_specific() {
)
.run();

p.cargo("check -p a -p b --features f1,f2,f3 -Zpackage-features2")
p.cargo("check -p a -p b --features f1,f2,f3 -Zpackage-features")
.masquerade_as_nightly_cargo()
.with_stderr_unordered(
"\
Expand Down Expand Up @@ -280,7 +280,7 @@ fn other_member_from_current() {
.with_stdout("f3f4")
.run();

p.cargo("run -p bar --features f1 -Zpackage-features2")
p.cargo("run -p bar --features f1 -Zpackage-features")
.masquerade_as_nightly_cargo()
.with_stdout("f1")
.run();
Expand All @@ -290,7 +290,7 @@ fn other_member_from_current() {
.with_stderr("[ERROR] Package `foo[..]` does not have these features: `f2`")
.run();

p.cargo("run -p bar --features f1,f2 -Zpackage-features2")
p.cargo("run -p bar --features f1,f2 -Zpackage-features")
.masquerade_as_nightly_cargo()
.with_stdout("f1f2")
.run();
Expand All @@ -299,7 +299,7 @@ fn other_member_from_current() {
.with_stdout("f1f3")
.run();

p.cargo("run -p bar --features bar/f1 -Zpackage-features2")
p.cargo("run -p bar --features bar/f1 -Zpackage-features")
.masquerade_as_nightly_cargo()
.with_stdout("f1")
.run();
Expand Down Expand Up @@ -375,43 +375,98 @@ fn virtual_member_slash() {
)
.run();

p.cargo("check -p a -Zpackage-features2")
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-features2")
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-features2")
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-features2")
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-features2")
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-features2")
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();
}

0 comments on commit 54ace8a

Please sign in to comment.