Make manifest serialization deterministic#4562
Conversation
Fixes rust-lang#4326 `cargo package` (and so `cargo publish`) parses a crate’s `Cargo.toml`, makes some modifications, and re-serializes it. Because the `TomlManifest` struct uses `HashMap` with its default `RandomState` hasher, the maps’ iteration order changed on every run. As a result, when using `cargo vendor`, updating a dependency would generate a diff larger than necessary, with non-significant order-changes obscuring significant changes. This replaces some uses of `HashMap` with `BTreeMap`, whose iteration order is deterministic (based on `Ord`).
|
r? @matklad (rust_highfive has picked a reviewer for you, use r? to override) |
|
Perhaps we should use |
|
I personally don’t care much about preserving the original order, as long as it doesn’t change on every update. |
| "checksum crypto-hash 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "34903878eec1694faf53cae8473a088df333181de421d4d3d48061d6559fe602" | ||
| "checksum curl 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)" = "7034c534a1d7d22f7971d6088aa9d281d219ef724026c3428092500f41ae9c2c" | ||
| "checksum curl-sys 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)" = "4bee31aa3a079d5f3ff9579ea4dcfb1b1a17a40886f5f467436d383e78134b55" | ||
| "checksum custom_derive 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "ef8ae57c4978a2acd8b869ce6b9ca1dfe817bff704c220209fdef2c0b75a01b9" |
There was a problem hiding this comment.
Hm, where do these lockfile changes come from?
There was a problem hiding this comment.
They’re unrelated, and happen when I compile master unmodified. @alexcrichton said (in person) that it’s likely that someone forgot to include that change when they changed Cargo’s Cargo.toml in a previous PR, and that including that in this PR was fine. I can remove it if you prefer.
There was a problem hiding this comment.
Or split it into a separate commit in the same PR.
|
@bors r+ Thanks @SimonSapin ! |
|
📌 Commit f38c53f has been approved by |
Make manifest serialization deterministic Fixes #4326 `cargo package` (and so `cargo publish`) parses a crate’s `Cargo.toml`, makes some modifications, and re-serializes it. Because the `TomlManifest` struct uses `HashMap` with its default `RandomState` hasher, the maps’ iteration order changed on every run. As a result, when using `cargo vendor`, updating a dependency would generate a diff larger than necessary, with non-significant order-changes obscuring significant changes. This replaces some uses of `HashMap` with `BTreeMap`, whose iteration order is deterministic (based on `Ord`).
|
☀️ Test successful - status-appveyor, status-travis |
Fixes #4326
cargo package(and socargo publish) parses a crate’sCargo.toml, makes some modifications, and re-serializes it. Because theTomlManifeststruct usesHashMapwith its defaultRandomStatehasher, the maps’ iteration order changed on every run.As a result, when using
cargo vendor, updating a dependency would generate a diff larger than necessary, with non-significant order-changes obscuring significant changes.This replaces some uses of
HashMapwithBTreeMap, whose iteration order is deterministic (based onOrd).