Skip to content

Commit

Permalink
Auto merge of #11409 - Muscraft:default-feat-workspace-dep, r=weihanglo
Browse files Browse the repository at this point in the history 
fix(toml): Add `default-features` to `TomlWorkspaceDependency`

In #11329 it was noted that `default-features` is ignored when used in a dependency that inherits from a workspace i.e.
```toml
[workspace]
members = []
[workspace.dependencies]
dep = "0.1"

[package]
name = "bar"
version = "0.2.0"
authors = []
[dependencies]
dep = { workspace = true, default-features = false }
```

This problem is caused by problems with deserializing a `TomlDependency` and not emitting an unused manifest key correctly. When discussed in a recent Cargo team meeting we felt the best course of action was to allow `default-features = false` when inheriting a dependency, but it does not change actually set `default-features`. It will be used to warn when there is a difference between the definition in `[workspace.dependencies]` and `[dependencies]` i.e.
```toml
[package]
name = "bar"
version = "0.2.0"
[dependencies]
dep = { workspace = true, default-features = false }

[workspace]
members = []
[workspace.dependencies]
dep = { version = "0.1", default-features = true }
```

This does not entirely resolve the problem with unused manifest keys. A follow up PR and issue will be created to address those concerns.

close #11329
  • Loading branch information
