Skip to content

Commit

Permalink
fix(toml)!: Remove support for inheriting badges
Browse files Browse the repository at this point in the history
We allowed `[badges]` to inherit from `[workspace.package.badges]`

This was a bug:
- This was not specified in the RFC
- We did not document this
- Even if someone were to try to guess to use this, it is inconsistent
  with how inheritance works because this should inherit from
  `workspace.badges` instead of `workspace.package.badges`

While keeping in mind that `[badges]` is effectively deprecated.

In that context, I think its safe to break support for this without a
transition period.

Fixes #13643
  • Loading branch information
epage committed Apr 22, 2024
1 parent e31c27d commit bdd4bda
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 50 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ cargo-platform = { path = "crates/cargo-platform", version = "0.1.5" }
cargo-test-macro = { version = "0.2.0", path = "crates/cargo-test-macro" }
cargo-test-support = { version = "0.2.0", path = "crates/cargo-test-support" }
cargo-util = { version = "0.2.9", path = "crates/cargo-util" }
cargo-util-schemas = { version = "0.3.0", path = "crates/cargo-util-schemas" }
cargo-util-schemas = { version = "0.4.0", path = "crates/cargo-util-schemas" }
cargo_metadata = "0.18.1"
clap = "4.5.4"
color-print = "0.3.5"
Expand Down
2 changes: 1 addition & 1 deletion crates/cargo-util-schemas/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-util-schemas"
version = "0.3.1"
version = "0.4.0"
rust-version = "1.77" # MSRV:1
edition.workspace = true
license.workspace = true
Expand Down
8 changes: 1 addition & 7 deletions crates/cargo-util-schemas/src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub struct TomlManifest {
pub replace: Option<BTreeMap<String, TomlDependency>>,
pub patch: Option<BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>,
pub workspace: Option<TomlWorkspace>,
pub badges: Option<InheritableBtreeMap>,
pub badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
pub lints: Option<InheritableLints>,

/// Report unused keys (see also nested `_unused_keys`)
Expand Down Expand Up @@ -106,12 +106,6 @@ impl TomlManifest {
self.features.as_ref()
}

pub fn resolved_badges(
&self,
) -> Result<Option<&BTreeMap<String, BTreeMap<String, String>>>, UnresolvedError> {
self.badges.as_ref().map(|l| l.resolved()).transpose()
}

pub fn resolved_lints(&self) -> Result<Option<&TomlLints>, UnresolvedError> {
self.lints.as_ref().map(|l| l.resolved()).transpose()
}
Expand Down
14 changes: 2 additions & 12 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,12 +451,7 @@ fn resolve_toml(

resolved_toml.lints = original_toml.lints.clone();

let resolved_badges = original_toml
.badges
.clone()
.map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges()))
.transpose()?;
resolved_toml.badges = resolved_badges.map(manifest::InheritableField::Value);
resolved_toml.badges = original_toml.badges.clone();
} else {
for field in original_toml.requires_package() {
bail!("this virtual manifest specifies a `{field}` section, which is not allowed");
Expand Down Expand Up @@ -799,7 +794,6 @@ impl InheritableFields {
package_field_getter! {
// Please keep this list lexicographically ordered.
("authors", authors -> Vec<String>),
("badges", badges -> BTreeMap<String, BTreeMap<String, String>>),
("categories", categories -> Vec<String>),
("description", description -> String),
("documentation", documentation -> String),
Expand Down Expand Up @@ -1340,11 +1334,7 @@ fn to_real_manifest(
.expect("previously resolved")
.cloned()
.unwrap_or_default(),
badges: resolved_toml
.resolved_badges()
.expect("previously resolved")
.cloned()
.unwrap_or_default(),
badges: resolved_toml.badges.clone().unwrap_or_default(),
links: resolved_package.links.clone(),
rust_version: rust_version.clone(),
};
Expand Down
30 changes: 2 additions & 28 deletions tests/testsuite/inheritable_workspace_fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ fn permit_additional_workspace_fields() {
exclude = ["foo.txt"]
include = ["bar.txt", "**/*.rs", "Cargo.toml", "LICENSE", "README.md"]
[workspace.package.badges]
gitlab = { repository = "https://gitlab.com/rust-lang/rust", branch = "master" }
[workspace.dependencies]
dep = "0.1"
"#,
Expand Down Expand Up @@ -117,8 +114,6 @@ fn inherit_own_workspace_fields() {
.file(
"Cargo.toml",
r#"
badges.workspace = true
[package]
name = "foo"
version.workspace = true
Expand Down Expand Up @@ -153,8 +148,6 @@ fn inherit_own_workspace_fields() {
rust-version = "1.60"
exclude = ["foo.txt"]
include = ["bar.txt", "**/*.rs", "Cargo.toml"]
[workspace.package.badges]
gitlab = { repository = "https://gitlab.com/rust-lang/rust", branch = "master" }
"#,
)
.file("src/main.rs", "fn main() {}")
Expand Down Expand Up @@ -186,9 +179,7 @@ You may press ctrl-c to skip waiting; the crate should be available shortly.
r#"
{
"authors": ["Rustaceans"],
"badges": {
"gitlab": { "branch": "master", "repository": "https://gitlab.com/rust-lang/rust" }
},
"badges": {},
"categories": ["development-tools"],
"deps": [],
"description": "This is a crate",
Expand Down Expand Up @@ -240,10 +231,6 @@ keywords = ["cli"]
categories = ["development-tools"]
license = "MIT"
repository = "https://github.com/example/example"
[badges.gitlab]
branch = "master"
repository = "https://gitlab.com/rust-lang/rust"
"#,
cargo::core::manifest::MANIFEST_PREAMBLE
),
Expand Down Expand Up @@ -665,15 +652,12 @@ fn inherit_workspace_fields() {
rust-version = "1.60"
exclude = ["foo.txt"]
include = ["bar.txt", "**/*.rs", "Cargo.toml", "LICENSE", "README.md"]
[workspace.package.badges]
gitlab = { repository = "https://gitlab.com/rust-lang/rust", branch = "master" }
"#,
)
.file("src/main.rs", "fn main() {}")
.file(
"bar/Cargo.toml",
r#"
badges.workspace = true
[package]
name = "bar"
workspace = ".."
Expand Down Expand Up @@ -731,9 +715,7 @@ You may press ctrl-c to skip waiting; the crate should be available shortly.
r#"
{
"authors": ["Rustaceans"],
"badges": {
"gitlab": { "branch": "master", "repository": "https://gitlab.com/rust-lang/rust" }
},
"badges": {},
"categories": ["development-tools"],
"deps": [],
"description": "This is a crate",
Expand Down Expand Up @@ -791,10 +773,6 @@ categories = ["development-tools"]
license = "MIT"
license-file = "LICENSE"
repository = "https://github.com/example/example"
[badges.gitlab]
branch = "master"
repository = "https://gitlab.com/rust-lang/rust"
"#,
cargo::core::manifest::MANIFEST_PREAMBLE
),
Expand Down Expand Up @@ -1715,8 +1693,6 @@ fn warn_inherit_unused_manifest_key_package() {
.file(
"Cargo.toml",
r#"
badges = { workspace = true, xyz = "abc"}
[workspace]
members = []
[workspace.package]
Expand All @@ -1734,8 +1710,6 @@ fn warn_inherit_unused_manifest_key_package() {
rust-version = "1.60"
exclude = ["foo.txt"]
include = ["bar.txt", "**/*.rs", "Cargo.toml"]
[workspace.package.badges]
gitlab = { repository = "https://gitlab.com/rust-lang/rust", branch = "master" }
[package]
name = "bar"
Expand Down

0 comments on commit bdd4bda

Please sign in to comment.