Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Enable new behavior of --feature" #5430

Merged
merged 1 commit into from
Apr 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/cargo/core/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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),
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
105 changes: 74 additions & 31 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>();
summaries.push((summary, method_to_resolve));
}
};

let root_replace = ws.root_replace();

Expand Down
28 changes: 16 additions & 12 deletions tests/testsuite/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
);
Expand Down
56 changes: 51 additions & 5 deletions tests/testsuite/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ([..])
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand Down