@bors
bors committed Jan 20, 2023
2 parents 975817b + 49d722c commit 7e2a3c2
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 0 deletions.
49 changes: 49 additions & 0 deletions src/cargo/util/toml/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@ impl<'de, P: Deserialize<'de> + Clone> de::Deserialize<'de> for TomlDependency<P
Ok(TomlDependency::Workspace(TomlWorkspaceDependency {
workspace: true,
features: details.features,
default_features: details.default_features,
default_features2: details.default_features2,
optional: details.optional,
}))
} else {
Expand Down Expand Up @@ -348,9 +350,13 @@ pub struct IntermediateDependency<P = String> {
}

#[derive(Deserialize, Serialize, Clone, Debug)]
#[serde(rename_all = "kebab-case")]
pub struct TomlWorkspaceDependency {
workspace: bool,
features: Option<Vec<String>>,
default_features: Option<bool>,
#[serde(rename = "default_features")]
default_features2: Option<bool>,
optional: Option<bool>,
}

Expand Down Expand Up @@ -2579,21 +2585,41 @@ impl TomlDependency {
cx: &mut Context<'_, '_>,
get_inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>,
) -> CargoResult<TomlDependency> {
fn default_features_msg(label: &str, ws_def_feat: Option<bool>, cx: &mut Context<'_, '_>) {
let ws_def_feat = match ws_def_feat {
Some(true) => "true",
Some(false) => "false",
None => "not specified",
};
cx.warnings.push(format!(
"`default-features` is ignored for {label}, since `default-features` was \
{ws_def_feat} for `workspace.dependencies.{label}`, \
this could become a hard error in the future"
))
}
match self {
TomlDependency::Detailed(d) => Ok(TomlDependency::Detailed(d)),
TomlDependency::Simple(s) => Ok(TomlDependency::Simple(s)),
TomlDependency::Workspace(TomlWorkspaceDependency {
workspace: true,
features,
optional,
default_features,
default_features2,
}) => {
if default_features.is_some() && default_features2.is_some() {
warn_on_deprecated("default-features", label, "dependency", cx.warnings);
}
let inheritable = get_inheritable()?;
inheritable.get_dependency(label).context(format!(
"error reading `dependencies.{}` from workspace root manifest's `workspace.dependencies.{}`",
label, label
)).map(|dep| {
match dep {
TomlDependency::Simple(s) => {
if let Some(false) = default_features.or(default_features2) {
default_features_msg(label, None, cx);
}
if optional.is_some() || features.is_some() {
Ok(TomlDependency::Detailed(DetailedTomlDependency {
version: Some(s),
Expand All @@ -2607,6 +2633,29 @@ impl TomlDependency {
},
TomlDependency::Detailed(d) => {
let mut dep = d.clone();
match (
default_features.or(default_features2),
d.default_features.or(d.default_features2)
) {
// member: default-features = true and
// workspace: default-features = false should turn on
// default-features
(Some(true), Some(false)) => {
dep.default_features = Some(true);
}
// member: default-features = false and
// workspace: default-features = true should ignore member
// default-features
(Some(false), Some(true)) => {
default_features_msg(label, Some(true), cx);
}
// member: default-features = false and
// workspace: dep = "1.0" should ignore member default-features
(Some(false), None) => {
default_features_msg(label, None, cx);
}
_ => {}
}
dep.add_features(features);
dep.update_optional(optional);
dep.resolve_path(label,inheritable.ws_root(), cx.root)?;
Expand Down
142 changes: 142 additions & 0 deletions tests/testsuite/inheritable_workspace_fields.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1356,3 +1356,145 @@ Caused by:
)
.run();
}

#[cargo_test]
fn warn_inherit_def_feat_true_member_def_feat_false() {
Package::new("dep", "0.1.0")
.feature("default", &["fancy_dep"])
.add_dep(Dependency::new("fancy_dep", "0.2").optional(true))
.file("src/lib.rs", "")
.publish();

Package::new("fancy_dep", "0.2.4").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "bar"
version = "0.2.0"
authors = []
[dependencies]
dep = { workspace = true, default-features = false }
[workspace]
members = []
[workspace.dependencies]
dep = { version = "0.1.0", default-features = true }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("check")
.with_stderr(
"\
[WARNING] [CWD]/Cargo.toml: `default-features` is ignored for dep, since `default-features` was \
true for `workspace.dependencies.dep`, this could become a hard error in the future
[UPDATING] `dummy-registry` index
[DOWNLOADING] crates ...
[DOWNLOADED] fancy_dep v0.2.4 ([..])
[DOWNLOADED] dep v0.1.0 ([..])
[CHECKING] fancy_dep v0.2.4
[CHECKING] dep v0.1.0
[CHECKING] bar v0.2.0 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run();
}

#[cargo_test]
fn warn_inherit_simple_member_def_feat_false() {
Package::new("dep", "0.1.0")
.feature("default", &["fancy_dep"])
.add_dep(Dependency::new("fancy_dep", "0.2").optional(true))
.file("src/lib.rs", "")
.publish();

Package::new("fancy_dep", "0.2.4").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "bar"
version = "0.2.0"
authors = []
[dependencies]
dep = { workspace = true, default-features = false }
[workspace]
members = []
[workspace.dependencies]
dep = "0.1.0"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("check")
.with_stderr(
"\
[WARNING] [CWD]/Cargo.toml: `default-features` is ignored for dep, since `default-features` was \
not specified for `workspace.dependencies.dep`, this could become a hard error in the future
[UPDATING] `dummy-registry` index
[DOWNLOADING] crates ...
[DOWNLOADED] fancy_dep v0.2.4 ([..])
[DOWNLOADED] dep v0.1.0 ([..])
[CHECKING] fancy_dep v0.2.4
[CHECKING] dep v0.1.0
[CHECKING] bar v0.2.0 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run();
}

#[cargo_test]
fn inherit_def_feat_false_member_def_feat_true() {
Package::new("dep", "0.1.0")
.feature("default", &["fancy_dep"])
.add_dep(Dependency::new("fancy_dep", "0.2").optional(true))
.file("src/lib.rs", "")
.publish();

Package::new("fancy_dep", "0.2.4").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "bar"
version = "0.2.0"
authors = []
[dependencies]
dep = { workspace = true, default-features = true }
[workspace]
members = []
[workspace.dependencies]
dep = { version = "0.1.0", default-features = false }
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("check")
.with_stderr(
"\
[UPDATING] `dummy-registry` index
[DOWNLOADING] crates ...
[DOWNLOADED] fancy_dep v0.2.4 ([..])
[DOWNLOADED] dep v0.1.0 ([..])
[CHECKING] fancy_dep v0.2.4
[CHECKING] dep v0.1.0
[CHECKING] bar v0.2.0 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run();
}

0 comments on commit 7e2a3c2

Please sign in to comment.