From 0139e91e98f40bf22a47eac600b8a7a606dba9a5 Mon Sep 17 00:00:00 2001 From: Daniel Lambert Date: Sat, 30 Sep 2023 14:36:00 -0500 Subject: [PATCH 1/7] replace dependency toml with toml_edit to preserve comments --- pkgs/crane-utils/Cargo.lock | 33 +- pkgs/crane-utils/Cargo.toml | 2 +- .../crane-resolve-workspace-inheritance.rs | 369 ++++++++++++++---- 3 files changed, 293 insertions(+), 111 deletions(-) diff --git a/pkgs/crane-utils/Cargo.lock b/pkgs/crane-utils/Cargo.lock index 9e49e745..7542da9b 100644 --- a/pkgs/crane-utils/Cargo.lock +++ b/pkgs/crane-utils/Cargo.lock @@ -16,7 +16,7 @@ dependencies = [ "pretty_assertions", "serde", "serde_json", - "toml", + "toml_edit", ] [[package]] @@ -124,15 +124,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_spanned" -version = "0.6.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96426c9936fd7a0124915f9185ea1d20aa9445cc9821142f0a73bc9207a2e186" -dependencies = [ - "serde", -] - [[package]] name = "syn" version = "2.0.28" @@ -144,37 +135,19 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "toml" -version = "0.7.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c17e963a819c331dcacd7ab957d80bc2b9a9c1e71c804826d2f283dd65306542" -dependencies = [ - "indexmap", - "serde", - "serde_spanned", - "toml_datetime", - "toml_edit", -] - [[package]] name = "toml_datetime" version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7cda73e2f1397b1262d6dfdcef8aafae14d1de7748d66822d3bfeeb6d03e5e4b" -dependencies = [ - "serde", -] [[package]] name = "toml_edit" -version = "0.19.14" +version = "0.20.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8123f27e969974a3dfba720fdb560be359f57b44302d280ba72e76a74480e8a" +checksum = "ca676d9ba1a322c1b64eb8045a5ec5c0cfb0c9d08e15e9ff622589ad5221c8fe" dependencies = [ "indexmap", - "serde", - "serde_spanned", "toml_datetime", "winnow", ] diff --git a/pkgs/crane-utils/Cargo.toml b/pkgs/crane-utils/Cargo.toml index 4279e369..59ac8901 100644 --- a/pkgs/crane-utils/Cargo.toml +++ b/pkgs/crane-utils/Cargo.toml @@ -8,7 +8,7 @@ publish = false anyhow = "1" serde_json = "1" serde = { version = "1", features = ["derive"] } -toml = { version = "0.7.3", features = ["preserve_order"] } +toml_edit = "0.20.1" [dev-dependencies] pretty_assertions = "1.3.0" diff --git a/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs b/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs index d5da5ed0..e79749a8 100644 --- a/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs +++ b/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs @@ -7,8 +7,9 @@ use std::{ mem, path::Path, process::{Command, Stdio}, + str::FromStr, }; -use toml::value::Table; +use toml_edit::Item; fn main() { let mut args = env::args(); @@ -64,39 +65,29 @@ fn resolve_and_print_cargo_toml(cargo_toml: &Path) -> anyhow::Result<()> { let mut cargo_toml = parse_toml(cargo_toml)?; merge(&mut cargo_toml, &parse_toml(&root_toml.join("Cargo.toml"))?); - toml::to_string(&cargo_toml) - .context("failed to serialize updated Cargo.toml") - .and_then(|string| { - stdout() - .write_all(string.as_bytes()) - .context("failed to print updated Cargo.toml") - .map(drop) - }) + stdout() + .write_all(cargo_toml.to_string().as_bytes()) + .context("failed to print updated Cargo.toml") } -fn parse_toml(path: &Path) -> anyhow::Result { +fn parse_toml(path: &Path) -> anyhow::Result { let mut buf = String::new(); File::open(path) .and_then(|mut file| file.read_to_string(&mut buf)) .with_context(|| format!("cannot read {}", path.display()))?; - toml::from_str(&buf).with_context(|| format!("cannot parse {}", path.display())) + toml_edit::Document::from_str(&buf).with_context(|| format!("cannot parse {}", path.display())) } -fn merge(cargo_toml: &mut toml::Value, root: &toml::Value) { - let (t, rt) = match (cargo_toml, root) { - (toml::Value::Table(t), toml::Value::Table(rt)) => (t, rt), - - // Bail if cargo_toml or workspace root are malformed - _ => return, - }; - - let w = if let Some(toml::Value::Table(w)) = rt.get("workspace") { - w - } else { - // no "workspace" entry, nothing to merge - return; - }; +/// Merge the workspace `root` toml into the specified crate's `cargo_toml` +fn merge(cargo_toml: &mut toml_edit::Document, root: &toml_edit::Document) { + let w: &dyn toml_edit::TableLike = + if let Some(w) = root.get("workspace").and_then(try_as_table_like) { + w + } else { + // no "workspace" entry, nothing to merge + return; + }; // https://doc.rust-lang.org/cargo/reference/workspaces.html#workspaces for (key, ws_key) in [ @@ -105,90 +96,300 @@ fn merge(cargo_toml: &mut toml::Value, root: &toml::Value) { ("dev-dependencies", "dependencies"), ("build-dependencies", "dependencies"), ] { - if let (Some(toml::Value::Table(p)), Some(toml::Value::Table(wp))) = - (t.get_mut(key), w.get(ws_key)) - { - merge_tables(p, wp); + if let Some((cargo_toml, root)) = cargo_toml.get_mut(key).zip(w.get(ws_key)) { + try_merge_cargo_tables(cargo_toml, root); }; - if let Some(toml::Value::Table(targets)) = t.get_mut("target") { - for (_, tp) in targets { - if let (Some(toml::Value::Table(p)), Some(toml::Value::Table(wp))) = - (tp.get_mut(key), w.get(ws_key)) - { - merge_tables(p, wp); - }; + if let Some(targets) = cargo_toml.get_mut("target").and_then(try_as_table_like_mut) { + for (_, tp) in targets.iter_mut() { + if let Some((cargo_toml, root)) = tp.get_mut(key).zip(w.get(ws_key)) { + try_merge_cargo_tables(cargo_toml, root); + } } } } + + // cleanup stray spaces left by merging InlineTable values into Tables + clear_value_formatting(cargo_toml.as_item_mut()); +} + +/// Return a [`toml_edit::TableLike`] representation of the [`Item`] (if any) +fn try_as_table_like(item: &Item) -> Option<&dyn toml_edit::TableLike> { + match item { + Item::Table(w) => Some(w), + Item::Value(toml_edit::Value::InlineTable(w)) => Some(w), + _ => None, + } } -fn merge_tables(cargo_toml: &mut Table, root: &Table) { +/// Return a mutable [`toml_edit::TableLike`] representation of the [`Item`] (if any) +fn try_as_table_like_mut(item: &mut Item) -> Option<&mut dyn toml_edit::TableLike> { + match item { + Item::Table(w) => Some(w), + Item::Value(toml_edit::Value::InlineTable(w)) => Some(w), + _ => None, + } +} + +/// Merge the specified `cargo_toml` and workspace `root` if both are tables +fn try_merge_cargo_tables(cargo_toml: &mut Item, root: &Item) { + match (cargo_toml, root) { + ( + Item::Table(p), // + Item::Table(wp), + ) => { + merge_cargo_tables(p, wp); + } + ( + Item::Value(toml_edit::Value::InlineTable(p)), // + Item::Table(wp), + ) => { + merge_cargo_tables(p, wp); + } + ( + Item::Table(p), // + Item::Value(toml_edit::Value::InlineTable(wp)), + ) => { + merge_cargo_tables(p, wp); + } + ( + Item::Value(toml_edit::Value::InlineTable(p)), + Item::Value(toml_edit::Value::InlineTable(wp)), + ) => { + merge_cargo_tables(p, wp); + } + _ => {} + } +} +/// Merge the specified `cargo_toml` and workspace `root` tables +fn merge_cargo_tables(cargo_toml: &mut T, root: &U) +where + T: toml_edit::TableLike, + U: toml_edit::TableLike, +{ cargo_toml.iter_mut().for_each(|(k, v)| { // Bail if: // - cargo_toml isn't a table (otherwise `workspace = true` can't show up // - the workspace root doesn't have this key - let (t, root_val) = match (&mut *v, root.get(k)) { - (toml::Value::Table(t), Some(root_val)) => (t, root_val), - _ => return, - }; - - if let Some(toml::Value::Boolean(true)) = t.get("workspace") { - t.remove("workspace"); - let orig_val = mem::replace(v, root_val.clone()); - merge_into(v, orig_val); + let (t, (root_key, root_val)) = + match try_as_table_like_mut(&mut *v).zip(root.get_key_value(&k)) { + Some((t, root_key_val)) => (t, root_key_val), + _ => return, + }; + + // copy any missing formatting to the current key + merge_key_formatting(k, root_key); + + if let Some(Item::Value(toml_edit::Value::Boolean(bool_value))) = t.get("workspace") { + if *bool_value.value() { + t.remove("workspace"); + let orig_val = mem::replace(v, root_val.clone()); + merge_items(v, orig_val); + } } }); } -fn merge_into(dest: &mut toml::Value, additional: toml::Value) { +/// Recursively merge the `additional` item into the specified `dest` +fn merge_items(dest: &mut Item, additional: Item) { + use toml_edit::Value; + match additional { - toml::Value::String(_) - | toml::Value::Integer(_) - | toml::Value::Float(_) - | toml::Value::Boolean(_) - | toml::Value::Datetime(_) => { - // Override dest completely for raw values - *dest = additional; - } + Item::Value(additional) => match additional { + Value::String(_) + | Value::Integer(_) + | Value::Float(_) + | Value::Boolean(_) + | Value::Datetime(_) => { + // Override dest completely for raw values + *dest = Item::Value(additional); + } - toml::Value::Array(additional) => { - if let toml::Value::Array(dest) = dest { + Value::Array(additional) => { + if let Item::Value(Value::Array(dest)) = dest { + dest.extend(additional); + } else { + // Override dest completely if types don't match + *dest = Item::Value(Value::Array(additional)); + } + } + + Value::InlineTable(additional) => { + merge_tables(dest, additional); + } + }, + Item::Table(additional) => { + merge_tables(dest, additional); + } + Item::None => {} + Item::ArrayOfTables(additional) => { + if let Item::ArrayOfTables(dest) = dest { dest.extend(additional); } else { - // Override dest completely if types don't match - *dest = toml::Value::Array(additional); + *dest = Item::ArrayOfTables(additional); } } + } +} - toml::Value::Table(additional) => { - if let toml::Value::Table(dest) = dest { - additional - .into_iter() - .for_each(|(k, v)| match dest.get_mut(&k) { - Some(existing) => merge_into(existing, v), - None => { - dest.insert(k, v); - } - }); - } else { +use table_like_ext::merge_tables; +mod table_like_ext { + //! Helper functions to merge values in any combination of the two [`TableLike`] items + //! found in [`toml_edit`] + + use toml_edit::{Item, TableLike}; + + /// Recursively merge the `additional` table into `dest` (overwriting if `dest` is not a table) + pub(super) fn merge_tables(dest: &mut Item, additional: T) + where + T: TableLikeExt, + { + match dest { + Item::Table(dest) => merge_table_like(dest, additional), + Item::Value(toml_edit::Value::InlineTable(dest)) => merge_table_like(dest, additional), + _ => { // Override dest completely if types don't match, but also // skip empty tables (i.e. if we had `key = { workspace = true }` if !additional.is_empty() { - *dest = toml::Value::Table(additional); + *dest = additional.into_item(); } } } } + + /// Recursively merge two tables + fn merge_table_like(dest: &mut T, additional: U) + where + T: TableLike, + U: TableLikeExt, + { + additional + .into_iter() + .map(U::map_iter_item) + .for_each(|(k, v)| match dest.get_mut(&k) { + Some(existing) => super::merge_items(existing, v), + None => { + dest.insert(&k, v); + } + }); + } + + /// Generalized form of the item yielded by [`IntoIterator`] for the two [`TableLike`] types + /// in [`toml_edit`] + type CommonIterItem = (toml_edit::InternalString, Item); + + /// Extension trait to iterate [`Item`]s from a [`TableLike`] item + pub(super) trait TableLikeExt: TableLike + IntoIterator { + /// Convert the iterator item to a common type + fn map_iter_item(item: Self::Item) -> CommonIterItem; + + /// Convert the table into an [`Item`] + fn into_item(self) -> Item; + } + + impl TableLikeExt for toml_edit::Table { + fn map_iter_item(item: Self::Item) -> CommonIterItem { + item + } + + fn into_item(self) -> Item { + Item::Table(self) + } + } + + impl TableLikeExt for toml_edit::InlineTable { + fn map_iter_item(item: Self::Item) -> CommonIterItem { + let (k, v) = item; + (k, Item::Value(v)) + } + + fn into_item(self) -> Item { + Item::Value(toml_edit::Value::InlineTable(self)) + } + } +} + +use fmt::{clear_value_formatting, merge_key_formatting}; +mod fmt { + use toml_edit::{Item, Value}; + + /// Replicate the formatting of keys, in case one comes from a [`toml_edit::Table`] and the other a + /// [`toml_edit::InlineTable`] + pub(super) fn merge_key_formatting( + mut dest: toml_edit::KeyMut<'_>, + additional: &toml_edit::Key, + ) { + let dest_decor = dest.decor_mut(); + let additional_decor = additional.decor(); + + let is_empty = |s: Option<&toml_edit::RawString>| { + s.and_then(toml_edit::RawString::as_str) + .map_or(true, str::is_empty) + }; + + if is_empty(dest_decor.prefix()) { + if let Some(prefix) = additional_decor.prefix() { + dest_decor.set_prefix(prefix.clone()); + } + } + if is_empty(dest_decor.suffix()) { + if let Some(suffix) = additional_decor.suffix() { + dest_decor.set_suffix(suffix.clone()); + } + } + } + + /// Remove formatting from all [`toml_edit::Value`]s recursively + pub(super) fn clear_value_formatting(item: &mut Item) { + match item { + Item::Value(value) => { + clear_value_formatting_value(value); + } + Item::Table(table) => table + .iter_mut() + .map(|(_k, v)| v) + .for_each(clear_value_formatting), + Item::None => {} + Item::ArrayOfTables(array) => array.iter_mut().for_each(|table| { + table + .iter_mut() + .map(|(_k, v)| v) + .for_each(clear_value_formatting) + }), + } + } + + fn clear_value_formatting_value(value: &mut Value) { + let decor = match value { + Value::String(value) => value.decor_mut(), + Value::Integer(value) => value.decor_mut(), + Value::Float(value) => value.decor_mut(), + Value::Boolean(value) => value.decor_mut(), + Value::Datetime(value) => value.decor_mut(), + Value::Array(value) => { + value.iter_mut().for_each(clear_value_formatting_value); + value.decor_mut() + } + Value::InlineTable(value) => { + value + .iter_mut() + .map(|(_k, v)| v) + .for_each(clear_value_formatting_value); + value.decor_mut() + } + }; + decor.clear(); + } } #[cfg(test)] mod tests { use pretty_assertions::assert_eq; + use std::str::FromStr; #[test] fn smoke() { - let mut cargo_toml = toml::from_str( + let mut cargo_toml = toml_edit::Document::from_str( r#" [package] authors.workspace = true @@ -209,6 +410,7 @@ mod tests { version.workspace = true [dependencies] + # the `foo` dependency is most imporant, so it goes first foo.workspace = true bar.workspace = true baz.workspace = true @@ -240,11 +442,15 @@ mod tests { grault = { version = "grault-vers" } garply = "garply-vers" waldo = "waldo-vers" + + [features] + # this feature is a demonstration that comments are preserved + my_feature = [] "#, ) .unwrap(); - let root_toml = toml::from_str( + let root_toml = toml_edit::Document::from_str( r#" [workspace.package] authors = ["first author", "second author"] @@ -265,6 +471,7 @@ mod tests { version = "some version" [workspace.dependencies] + # workspace comments are not copied foo = { version = "foo-vers" } bar = { version = "bar-vers", default-features = false } baz = { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } @@ -278,11 +485,10 @@ mod tests { ) .unwrap(); - let expected_toml = toml::from_str::( - r#" + let expected_toml_str = r#" [package] authors = ["first author", "second author"] - categories = ["first category", "second category" ] + categories = ["first category", "second category"] description = "some description" documentation = "some doc url" edition = "2021" @@ -299,6 +505,7 @@ mod tests { version = "some version" [dependencies] + # the `foo` dependency is most imporant, so it goes first foo = { version = "foo-vers" } bar = { version = "bar-vers", default-features = false } baz = { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } @@ -330,12 +537,14 @@ mod tests { grault = { version = "grault-vers" } garply = "garply-vers" waldo = "waldo-vers" - "#, - ) - .unwrap(); + + [features] + # this feature is a demonstration that comments are preserved + my_feature = [] + "#; super::merge(&mut cargo_toml, &root_toml); - assert_eq!(expected_toml, cargo_toml); + assert_eq!(expected_toml_str, cargo_toml.to_string()); } } From 40b6a3fe0f1285b3bfa4ae28a1dae6f4b50f0006 Mon Sep 17 00:00:00 2001 From: Daniel Lambert Date: Sat, 30 Sep 2023 16:36:57 -0500 Subject: [PATCH 2/7] update changelog for PR #407 --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22c89ff7..c42817a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ([#376](https://github.com/ipetkov/crane/pull/376)) * Replaced various internal usages of `runCommandLocal` with `runCommand` for more optimal behavior when downloading cached artifacts +* Fixed a bug when a git dependency crate relies on reading non-TOML metadata + from Cargo.toml at build time. + ([#407](https://github.com/ipetkov/crane/pull/407)) ## [0.13.1] - 2023-08-22 From b62e884891bb993e36a1176990a9e08e16ae8772 Mon Sep 17 00:00:00 2001 From: Daniel Lambert Date: Sun, 1 Oct 2023 09:16:00 -0500 Subject: [PATCH 3/7] fixup CHANGELOG addition for #407 --- CHANGELOG.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c42817a9..cea1347e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Unreleased +### Changed +* Fixed a bug when a git dependency crate relies on reading non-TOML metadata + from Cargo.toml at build time. + ([#407](https://github.com/ipetkov/crane/pull/407)) + ## [0.14.0] - 2023-09-21 ### Added @@ -31,9 +36,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ([#376](https://github.com/ipetkov/crane/pull/376)) * Replaced various internal usages of `runCommandLocal` with `runCommand` for more optimal behavior when downloading cached artifacts -* Fixed a bug when a git dependency crate relies on reading non-TOML metadata - from Cargo.toml at build time. - ([#407](https://github.com/ipetkov/crane/pull/407)) ## [0.13.1] - 2023-08-22 From 3efef9b4ea5246e86eb0c376528666186dcf5318 Mon Sep 17 00:00:00 2001 From: Daniel Lambert Date: Sun, 1 Oct 2023 09:25:54 -0500 Subject: [PATCH 4/7] replace exhaustive table match with dyn, add missing comments --- .../crane-resolve-workspace-inheritance.rs | 36 +++++-------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs b/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs index e79749a8..aeda5405 100644 --- a/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs +++ b/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs @@ -133,39 +133,18 @@ fn try_as_table_like_mut(item: &mut Item) -> Option<&mut dyn toml_edit::TableLik /// Merge the specified `cargo_toml` and workspace `root` if both are tables fn try_merge_cargo_tables(cargo_toml: &mut Item, root: &Item) { - match (cargo_toml, root) { - ( - Item::Table(p), // - Item::Table(wp), - ) => { - merge_cargo_tables(p, wp); - } - ( - Item::Value(toml_edit::Value::InlineTable(p)), // - Item::Table(wp), - ) => { - merge_cargo_tables(p, wp); - } - ( - Item::Table(p), // - Item::Value(toml_edit::Value::InlineTable(wp)), - ) => { - merge_cargo_tables(p, wp); - } - ( - Item::Value(toml_edit::Value::InlineTable(p)), - Item::Value(toml_edit::Value::InlineTable(wp)), - ) => { - merge_cargo_tables(p, wp); - } - _ => {} + let cargo_toml = try_as_table_like_mut(cargo_toml); + let root = try_as_table_like(root); + + if let Some((cargo_toml, root)) = cargo_toml.zip(root) { + merge_cargo_tables(cargo_toml, root); } } /// Merge the specified `cargo_toml` and workspace `root` tables fn merge_cargo_tables(cargo_toml: &mut T, root: &U) where - T: toml_edit::TableLike, - U: toml_edit::TableLike, + T: toml_edit::TableLike + ?Sized, + U: toml_edit::TableLike + ?Sized, { cargo_toml.iter_mut().for_each(|(k, v)| { // Bail if: @@ -226,6 +205,7 @@ fn merge_items(dest: &mut Item, additional: Item) { if let Item::ArrayOfTables(dest) = dest { dest.extend(additional); } else { + // Override dest completely if types don't match *dest = Item::ArrayOfTables(additional); } } From be233e6645aff2a4a5c0a341c760a67020adca97 Mon Sep 17 00:00:00 2001 From: Daniel Lambert Date: Sun, 1 Oct 2023 09:39:06 -0500 Subject: [PATCH 5/7] clarify TOML source comment --- pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs b/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs index aeda5405..52193fe3 100644 --- a/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs +++ b/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs @@ -451,7 +451,7 @@ mod tests { version = "some version" [workspace.dependencies] - # workspace comments are not copied + # top-level workspace comments are not copied - only the values are merged foo = { version = "foo-vers" } bar = { version = "bar-vers", default-features = false } baz = { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } From 242a4e47f893964e5c14b63c720dfb335bfae490 Mon Sep 17 00:00:00 2001 From: Daniel Lambert Date: Sun, 1 Oct 2023 09:46:55 -0500 Subject: [PATCH 6/7] remove key/value formatting merge and update test string accordingly --- .../crane-resolve-workspace-inheritance.rs | 154 +++++------------- 1 file changed, 38 insertions(+), 116 deletions(-) diff --git a/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs b/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs index 52193fe3..147d7e11 100644 --- a/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs +++ b/pkgs/crane-utils/src/bin/crane-resolve-workspace-inheritance.rs @@ -108,9 +108,6 @@ fn merge(cargo_toml: &mut toml_edit::Document, root: &toml_edit::Document) { } } } - - // cleanup stray spaces left by merging InlineTable values into Tables - clear_value_formatting(cargo_toml.as_item_mut()); } /// Return a [`toml_edit::TableLike`] representation of the [`Item`] (if any) @@ -150,14 +147,10 @@ where // Bail if: // - cargo_toml isn't a table (otherwise `workspace = true` can't show up // - the workspace root doesn't have this key - let (t, (root_key, root_val)) = - match try_as_table_like_mut(&mut *v).zip(root.get_key_value(&k)) { - Some((t, root_key_val)) => (t, root_key_val), - _ => return, - }; - - // copy any missing formatting to the current key - merge_key_formatting(k, root_key); + let (t, root_val) = match try_as_table_like_mut(&mut *v).zip(root.get(&k)) { + Some((t, root_val)) => (t, root_val), + _ => return, + }; if let Some(Item::Value(toml_edit::Value::Boolean(bool_value))) = t.get("workspace") { if *bool_value.value() { @@ -289,79 +282,6 @@ mod table_like_ext { } } -use fmt::{clear_value_formatting, merge_key_formatting}; -mod fmt { - use toml_edit::{Item, Value}; - - /// Replicate the formatting of keys, in case one comes from a [`toml_edit::Table`] and the other a - /// [`toml_edit::InlineTable`] - pub(super) fn merge_key_formatting( - mut dest: toml_edit::KeyMut<'_>, - additional: &toml_edit::Key, - ) { - let dest_decor = dest.decor_mut(); - let additional_decor = additional.decor(); - - let is_empty = |s: Option<&toml_edit::RawString>| { - s.and_then(toml_edit::RawString::as_str) - .map_or(true, str::is_empty) - }; - - if is_empty(dest_decor.prefix()) { - if let Some(prefix) = additional_decor.prefix() { - dest_decor.set_prefix(prefix.clone()); - } - } - if is_empty(dest_decor.suffix()) { - if let Some(suffix) = additional_decor.suffix() { - dest_decor.set_suffix(suffix.clone()); - } - } - } - - /// Remove formatting from all [`toml_edit::Value`]s recursively - pub(super) fn clear_value_formatting(item: &mut Item) { - match item { - Item::Value(value) => { - clear_value_formatting_value(value); - } - Item::Table(table) => table - .iter_mut() - .map(|(_k, v)| v) - .for_each(clear_value_formatting), - Item::None => {} - Item::ArrayOfTables(array) => array.iter_mut().for_each(|table| { - table - .iter_mut() - .map(|(_k, v)| v) - .for_each(clear_value_formatting) - }), - } - } - - fn clear_value_formatting_value(value: &mut Value) { - let decor = match value { - Value::String(value) => value.decor_mut(), - Value::Integer(value) => value.decor_mut(), - Value::Float(value) => value.decor_mut(), - Value::Boolean(value) => value.decor_mut(), - Value::Datetime(value) => value.decor_mut(), - Value::Array(value) => { - value.iter_mut().for_each(clear_value_formatting_value); - value.decor_mut() - } - Value::InlineTable(value) => { - value - .iter_mut() - .map(|(_k, v)| v) - .for_each(clear_value_formatting_value); - value.decor_mut() - } - }; - decor.clear(); - } -} - #[cfg(test)] mod tests { use pretty_assertions::assert_eq; @@ -465,55 +385,57 @@ mod tests { ) .unwrap(); + // NOTE: The nonstandard spacing is due to reusing decorations from original keys/values + // in cargo_toml let expected_toml_str = r#" [package] - authors = ["first author", "second author"] - categories = ["first category", "second category"] - description = "some description" - documentation = "some doc url" - edition = "2021" - exclude = ["first exclusion", "second exclusion"] - homepage = "some home page" - include = ["first inclusion", "second inclusion"] - keyword = ["first keyword", "second keyword"] - license = "some license" - license-file = "some license-file" - publish = true - readme = "some readme" - repository = "some repository" - rust-version = "some rust-version" - version = "some version" + authors= ["first author", "second author"] + categories= ["first category", "second category" ] + description= "some description" + documentation= "some doc url" + edition= "2021" + exclude= ["first exclusion", "second exclusion"] + homepage= "some home page" + include= ["first inclusion", "second inclusion"] + keyword= ["first keyword", "second keyword"] + license= "some license" + license-file= "some license-file" + publish= true + readme= "some readme" + repository= "some repository" + rust-version= "some rust-version" + version= "some version" [dependencies] # the `foo` dependency is most imporant, so it goes first - foo = { version = "foo-vers" } - bar = { version = "bar-vers", default-features = false } - baz = { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } - qux = { version = "qux-vers", features = ["qux-feat", "qux-additional"] } - corge = { version = "corge-vers-override", features = ["qux-feat"] } + foo= { version = "foo-vers" } + bar= { version = "bar-vers", default-features = false } + baz= { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } + qux = { version = "qux-vers", features = ["qux-feat","qux-additional"] } + corge = { version = "corge-vers-override" , features = ["qux-feat"] } grault = { version = "grault-vers" } garply = "garply-vers" waldo = "waldo-vers" [target.'cfg(unix)'.dependencies] - unix = { version = "unix-vers", features = ["some"] } + unix = { version = "unix-vers" , features = ["some"] } [dev-dependencies] - foo = { version = "foo-vers" } - bar = { version = "bar-vers", default-features = false } - baz = { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } - qux = { version = "qux-vers", features = ["qux-feat", "qux-additional"] } - corge = { version = "corge-vers-override", features = ["qux-feat"] } + foo= { version = "foo-vers" } + bar= { version = "bar-vers", default-features = false } + baz= { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } + qux = { version = "qux-vers", features = ["qux-feat","qux-additional"] } + corge = { version = "corge-vers-override" , features = ["qux-feat"] } grault = { version = "grault-vers" } garply = "garply-vers" waldo = "waldo-vers" [build-dependencies] - foo = { version = "foo-vers" } - bar = { version = "bar-vers", default-features = false } - baz = { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } - qux = { version = "qux-vers", features = ["qux-feat", "qux-additional"] } - corge = { version = "corge-vers-override", features = ["qux-feat"] } + foo= { version = "foo-vers" } + bar= { version = "bar-vers", default-features = false } + baz= { version = "baz-vers", features = ["baz-feat", "baz-feat2"] } + qux = { version = "qux-vers", features = ["qux-feat","qux-additional"] } + corge = { version = "corge-vers-override" , features = ["qux-feat"] } grault = { version = "grault-vers" } garply = "garply-vers" waldo = "waldo-vers" From 8f0f13e191be24cdda76dbcbeb82e39de4f82c42 Mon Sep 17 00:00:00 2001 From: Ivan Petkov Date: Sun, 1 Oct 2023 13:42:53 -0700 Subject: [PATCH 7/7] Slightly reword CHANGELOG --- CHANGELOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f21e71..4a76c70e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Unreleased -### Changed -* Fixed a bug when a git dependency crate relies on reading non-TOML metadata - from Cargo.toml at build time. - ([#407](https://github.com/ipetkov/crane/pull/407)) +### Fixed +* Fixed handling of Cargo workspace inheritance for git-dependencies where said + crate relies on reading non-TOML metadata (i.e. comments) from its Cargo.toml + at build time. ([#407](https://github.com/ipetkov/crane/pull/407)) ## [0.14.1] - 2023-09-23