From e357d174841c9e373ce3367918dda4feccee3fcb Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 5 Apr 2018 15:22:37 +0200 Subject: [PATCH 1/8] Simplify retrieval of dependency data --- src/cargo/core/summary.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 1392aa36f64..a824233f950 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -145,11 +145,12 @@ fn build_feature_map( // Find data for the referenced dependency... let dep_data = { - let dep_name = match val { - Feature(_) => "", - Crate(ref dep_name) | CrateFeature(ref dep_name, _) => dep_name, - }; - dependencies.iter().find(|d| *d.name() == *dep_name) + match val { + Feature(_) => None, + Crate(ref dep_name) | CrateFeature(ref dep_name, _) => { + dependencies.iter().find(|d| d.name() == *dep_name) + } + } }; match (&val, dep_data) { From 4966f5394fbd562db1c7f069018a4e5781eadb7e Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 5 Apr 2018 16:23:11 +0200 Subject: [PATCH 2/8] Make a dependencies map while building feature map --- src/cargo/core/summary.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index a824233f950..d51db6b8898 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use std::mem; use std::rc::Rc; @@ -133,6 +133,11 @@ fn build_feature_map( dependencies: &[Dependency], ) -> CargoResult { use self::FeatureValue::*; + let dep_map: HashMap<_, _> = dependencies + .iter() + .map(|d| (d.name().as_str(), d)) + .collect(); + let mut map = BTreeMap::new(); for (feature, list) in features.iter() { let mut values = vec![]; @@ -148,7 +153,7 @@ fn build_feature_map( match val { Feature(_) => None, Crate(ref dep_name) | CrateFeature(ref dep_name, _) => { - dependencies.iter().find(|d| d.name() == *dep_name) + dep_map.get(dep_name.as_str()) } } }; From 797d4534243c2da8dfc0240a0976f38fcc6e45a1 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 2 Oct 2017 09:30:22 +0200 Subject: [PATCH 3/8] Add namespaced-features option for manifest [project] section --- src/cargo/util/toml/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 28cebea35d3..da390b4a6fb 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -543,6 +543,8 @@ pub struct TomlProject { autoexamples: Option, autotests: Option, autobenches: Option, + #[serde(rename = "namespaced-features")] + namespaced_features: Option, // package metadata description: Option, From a9f163e390377105b7ecf3a29a05f9631f898e3a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 2 Oct 2017 11:00:18 +0200 Subject: [PATCH 4/8] Keep track of namespaced-features flag in Summary objects For now, all Summaries from a registry have it set to false. --- src/cargo/core/summary.rs | 6 ++++++ src/cargo/sources/registry/index.rs | 2 +- src/cargo/util/toml/mod.rs | 1 + tests/testsuite/resolve.rs | 16 +++++++++++----- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index d51db6b8898..cfe1407edd9 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -26,6 +26,7 @@ struct Inner { features: FeatureMap, checksum: Option, links: Option, + namespaced_features: bool, } impl Summary { @@ -34,6 +35,7 @@ impl Summary { dependencies: Vec, features: BTreeMap>, links: Option, + namespaced_features: bool, ) -> CargoResult { for dep in dependencies.iter() { if features.get(&*dep.name()).is_some() { @@ -58,6 +60,7 @@ impl Summary { features: feature_map, checksum: None, links: links.map(|l| InternedString::new(&l)), + namespaced_features, }), }) } @@ -86,6 +89,9 @@ impl Summary { pub fn links(&self) -> Option { self.inner.links } + pub fn namespaced_features(&self) -> bool { + self.inner.namespaced_features + } pub fn override_id(mut self, id: PackageId) -> Summary { Rc::make_mut(&mut self.inner).package_id = id; diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index f201250c09f..a8e887d9616 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -153,7 +153,7 @@ impl<'cfg> RegistryIndex<'cfg> { let deps = deps.into_iter() .map(|dep| dep.into_dep(&self.source_id)) .collect::>>()?; - let summary = Summary::new(pkgid, deps, features, links)?; + let summary = Summary::new(pkgid, deps, features, links, false)?; let summary = summary.set_checksum(cksum.clone()); if self.hashes.contains_key(&name[..]) { self.hashes.get_mut(&name[..]).unwrap().insert(vers, cksum); diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index da390b4a6fb..5ef05ad009b 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -845,6 +845,7 @@ impl TomlManifest { deps, me.features.clone().unwrap_or_else(BTreeMap::new), project.links.clone(), + project.namespaced_features.unwrap_or(false), )?; let metadata = ManifestMetadata { description: project.description.clone(), diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index a8bf3545b25..e6377a5418a 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -44,7 +44,7 @@ fn resolve_with_config( } } let mut registry = MyRegistry(registry); - let summary = Summary::new(pkg.clone(), deps, BTreeMap::new(), None).unwrap(); + let summary = Summary::new(pkg.clone(), deps, BTreeMap::new(), None, false).unwrap(); let method = Method::Everything; let resolve = resolver::resolve( &[(summary, method)], @@ -106,13 +106,13 @@ macro_rules! pkg { let pkgid = $pkgid.to_pkgid(); let link = if pkgid.name().ends_with("-sys") {Some(pkgid.name().to_string())} else {None}; - Summary::new(pkgid, d, BTreeMap::new(), link).unwrap() + Summary::new(pkgid, d, BTreeMap::new(), link, false).unwrap() }); ($pkgid:expr) => ({ let pkgid = $pkgid.to_pkgid(); let link = if pkgid.name().ends_with("-sys") {Some(pkgid.name().to_string())} else {None}; - Summary::new(pkgid, Vec::new(), BTreeMap::new(), link).unwrap() + Summary::new(pkgid, Vec::new(), BTreeMap::new(), link, false).unwrap() }) } @@ -127,7 +127,7 @@ fn pkg(name: &str) -> Summary { } else { None }; - Summary::new(pkg_id(name), Vec::new(), BTreeMap::new(), link).unwrap() + Summary::new(pkg_id(name), Vec::new(), BTreeMap::new(), link, false).unwrap() } fn pkg_id(name: &str) -> PackageId { @@ -148,7 +148,13 @@ fn pkg_loc(name: &str, loc: &str) -> Summary { } else { None }; - Summary::new(pkg_id_loc(name, loc), Vec::new(), BTreeMap::new(), link).unwrap() + Summary::new( + pkg_id_loc(name, loc), + Vec::new(), + BTreeMap::new(), + link, + false, + ).unwrap() } fn dep(name: &str) -> Dependency { From cb533ae1bf363606994be75eb533ccc7a542a81a Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Thu, 5 Apr 2018 16:59:51 +0200 Subject: [PATCH 5/8] Take feature namespace into account while building summary (fixes #1286) Here's an attempt at a table to cover the different cases: Feature Old (must be in features table) Continue Namespaced (might be stray value) In features table: Check that Crate dependency is in the list -> Non-optional dependency: Bail [PREVIOUSLY: bailed for non-optional dependency] -> Optional dependency: Insert feature of this name -> Else: Bail [PREVIOUSLY: bailed for unknown dependency or feature] Crate Old (might be stray value) Non-optional dependency: Bail No dependency found: Bail Namespaced Non-optional dependency: Bail No dependency found: Bail CrateFeature Old No dependency found: Bail Namespaced No dependency found: Bail --- src/cargo/core/summary.rs | 200 +++++++++++++++++++++++++++++++------- src/cargo/ops/registry.rs | 5 +- 2 files changed, 168 insertions(+), 37 deletions(-) diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index cfe1407edd9..22c923ad904 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -38,7 +38,7 @@ impl Summary { namespaced_features: bool, ) -> CargoResult { for dep in dependencies.iter() { - if features.get(&*dep.name()).is_some() { + if !namespaced_features && features.get(&*dep.name()).is_some() { bail!( "Features and dependencies cannot have the \ same name: `{}`", @@ -52,7 +52,7 @@ impl Summary { ) } } - let feature_map = build_feature_map(features, &dependencies)?; + let feature_map = build_feature_map(features, &dependencies, namespaced_features)?; Ok(Summary { inner: Rc::new(Inner { package_id: pkg_id, @@ -137,6 +137,7 @@ impl PartialEq for Summary { fn build_feature_map( features: BTreeMap>, dependencies: &[Dependency], + namespaced: bool, ) -> CargoResult { use self::FeatureValue::*; let dep_map: HashMap<_, _> = dependencies @@ -146,52 +147,169 @@ fn build_feature_map( let mut map = BTreeMap::new(); for (feature, list) in features.iter() { + // If namespaced features is active and the key is the same as that of an + // optional dependency, that dependency must be included in the values. + // Thus, if a `feature` is found that has the same name as a dependency, we + // (a) bail out if the dependency is non-optional, and (b) we track if the + // feature requirements include the dependency `crate:feature` in the list. + // This is done with the `dependency_found` variable, which can only be + // false if features are namespaced and the current feature key is the same + // as the name of an optional dependency. If so, it gets set to true during + // iteration over the list if the dependency is found in the list. + let mut dependency_found = if namespaced { + match dep_map.get(feature.as_str()) { + Some(ref dep_data) => { + if !dep_data.is_optional() { + bail!( + "Feature `{}` includes the dependency of the same name, but this is \ + left implicit in the features included by this feature.\n\ + Additionally, the dependency must be marked as optional to be \ + included in the feature definition.\n\ + Consider adding `crate:{}` to this feature's requirements \ + and marking the dependency as `optional = true`", + feature, + feature + ) + } else { + false + } + } + None => true, + } + } else { + true + }; + let mut values = vec![]; for dep in list { - let val = FeatureValue::build(InternedString::new(dep), |fs| features.contains_key(fs)); - if let &Feature(_) = &val { - values.push(val); - continue; - } + let val = FeatureValue::build( + InternedString::new(dep), + |fs| features.contains_key(fs), + namespaced, + ); // Find data for the referenced dependency... let dep_data = { match val { - Feature(_) => None, - Crate(ref dep_name) | CrateFeature(ref dep_name, _) => { + Feature(ref dep_name) | Crate(ref dep_name) | CrateFeature(ref dep_name, _) => { dep_map.get(dep_name.as_str()) } } }; + let is_optional_dep = dep_data.map_or(false, |d| d.is_optional()); + if let FeatureValue::Crate(ref dep_name) = val { + // If we have a dependency value, check if this is the dependency named + // the same as the feature that we were looking for. + if !dependency_found && feature == dep_name.as_str() { + dependency_found = true; + } + } - match (&val, dep_data) { - (&Crate(ref dep), Some(d)) => { - if !d.is_optional() { - bail!( - "Feature `{}` depends on `{}` which is not an \ - optional dependency.\nConsider adding \ - `optional = true` to the dependency", - feature, - dep - ) + match (&val, dep_data.is_some(), is_optional_dep) { + // The value is a feature. If features are namespaced, this just means + // it's not prefixed with `crate:`, so we have to check whether the + // feature actually exist. If the feature is not defined *and* an optional + // dependency of the same name exists, the feature is defined implicitly + // here by adding it to the feature map, pointing to the dependency. + // If features are not namespaced, it's been validated as a feature already + // while instantiating the `FeatureValue` in `FeatureValue::build()`, so + // we don't have to do so here. + (&Feature(feat), _, true) => { + if namespaced && !features.contains_key(&*feat) { + map.insert(feat.to_string(), vec![FeatureValue::Crate(feat)]); } } - (&CrateFeature(ref dep_name, _), None) => bail!( + // If features are namespaced and the value is not defined as a feature + // and there is no optional dependency of the same name, error out. + // If features are not namespaced, there must be an existing feature + // here (checked by `FeatureValue::build()`), so it will always be defined. + (&Feature(feat), dep_exists, false) => { + if namespaced && !features.contains_key(&*feat) { + if dep_exists { + bail!( + "Feature `{}` includes `{}` which is not defined as a feature.\n\ + A non-optional dependency of the same name is defined; consider \ + adding `optional = true` to its definition", + feature, + feat + ) + } else { + bail!( + "Feature `{}` includes `{}` which is not defined as a feature", + feature, + feat + ) + } + } + } + // The value is a dependency. If features are namespaced, it is explicitly + // tagged as such (`crate:value`). If features are not namespaced, any value + // not recognized as a feature is pegged as a `Crate`. Here we handle the case + // where the dependency exists but is non-optional. It branches on namespaced + // just to provide the correct string for the crate dependency in the error. + (&Crate(ref dep), true, false) => if namespaced { + bail!( + "Feature `{}` includes `crate:{}` which is not an \ + optional dependency.\nConsider adding \ + `optional = true` to the dependency", + feature, + dep + ) + } else { + bail!( + "Feature `{}` depends on `{}` which is not an \ + optional dependency.\nConsider adding \ + `optional = true` to the dependency", + feature, + dep + ) + }, + // If namespaced, the value was tagged as a dependency; if not namespaced, + // this could be anything not defined as a feature. This handles the case + // where no such dependency is actually defined; again, the branch on + // namespaced here is just to provide the correct string in the error. + (&Crate(ref dep), false, _) => if namespaced { + bail!( + "Feature `{}` includes `crate:{}` which is not a known \ + dependency", + feature, + dep + ) + } else { + bail!( + "Feature `{}` includes `{}` which is neither a dependency nor \ + another feature", + feature, + dep + ) + }, + (&Crate(_), true, true) => {} + // If the value is a feature for one of the dependencies, bail out if no such + // dependency is actually defined in the manifest. + (&CrateFeature(ref dep, _), false, _) => bail!( "Feature `{}` requires a feature of `{}` which is not a \ dependency", feature, - dep_name - ), - (&Crate(ref dep), None) => bail!( - "Feature `{}` includes `{}` which is neither \ - a dependency nor another feature", - feature, dep ), - (&CrateFeature(_, _), Some(_)) | (&Feature(_), _) => {} + (&CrateFeature(_, _), true, _) => {} } values.push(val); } + + if !dependency_found { + // If we have not found the dependency of the same-named feature, we should + // bail here. + bail!( + "Feature `{}` includes the optional dependency of the \ + same name, but this is left implicit in the features \ + included by this feature.\nConsider adding \ + `crate:{}` to this feature's requirements.", + feature, + feature + ) + } + map.insert(feature.clone(), values); } Ok(map) @@ -212,30 +330,42 @@ pub enum FeatureValue { } impl FeatureValue { - fn build(feature: InternedString, is_feature: T) -> FeatureValue + fn build(feature: InternedString, is_feature: T, namespaced: bool) -> FeatureValue where T: Fn(&str) -> bool, { - match feature.find('/') { - Some(pos) => { + match (feature.find('/'), namespaced) { + (Some(pos), _) => { let (dep, dep_feat) = feature.split_at(pos); let dep_feat = &dep_feat[1..]; FeatureValue::CrateFeature(InternedString::new(dep), InternedString::new(dep_feat)) } - None if is_feature(&feature) => FeatureValue::Feature(feature), - None => FeatureValue::Crate(feature), + (None, true) if feature.starts_with("crate:") => { + FeatureValue::Crate(InternedString::new(&feature[6..])) + } + (None, true) => FeatureValue::Feature(feature), + (None, false) if is_feature(&feature) => FeatureValue::Feature(feature), + (None, false) => FeatureValue::Crate(feature), } } pub fn new(feature: InternedString, s: &Summary) -> FeatureValue { - Self::build(feature, |fs| s.features().contains_key(fs)) + Self::build( + feature, + |fs| s.features().contains_key(fs), + s.namespaced_features(), + ) } - pub fn to_string(&self) -> String { + pub fn to_string(&self, s: &Summary) -> String { use self::FeatureValue::*; match *self { Feature(ref f) => f.to_string(), - Crate(ref c) => c.to_string(), + Crate(ref c) => if s.namespaced_features() { + format!("crate:{}", &c) + } else { + c.to_string() + }, CrateFeature(ref c, ref f) => [c.as_ref(), f.as_ref()].join("/"), } } diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 33683c6b166..2e1f67142b8 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -214,13 +214,14 @@ fn transmit( return Ok(()); } - let string_features = pkg.summary() + let summary = pkg.summary(); + let string_features = summary .features() .iter() .map(|(feat, values)| { ( feat.clone(), - values.iter().map(|fv| fv.to_string()).collect(), + values.iter().map(|fv| fv.to_string(&summary)).collect(), ) }) .collect::>>(); From dc5d023482c726c7f9dc4c0873a8f984e74e8f7b Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 18 Apr 2018 12:12:43 +0200 Subject: [PATCH 6/8] Add tests for namespaced features --- tests/testsuite/features.rs | 259 ++++++++++++++++++++++++++++++++++++ 1 file changed, 259 insertions(+) diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index 42493b9b68a..f65644b3dc4 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -1714,3 +1714,262 @@ fn combining_features_and_package() { execs().with_status(0), ); } + +#[test] +fn namespaced_invalid_feature() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + namespaced-features = true + + [features] + bar = ["baz"] + "#, + ) + .file("src/main.rs", "") + .build(); + + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]` + +Caused by: + Feature `bar` includes `baz` which is not defined as a feature +", + ), + ); +} + +#[test] +fn namespaced_invalid_dependency() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + namespaced-features = true + + [features] + bar = ["crate:baz"] + "#, + ) + .file("src/main.rs", "") + .build(); + + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]` + +Caused by: + Feature `bar` includes `crate:baz` which is not a known dependency +", + ), + ); +} + +#[test] +fn namespaced_non_optional_dependency() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + namespaced-features = true + + [features] + bar = ["crate:baz"] + + [dependencies] + baz = "0.1" + "#, + ) + .file("src/main.rs", "") + .build(); + + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]` + +Caused by: + Feature `bar` includes `crate:baz` which is not an optional dependency. +Consider adding `optional = true` to the dependency +", + ), + ); +} + +#[test] +fn namespaced_implicit_feature() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + namespaced-features = true + + [features] + bar = ["baz"] + + [dependencies] + baz = { version = "0.1", optional = true } + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + assert_that(p.cargo("build"), execs().with_status(0)); +} + +#[test] +fn namespaced_shadowed_dep() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + namespaced-features = true + + [features] + baz = [] + + [dependencies] + baz = { version = "0.1", optional = true } + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]` + +Caused by: + Feature `baz` includes the optional dependency of the same name, but this is left implicit in the features included by this feature. +Consider adding `crate:baz` to this feature's requirements. +", + ), + ); +} + +#[test] +fn namespaced_shadowed_non_optional() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + namespaced-features = true + + [features] + baz = [] + + [dependencies] + baz = "0.1" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]` + +Caused by: + Feature `baz` includes the dependency of the same name, but this is left implicit in the features included by this feature. +Additionally, the dependency must be marked as optional to be included in the feature definition. +Consider adding `crate:baz` to this feature's requirements and marking the dependency as `optional = true` +", + ), + ); +} + +#[test] +fn namespaced_implicit_non_optional() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + namespaced-features = true + + [features] + bar = ["baz"] + + [dependencies] + baz = "0.1" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + assert_that( + p.cargo("build"), + execs().with_status(101).with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]` + +Caused by: + Feature `bar` includes `baz` which is not defined as a feature. +A non-optional dependency of the same name is defined; consider adding `optional = true` to its definition +", + ), + ); +} + +#[test] +fn namespaced_same_name() { + let p = project("foo") + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + namespaced-features = true + + [features] + baz = ["crate:baz"] + + [dependencies] + baz = { version = "0.1", optional = true } + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + assert_that(p.cargo("build"), execs().with_status(0)); +} From f5a4282e0bffad9048437d3d11288fc152e33022 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 23 Apr 2018 09:18:08 +0200 Subject: [PATCH 7/8] Add some documentation about unstable namespaced-features feature --- src/doc/src/reference/unstable.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 483e5e2537e..ca004ccce62 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -227,3 +227,30 @@ opt-level = 3 ``` Overrides can only be specified for dev and release profiles. + + +### Namespaced features +* Original issue: [#1286](https://github.com/rust-lang/cargo/issues/1286) + +Currently, it is not possible to have a feature and a dependency with the same +name in the manifest. If you set `namespaced-features` to `true`, the namespaces +for features and dependencies are separated. The effect of this is that, in the +feature requirements, dependencies have to be prefixed with `crate:`. Like this: + +```toml +[project] +namespaced-features = true + +[features] +bar = ["crate:baz", "foo"] +foo = [] + +[dependencies] +baz = { version = "0.1", optional = true } +``` + +To prevent unnecessary boilerplate from having to explicitly declare features +for each optional dependency, implicit features get created for any optional +dependencies where a feature of the same name is not defined. However, if +a feature of the same name as a dependency is defined, that feature must +include the dependency as a requirement, as `foo = ["crate:foo"]`. From 0b6f420670cef2bbfc9ea9705e46d2479d308a11 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Mon, 30 Apr 2018 10:42:52 +0200 Subject: [PATCH 8/8] Put namespaced features behind a feature gate --- src/cargo/core/features.rs | 3 +++ src/cargo/util/toml/mod.rs | 3 +++ tests/testsuite/features.rs | 38 +++++++++++++++++++++++++++++-------- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index b38c0950e9e..0ab8dbe5676 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -177,6 +177,9 @@ features! { // Overriding profiles for dependencies. [unstable] profile_overrides: bool, + + // Separating the namespaces for features and dependencies + [unstable] namespaced_features: bool, } } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 5ef05ad009b..f15d0e2fc56 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -839,6 +839,9 @@ impl TomlManifest { let exclude = project.exclude.clone().unwrap_or_default(); let include = project.include.clone().unwrap_or_default(); + if project.namespaced_features.is_some() { + features.require(Feature::namespaced_features())?; + } let summary = Summary::new( pkgid, diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index f65644b3dc4..d0412414129 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -1721,6 +1721,8 @@ fn namespaced_invalid_feature() { .file( "Cargo.toml", r#" + cargo-features = ["namespaced-features"] + [project] name = "foo" version = "0.0.1" @@ -1735,7 +1737,7 @@ fn namespaced_invalid_feature() { .build(); assert_that( - p.cargo("build"), + p.cargo("build").masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr( "\ [ERROR] failed to parse manifest at `[..]` @@ -1753,6 +1755,8 @@ fn namespaced_invalid_dependency() { .file( "Cargo.toml", r#" + cargo-features = ["namespaced-features"] + [project] name = "foo" version = "0.0.1" @@ -1767,7 +1771,7 @@ fn namespaced_invalid_dependency() { .build(); assert_that( - p.cargo("build"), + p.cargo("build").masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr( "\ [ERROR] failed to parse manifest at `[..]` @@ -1785,6 +1789,8 @@ fn namespaced_non_optional_dependency() { .file( "Cargo.toml", r#" + cargo-features = ["namespaced-features"] + [project] name = "foo" version = "0.0.1" @@ -1802,7 +1808,7 @@ fn namespaced_non_optional_dependency() { .build(); assert_that( - p.cargo("build"), + p.cargo("build").masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr( "\ [ERROR] failed to parse manifest at `[..]` @@ -1821,6 +1827,8 @@ fn namespaced_implicit_feature() { .file( "Cargo.toml", r#" + cargo-features = ["namespaced-features"] + [project] name = "foo" version = "0.0.1" @@ -1837,7 +1845,10 @@ fn namespaced_implicit_feature() { .file("src/main.rs", "fn main() {}") .build(); - assert_that(p.cargo("build"), execs().with_status(0)); + assert_that( + p.cargo("build").masquerade_as_nightly_cargo(), + execs().with_status(0), + ); } #[test] @@ -1846,6 +1857,8 @@ fn namespaced_shadowed_dep() { .file( "Cargo.toml", r#" + cargo-features = ["namespaced-features"] + [project] name = "foo" version = "0.0.1" @@ -1863,7 +1876,7 @@ fn namespaced_shadowed_dep() { .build(); assert_that( - p.cargo("build"), + p.cargo("build").masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr( "\ [ERROR] failed to parse manifest at `[..]` @@ -1882,6 +1895,8 @@ fn namespaced_shadowed_non_optional() { .file( "Cargo.toml", r#" + cargo-features = ["namespaced-features"] + [project] name = "foo" version = "0.0.1" @@ -1899,7 +1914,7 @@ fn namespaced_shadowed_non_optional() { .build(); assert_that( - p.cargo("build"), + p.cargo("build").masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr( "\ [ERROR] failed to parse manifest at `[..]` @@ -1919,6 +1934,8 @@ fn namespaced_implicit_non_optional() { .file( "Cargo.toml", r#" + cargo-features = ["namespaced-features"] + [project] name = "foo" version = "0.0.1" @@ -1936,7 +1953,7 @@ fn namespaced_implicit_non_optional() { .build(); assert_that( - p.cargo("build"), + p.cargo("build").masquerade_as_nightly_cargo(), execs().with_status(101).with_stderr( "\ [ERROR] failed to parse manifest at `[..]` @@ -1955,6 +1972,8 @@ fn namespaced_same_name() { .file( "Cargo.toml", r#" + cargo-features = ["namespaced-features"] + [project] name = "foo" version = "0.0.1" @@ -1971,5 +1990,8 @@ fn namespaced_same_name() { .file("src/main.rs", "fn main() {}") .build(); - assert_that(p.cargo("build"), execs().with_status(0)); + assert_that( + p.cargo("build").masquerade_as_nightly_cargo(), + execs().with_status(0), + ); }