diff --git a/src/cargo/core/resolver.rs b/src/cargo/core/resolver.rs index d1176808b51..bdf5ddaa0cf 100644 --- a/src/cargo/core/resolver.rs +++ b/src/cargo/core/resolver.rs @@ -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::>(); - - // 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::>().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() { @@ -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() { @@ -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) + -> CargoResult<(Vec<&'a Dependency>, + HashMap<&'a str, Vec>)> { + 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::>(); + + // 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::>(); + 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, HashSet)> { - let mut deps = HashSet::new(); + -> CargoResult<(HashMap>, HashSet)> { + let mut deps = HashMap::new(); let mut used = HashSet::new(); let mut visited = HashSet::new(); match method { @@ -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, + deps: &mut HashMap>, used: &mut HashSet, visited: &mut HashSet) -> 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(()) } } diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index c69fc950017..582f988f001 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -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 \ diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index c035441950d..45d51b9dc58 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -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::(); + 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 { diff --git a/src/doc/manifest.md b/src/doc/manifest.md index 587654cf0e6..2410bcf322b 100644 --- a/src/doc/manifest.md +++ b/src/doc/manifest.md @@ -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 diff --git a/src/doc/native-build.md b/src/doc/native-build.md index b06f9679369..da6dcf56457 100644 --- a/src/doc/native-build.md +++ b/src/doc/native-build.md @@ -84,6 +84,10 @@ commands. profile currently being built. * `PROFILE` - name of the profile currently being built (see [profiles][profile]). +* `CARGO_FEATURE_` - For each activated feature of the package being + built, this environment variable will be present + where `` is the name of the feature uppercased + and having `-` translated to `_`. [profile]: manifest.html#the-[profile.*]-sections diff --git a/src/snapshots.txt b/src/snapshots.txt index c2913ffe216..7e08df139b8 100644 --- a/src/snapshots.txt +++ b/src/snapshots.txt @@ -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 diff --git a/tests/test_cargo_compile.rs b/tests/test_cargo_compile.rs index b0ff9d7165d..21caae85202 100644 --- a/tests/test_cargo_compile.rs +++ b/tests/test_cargo_compile.rs @@ -745,6 +745,9 @@ test!(custom_build_env_vars { version = "0.5.0" authors = ["wycats@example.com"] + [features] + foo = [] + [[bin]] name = "foo" "#) @@ -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"); @@ -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 @@ -789,6 +794,9 @@ test!(custom_build_env_vars { authors = ["wycats@example.com"] build = '{}' + [features] + foo = [] + [[bin]] name = "foo" @@ -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 { diff --git a/tests/test_cargo_features.rs b/tests/test_cargo_features.rs index 64fb2de91c8..cc0e8ab5a9a 100644 --- a/tests/test_cargo_features.rs +++ b/tests/test_cargo_features.rs @@ -137,6 +137,76 @@ Dev-dependencies are not allowed to be optional: `bar` ").as_slice())); }) +test!(invalid6 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + foo = ["bar/baz"] + "#) + .file("src/main.rs", ""); + + assert_that(p.cargo_process("build").arg("--features").arg("foo"), + execs().with_status(101).with_stderr(format!("\ +Cargo.toml is not a valid manifest + +Feature `foo` requires `bar` which is not an optional dependency +").as_slice())); +}) + +test!(invalid7 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + foo = ["bar/baz"] + bar = [] + "#) + .file("src/main.rs", ""); + + assert_that(p.cargo_process("build").arg("--features").arg("foo"), + execs().with_status(101).with_stderr(format!("\ +Cargo.toml is not a valid manifest + +Feature `foo` requires `bar` which is not an optional dependency +").as_slice())); +}) + +test!(invalid8 { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + path = "bar" + features = ["foo/bar"] + "#) + .file("src/main.rs", "") + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + "#) + .file("bar/src/lib.rs", ""); + + assert_that(p.cargo_process("build").arg("--features").arg("foo"), + execs().with_status(101).with_stderr(format!("\ +features in dependencies cannot enable features in other dependencies: `foo/bar` +").as_slice())); +}) + test!(no_feature_doesnt_build { let p = project("foo") .file("Cargo.toml", r#" @@ -477,3 +547,40 @@ test!(empty_features { assert_that(p.cargo_process("build").arg("--features").arg(""), execs().with_status(0)); }) + +// Tests that all cmd lines work with `--features ""` +test!(transitive_features { + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [features] + foo = ["bar/baz"] + + [dependencies.bar] + path = "bar" + "#) + .file("src/main.rs", " + extern crate bar; + fn main() { bar::baz(); } + ") + .file("bar/Cargo.toml", r#" + [package] + name = "bar" + version = "0.0.1" + authors = [] + + [features] + baz = [] + "#) + .file("bar/src/lib.rs", r#" + #[cfg(feature = "baz")] + pub fn baz() {} + "#); + + assert_that(p.cargo_process("build").arg("--features").arg("foo"), + execs().with_status(0)); +})