Skip to content

Commit

Permalink
Enable new behavior of --feature
Browse files Browse the repository at this point in the history
  • Loading branch information
@matklad
matklad committed Apr 18, 2018
1 parent 5ac4ab1 commit 038eec5
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 144 deletions.
2 changes: 0 additions & 2 deletions src/cargo/core/features.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@ 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 @@ -326,7 +325,6 @@ 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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl<'a> RegistryQueryer<'a> {
}
}

#[derive(Clone, Copy)]
#[derive(Clone, Copy, Eq, PartialEq)]
pub enum Method<'a> {
Everything, // equivalent to Required { dev_deps: true, all_features: true, .. }
Required {
Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/workspace.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ 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: 31 additions & 74 deletions src/cargo/ops/resolve.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,86 +217,43 @@ pub fn resolve_with_previous<'a, 'cfg>(
registry.add_sources(&[member.package_id().source_id().clone()])?;
}

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()))),
);
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 {
// Edge case: running `cargo build -p foo`, where `foo` is not a member
// 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());
// 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");
}
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());
summaries.push((summary, method_to_resolve));
}
};
(summary, method)
})
.collect::<Vec<_>>();

let root_replace = ws.root_replace();

Expand Down
28 changes: 12 additions & 16 deletions tests/testsuite/features.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1674,46 +1674,42 @@ fn combining_features_and_package() {
.build();

assert_that(
p.cargo("build -Z package-features --all --features main")
p.cargo("build --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 -Z package-features --package dep --features main")
p.cargo("build --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 -Z package-features --package dep --all-features")
p.cargo("build --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 -Z package-features --package dep --no-default-features")
p.cargo("build --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 -Z package-features --all --all-features")
p.cargo("build --all --all-features")
.masquerade_as_nightly_cargo(),
execs().with_status(0),
);
assert_that(
p.cargo("run -Z package-features --package foo --features main")
p.cargo("run --package foo --features main")
.masquerade_as_nightly_cargo(),
execs().with_status(0),
);
Expand Down
56 changes: 5 additions & 51 deletions tests/testsuite/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2867,13 +2867,7 @@ fn selective_test_optional_dep() {
let p = p.build();

assert_that(
p.cargo("test")
.arg("-v")
.arg("--no-run")
.arg("--features")
.arg("a")
.arg("-p")
.arg("a"),
p.cargo("test -v --no-run --package a"),
execs().with_status(0).with_stderr(
"\
[COMPILING] a v0.0.1 ([..])
Expand Down Expand Up @@ -3060,6 +3054,8 @@ 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 @@ -3137,10 +3133,7 @@ fn pass_correct_cfgs_flags_to_rustdoc() {
let p = p.build();

assert_that(
p.cargo("test")
.arg("--package")
.arg("feature_a")
.arg("--verbose"),
p.cargo("test --package feature_a --verbose"),
execs().with_status(0).with_stderr_contains(
"\
[DOCTEST] feature_a
Expand All @@ -3149,7 +3142,7 @@ fn pass_correct_cfgs_flags_to_rustdoc() {
);

assert_that(
p.cargo("test").arg("--verbose"),
p.cargo("test --verbose"),
execs().with_status(0).with_stderr_contains(
"\
[DOCTEST] foo
Expand Down Expand Up @@ -3199,45 +3192,6 @@ 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

0 comments on commit 038eec5

Please sign in to comment.