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.

Sign up for GitHub

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

New semantics for --features flag #5353

Merged
merged 1 commit into from
Apr 13, 2018
+158 −41
Merged

New semantics for --features flag #5353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,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 @@ -325,6 +326,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
115 changes: 74 additions & 41 deletions src/cargo/ops/resolve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,53 +213,86 @@ pub fn resolve_with_previous<'a, 'cfg>(
registry.lock_patches();
}

let mut summaries = Vec::new();

for member in ws.members() {
registry.add_sources(&[member.package_id().source_id().clone()])?;
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 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. 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());
}
}
};
}
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 summary = registry.lock(member.summary().clone());
summaries.push((summary, method_to_resolve));
}
};

let summary = registry.lock(member.summary().clone());
summaries.push((summary, method_to_resolve));
}

let root_replace = ws.root_replace();

Expand Down
82 changes: 82 additions & 0 deletions tests/testsuite/features.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use std::io::prelude::*;

use cargotest::support::paths::CargoPathExt;
use cargotest::support::{execs, project};
use cargotest::ChannelChanger;
use hamcrest::assert_that;
use cargotest::support::registry::Package;

#[test]
fn invalid1() {
Expand Down Expand Up @@ -1629,3 +1631,83 @@ fn many_cli_features_comma_and_space_delimited() {
)),
);
}

#[test]
fn combining_features_and_package() {
Package::new("dep", "1.0.0").publish();

let p = project("ws")
.file(
"Cargo.toml",
r#"
[project]
name = "ws"
version = "0.0.1"
authors = []

[workspace]
members = ["foo"]

[dependencies]
dep = "1"
"#,
)
.file("src/lib.rs", "")
.file(
"foo/Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
[features]
main = []
"#,
)
.file("foo/src/main.rs", r#"
#[cfg(feature = "main")]
fn main() {}
"#)
.build();

assert_that(
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"
),
);

assert_that(
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"
),
);
assert_that(
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"
),
);
assert_that(
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"
),
);

assert_that(
p.cargo("build -Z package-features --all --all-features")
.masquerade_as_nightly_cargo(),
execs().with_status(0),
);
assert_that(
p.cargo("run -Z package-features --package foo --features main")
.masquerade_as_nightly_cargo(),
execs().with_status(0),
);
}