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

Allow reexporting of features between packages #712

Merged
merged 2 commits into from
Oct 17, 2014
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
174 changes: 122 additions & 52 deletions src/cargo/core/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,40 +318,7 @@ fn resolve_deps<'a, R: Registry>(parent: &Summary,
method: ResolveMethod,
ctx: &mut Context<'a, R>)
-> CargoResult<()> {
let dev_deps = match method {
ResolveEverything => true,
ResolveRequired(dev_deps, _, _) => dev_deps,
};

// First, filter by dev-dependencies
let deps = parent.get_dependencies();
let deps = deps.iter().filter(|d| d.is_transitive() || dev_deps);

// Second, weed out optional dependencies, but not those required
let (mut feature_deps, used_features) = try!(build_features(parent, method));
let deps = deps.filter(|d| {
!d.is_optional() || feature_deps.remove(&d.get_name().to_string())
}).collect::<Vec<&Dependency>>();

// All features can only point to optional dependencies, in which case they
// should have all been weeded out by the above iteration. Any remaining
// features are bugs in that the package does not actually have those
// features.
if feature_deps.len() > 0 {
let features = feature_deps.iter().map(|s| s.as_slice())
.collect::<Vec<&str>>().connect(", ");
return Err(human(format!("Package `{}` does not have these features: \
`{}`", parent.get_package_id(), features)))
}

// Record what list of features is active for this package.
{
let pkgid = parent.get_package_id().clone();
match ctx.resolve.features.entry(pkgid) {
Occupied(entry) => entry.into_mut(),
Vacant(entry) => entry.set(HashSet::new()),
}.extend(used_features.into_iter());
}
let (deps, features) = try!(resolve_features(parent, method, ctx));

// Recursively resolve all dependencies
for &dep in deps.iter() {
Expand Down Expand Up @@ -409,8 +376,15 @@ fn resolve_deps<'a, R: Registry>(parent: &Summary,
depends on itself",
summary.get_package_id())))
}

// The list of enabled features for this dependency are both those
// listed in the dependency itself as well as any of our own features
// which enabled a feature of this package.
let features = features.find_equiv(&dep.get_name())
.map(|v| v.as_slice())
.unwrap_or(&[]);
try!(resolve_deps(summary,
ResolveRequired(false, dep.get_features(),
ResolveRequired(false, features,
dep.uses_default_features()),
ctx));
if dep.is_transitive() {
Expand All @@ -421,10 +395,81 @@ fn resolve_deps<'a, R: Registry>(parent: &Summary,
Ok(())
}

fn resolve_features<'a, R>(parent: &'a Summary, method: ResolveMethod,
ctx: &mut Context<R>)
-> CargoResult<(Vec<&'a Dependency>,
HashMap<&'a str, Vec<String>>)> {
let dev_deps = match method {
ResolveEverything => true,
ResolveRequired(dev_deps, _, _) => dev_deps,
};

// First, filter by dev-dependencies
let deps = parent.get_dependencies();
let deps = deps.iter().filter(|d| d.is_transitive() || dev_deps);

// Second, weed out optional dependencies, but not those required
let (mut feature_deps, used_features) = try!(build_features(parent, method));
let mut ret = HashMap::new();
let deps = deps.filter(|d| {
!d.is_optional() || feature_deps.contains_key_equiv(&d.get_name())
}).collect::<Vec<&Dependency>>();

// Next, sanitize all requested features by whitelisting all the requested
// features that correspond to optional dependencies
for dep in deps.iter() {
let mut base = feature_deps.pop_equiv(&dep.get_name())
.unwrap_or(Vec::new());
for feature in dep.get_features().iter() {
base.push(feature.clone());
if feature.as_slice().contains("/") {
return Err(human(format!("features in dependencies \
cannot enable features in \
other dependencies: `{}`",
feature)));
}
}
ret.insert(dep.get_name(), base);
}

// All features can only point to optional dependencies, in which case they
// should have all been weeded out by the above iteration. Any remaining
// features are bugs in that the package does not actually have those
// features.
if feature_deps.len() > 0 {
let unknown = feature_deps.keys().map(|s| s.as_slice())
.collect::<Vec<&str>>();
if unknown.len() > 0 {
let features = unknown.connect(", ");
return Err(human(format!("Package `{}` does not have these features: \
`{}`", parent.get_package_id(), features)))
}
}

// Record what list of features is active for this package.
{
let pkgid = parent.get_package_id().clone();
match ctx.resolve.features.entry(pkgid) {
Occupied(entry) => entry.into_mut(),
Vacant(entry) => entry.set(HashSet::new()),
}.extend(used_features.into_iter());
}

Ok((deps, ret))
}

