From d369f97c19924e025a827cc2c28f0e8630511f7e Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 28 Apr 2018 17:28:39 +0300 Subject: [PATCH] Revert "Enable new behavior of `--feature`" This reverts commit 038eec5cb3bd25a0855b0be6ad2aeba5391c6c6e. --- src/cargo/core/features.rs | 2 + src/cargo/core/resolver/types.rs | 2 +- src/cargo/core/workspace.rs | 1 - src/cargo/ops/resolve.rs | 105 ++++++++++++++++++++++--------- tests/testsuite/features.rs | 28 +++++---- tests/testsuite/test.rs | 56 +++++++++++++++-- 6 files changed, 144 insertions(+), 50 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index b38c0950e9e..c14ed59dc50 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -299,6 +299,7 @@ pub struct CliUnstable { pub no_index_update: bool, pub avoid_dev_deps: bool, pub minimal_versions: bool, + pub package_features: bool, } impl CliUnstable { @@ -332,6 +333,7 @@ impl CliUnstable { "no-index-update" => self.no_index_update = true, "avoid-dev-deps" => self.avoid_dev_deps = true, "minimal-versions" => self.minimal_versions = true, + "package-features" => self.package_features = true, _ => bail!("unknown `-Z` flag specified: {}", k), } diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index bd552745f66..543c72e7e4a 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -155,7 +155,7 @@ impl<'a> RegistryQueryer<'a> { } } -#[derive(Clone, Copy, Eq, PartialEq)] +#[derive(Clone, Copy)] pub enum Method<'a> { Everything, // equivalent to Required { dev_deps: true, all_features: true, .. } Required { diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index cd5bc220a3c..679daaddd97 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -118,7 +118,6 @@ pub struct WorkspaceRootConfig { /// An iterator over the member packages of a workspace, returned by /// `Workspace::members` -#[derive(Clone)] pub struct Members<'a, 'cfg: 'a> { ws: &'a Workspace<'cfg>, iter: slice::Iter<'a, PathBuf>, diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 7801f591225..8df8e4da6b4 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -217,43 +217,86 @@ pub fn resolve_with_previous<'a, 'cfg>( registry.add_sources(&[member.package_id().source_id().clone()])?; } - let method = match method { - Method::Everything => Method::Everything, - Method::Required { - features, - all_features, - uses_default_features, - .. - } => { - if specs.len() > 1 && !features.is_empty() { - bail!("cannot specify features for more than one package"); - } - let members_requested = ws.members() - .filter(|m| specs.iter().any(|spec| spec.matches(m.package_id()))) - .count(); - if members_requested == 0 { + let mut summaries = Vec::new(); + if ws.config().cli_unstable().package_features { + let mut members = Vec::new(); + match method { + Method::Everything => members.extend(ws.members()), + Method::Required { + features, + all_features, + uses_default_features, + .. + } => { + if specs.len() > 1 && !features.is_empty() { + bail!("cannot specify features for more than one package"); + } + members.extend( + ws.members() + .filter(|m| specs.iter().any(|spec| spec.matches(m.package_id()))), + ); // Edge case: running `cargo build -p foo`, where `foo` is not a member - // of current workspace. Resolve whole workspace to get `foo` into the - // resolution graph. - if !(features.is_empty() && !all_features && uses_default_features) { - bail!("cannot specify features for packages outside of workspace"); + // of current workspace. Add all packages from workspace to get `foo` + // into the resolution graph. + if members.is_empty() { + if !(features.is_empty() && !all_features && uses_default_features) { + bail!("cannot specify features for packages outside of workspace"); + } + members.extend(ws.members()); } - Method::Everything - } else { - method } } - }; + for member in members { + let summary = registry.lock(member.summary().clone()); + summaries.push((summary, method)) + } + } else { + for member in ws.members() { + let method_to_resolve = match method { + // When everything for a workspace we want to be sure to resolve all + // members in the workspace, so propagate the `Method::Everything`. + Method::Everything => Method::Everything, + + // If we're not resolving everything though then we're constructing the + // exact crate graph we're going to build. Here we don't necessarily + // want to keep around all workspace crates as they may not all be + // built/tested. + // + // Additionally, the `method` specified represents command line + // flags, which really only matters for the current package + // (determined by the cwd). If other packages are specified (via + // `-p`) then the command line flags like features don't apply to + // them. + // + // As a result, if this `member` is the current member of the + // workspace, then we use `method` specified. Otherwise we use a + // base method with no features specified but using default features + // for any other packages specified with `-p`. + Method::Required { dev_deps, .. } => { + let base = Method::Required { + dev_deps, + features: &[], + all_features: false, + uses_default_features: true, + }; + let member_id = member.package_id(); + match ws.current_opt() { + Some(current) if member_id == current.package_id() => method, + _ => { + if specs.iter().any(|spec| spec.matches(member_id)) { + base + } else { + continue; + } + } + } + } + }; - let summaries = ws.members() - .filter(|m| { - method == Method::Everything || specs.iter().any(|spec| spec.matches(m.package_id())) - }) - .map(|member| { let summary = registry.lock(member.summary().clone()); - (summary, method) - }) - .collect::>(); + summaries.push((summary, method_to_resolve)); + } + }; let root_replace = ws.root_replace(); diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index 42493b9b68a..e224b45074b 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -1674,42 +1674,46 @@ fn combining_features_and_package() { .build(); assert_that( - p.cargo("build --all --features main") + p.cargo("build -Z package-features --all --features main") .masquerade_as_nightly_cargo(), - execs() - .with_status(101) - .with_stderr_contains("[ERROR] cannot specify features for more than one package"), + execs().with_status(101).with_stderr_contains( + "\ + [ERROR] cannot specify features for more than one package", + ), ); assert_that( - p.cargo("build --package dep --features main") + p.cargo("build -Z package-features --package dep --features main") .masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr_contains( - "[ERROR] cannot specify features for packages outside of workspace", + "\ + [ERROR] cannot specify features for packages outside of workspace", ), ); assert_that( - p.cargo("build --package dep --all-features") + p.cargo("build -Z package-features --package dep --all-features") .masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr_contains( - "[ERROR] cannot specify features for packages outside of workspace", + "\ + [ERROR] cannot specify features for packages outside of workspace", ), ); assert_that( - p.cargo("build --package dep --no-default-features") + p.cargo("build -Z package-features --package dep --no-default-features") .masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr_contains( - "[ERROR] cannot specify features for packages outside of workspace", + "\ + [ERROR] cannot specify features for packages outside of workspace", ), ); assert_that( - p.cargo("build --all --all-features") + p.cargo("build -Z package-features --all --all-features") .masquerade_as_nightly_cargo(), execs().with_status(0), ); assert_that( - p.cargo("run --package foo --features main") + p.cargo("run -Z package-features --package foo --features main") .masquerade_as_nightly_cargo(), execs().with_status(0), ); diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index 315e886c506..36c2f9b369e 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -2863,7 +2863,13 @@ fn selective_test_optional_dep() { let p = p.build(); assert_that( - p.cargo("test -v --no-run --package a"), + p.cargo("test") + .arg("-v") + .arg("--no-run") + .arg("--features") + .arg("a") + .arg("-p") + .arg("a"), execs().with_status(0).with_stderr( "\ [COMPILING] a v0.0.1 ([..]) @@ -3050,8 +3056,6 @@ fn pass_correct_cfgs_flags_to_rustdoc() { version = "0.1.0" authors = [] - [workspace] - [features] default = ["feature_a/default"] nightly = ["feature_a/nightly"] @@ -3129,7 +3133,10 @@ fn pass_correct_cfgs_flags_to_rustdoc() { let p = p.build(); assert_that( - p.cargo("test --package feature_a --verbose"), + p.cargo("test") + .arg("--package") + .arg("feature_a") + .arg("--verbose"), execs().with_status(0).with_stderr_contains( "\ [DOCTEST] feature_a @@ -3138,7 +3145,7 @@ fn pass_correct_cfgs_flags_to_rustdoc() { ); assert_that( - p.cargo("test --verbose"), + p.cargo("test").arg("--verbose"), execs().with_status(0).with_stderr_contains( "\ [DOCTEST] foo @@ -3188,6 +3195,45 @@ fn test_release_ignore_panic() { assert_that(p.cargo("bench").arg("-v"), execs().with_status(0)); } +#[test] +fn test_many_with_features() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + a = { path = "a" } + + [features] + foo = [] + + [workspace] + "#, + ) + .file("src/lib.rs", "") + .file( + "a/Cargo.toml", + r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + "#, + ) + .file("a/src/lib.rs", "") + .build(); + + assert_that( + p.cargo("test -v -p a -p foo --features foo"), + execs().with_status(0), + ); +} + #[test] fn test_all_workspace() { let p = project("foo")