// Returns a pair of (feature dependencies, all used features)
//
// The feature dependencies map is a mapping of package name to list of features
// enabled. Each package should be enabled, and each package should have the
// specified set of features enabled.
//
// The all used features set is the set of features which this local package had
// enabled, which is later used when compiling to instruct the code what
// features were enabled.
fn build_features(s: &Summary, method: ResolveMethod)
-> CargoResult<(HashSet<String>, HashSet<String>)> {
let mut deps = HashSet::new();
-> CargoResult<(HashMap<String, Vec<String>>, HashSet<String>)> {
let mut deps = HashMap::new();
let mut used = HashSet::new();
let mut visited = HashSet::new();
match method {
Expand Down Expand Up @@ -454,26 +499,51 @@ fn build_features(s: &Summary, method: ResolveMethod)
return Ok((deps, used));

fn add_feature(s: &Summary, feat: &str,
deps: &mut HashSet<String>,
deps: &mut HashMap<String, Vec<String>>,
used: &mut HashSet<String>,
visited: &mut HashSet<String>) -> CargoResult<()> {
if feat.is_empty() {
return Ok(())
}
if !visited.insert(feat.to_string()) {
return Err(human(format!("Cyclic feature dependency: feature `{}` \
depends on itself", feat)))
}
used.insert(feat.to_string());
match s.get_features().find_equiv(&feat) {
Some(recursive) => {
for f in recursive.iter() {
try!(add_feature(s, f.as_slice(), deps, used, visited));
if feat.is_empty() { return Ok(()) }

// If this feature is of the form `foo/bar`, then we just lookup package
// `foo` and enable its feature `bar`. Otherwise this feature is of the
// form `foo` and we need to recurse to enable the feature `foo` for our
// own package, which may end up enabling more features or just enabling
// a dependency.
let mut parts = feat.splitn(1, '/');
let feat_or_package = parts.next().unwrap();
match parts.next() {
Some(feat) => {
let package = feat_or_package;
match deps.entry(package.to_string()) {
Occupied(e) => e.into_mut(),
Vacant(e) => e.set(Vec::new()),
}.push(feat.to_string());
}
None => {
let feat = feat_or_package;
if !visited.insert(feat.to_string()) {
return Err(human(format!("Cyclic feature dependency: \
feature `{}` depends on itself",
feat)))
}
used.insert(feat.to_string());
match s.get_features().find_equiv(&feat) {
Some(recursive) => {
for f in recursive.iter() {
try!(add_feature(s, f.as_slice(), deps, used,
visited));
}
}
None => {
match deps.entry(feat.to_string()) {
Occupied(..) => {} // already activated
Vacant(e) => { e.set(Vec::new()); }
}
}
}
visited.remove(&feat.to_string());
}
None => { deps.insert(feat.to_string()); }
}
visited.remove(&feat.to_string());
Ok(())
}
}
Expand Down
17 changes: 11 additions & 6 deletions src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,24 @@ impl Summary {
}
for (feature, list) in features.iter() {
for dep in list.iter() {
if features.find_equiv(&dep.as_slice()).is_some() { continue }
let d = dependencies.iter().find(|d| {
d.get_name() == dep.as_slice()
});
match d {
let mut parts = dep.as_slice().splitn(1, '/');
let dep = parts.next().unwrap();
let is_reexport = parts.next().is_some();
if !is_reexport && features.find_equiv(&dep).is_some() { continue }
match dependencies.iter().find(|d| d.get_name() == dep) {
Some(d) => {
if d.is_optional() { continue }
if d.is_optional() || is_reexport { continue }
return Err(human(format!("Feature `{}` depends on `{}` \
which is not an optional \
dependency.\nConsider adding \
`optional = true` to the \
dependency", feature, dep)))
}
None if is_reexport => {
return Err(human(format!("Feature `{}` requires `{}` \
which is not an optional \
dependency", feature, dep)))
}
None => {
return Err(human(format!("Feature `{}` includes `{}` \
which is neither a dependency \
Expand Down
14 changes: 14 additions & 0 deletions src/cargo/ops/cargo_rustc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,20 @@ fn compile_custom(pkg: &Package, cmd: &str,
for arg in cmd {
p = p.arg(arg);
}
match cx.resolve.features(pkg.get_package_id()) {
Some(features) => {
for feat in features.iter() {
let feat = feat.as_slice().chars()
.map(|c| c.to_uppercase())
.map(|c| if c == '-' {'_'} else {c})
.collect::<String>();
p = p.env(format!("CARGO_FEATURE_{}", feat).as_slice(), Some("1"));
}
}
None => {}
}


for &(pkg, _) in cx.dep_targets(pkg).iter() {
let name: String = pkg.get_name().chars().map(|c| {
match c {
Expand Down
5 changes: 5 additions & 0 deletions src/doc/manifest.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ default = ["jquery", "uglifier"]
# requirements to the feature in the future.
secure-password = ["bcrypt"]

# Features can be used to reexport features of other packages.
# The `session` feature of package `awesome` will ensure that the
# `session` feature of the package `cookie` is also enabled.
session = ["cookie/session"]

[dependencies]

# These packages are mandatory and form the core of this
Expand Down
4 changes: 4 additions & 0 deletions src/doc/native-build.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ commands.
profile currently being built.
* `PROFILE` - name of the profile currently being built (see
[profiles][profile]).
* `CARGO_FEATURE_<name>` - For each activated feature of the package being
built, this environment variable will be present
where `<name>` is the name of the feature uppercased
and having `-` translated to `_`.

[profile]: manifest.html#the-[profile.*]-sections

Expand Down
8 changes: 8 additions & 0 deletions src/snapshots.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
2014-10-16
linux-i386 61417861716cd41d8f372be36bb0572e4f29dec8
linux-x86_64 59be4ff9f547f1ba47ad133ab74151a48bc2659b
macos-i386 cb5267d2e7df8406c26bb0337b1c2e80b125e2cb
macos-x86_64 9283adb4dfd1b60c7bfe38ef755f9187fe7d5580
winnt-i386 88deb2950fa2b73358bc15763e6373ade6325f53
winnt-x86_64 0143d4b0e4b20e84dbb27a4440b4b55d369f4456

2014-09-19
linux-i386 c92895421e6fa170dbd713e74334b8c3cf22b817
linux-x86_64 66ee4126f9e4820cd82e78181931f8ea365904de
Expand Down
13 changes: 11 additions & 2 deletions tests/test_cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,9 @@ test!(custom_build_env_vars {
version = "0.5.0"
authors = ["wycats@example.com"]

[features]
foo = []

[[bin]]
name = "foo"
"#)
Expand All @@ -753,6 +756,7 @@ test!(custom_build_env_vars {
use std::io::fs::PathExtensions;
fn main() {{
let _ncpus = os::getenv("NUM_JOBS").unwrap();
let _feat = os::getenv("CARGO_FEATURE_FOO").unwrap();
let debug = os::getenv("DEBUG").unwrap();
assert_eq!(debug.as_slice(), "true");

Expand All @@ -777,7 +781,8 @@ test!(custom_build_env_vars {
}}
"#,
p.root().join("target").join("native").display()));
assert_that(build.cargo_process("build"), execs().with_status(0));
assert_that(build.cargo_process("build").arg("--features").arg("foo"),
execs().with_status(0));


p = p
Expand All @@ -789,6 +794,9 @@ test!(custom_build_env_vars {
authors = ["wycats@example.com"]
build = '{}'

[features]
foo = []

[[bin]]
name = "foo"

Expand All @@ -798,7 +806,8 @@ test!(custom_build_env_vars {
.file("src/foo.rs", r#"
fn main() {}
"#);
assert_that(p.cargo_process("build"), execs().with_status(0));
assert_that(p.cargo_process("build").arg("--features").arg("foo"),
execs().with_status(0));
})

test!(crate_version_env_vars {
Expand Down
Loading