Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use BTreeMap for deterministic serialization #228

Merged
merged 2 commits into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ jobs:
- name: Check formatting
run: cargo fmt -- --check

clippy:
name: clippy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install rust
run: rustup update --no-self-update stable && rustup default stable
- name: Clippy check
run: cargo clippy --all-features -- -Dwarnings

test:
name: Test
runs-on: ubuntu-latest
Expand Down
3 changes: 3 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
disallowed-types = [
{ path = "std::collections::HashMap", reason = "HashMap's key order is unspecified and thus serializing such a type will result in a random output, use BTreeMap instead." }
SteveLauC marked this conversation as resolved.
Show resolved Hide resolved
]
25 changes: 6 additions & 19 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
use camino::Utf8PathBuf;
#[cfg(feature = "builder")]
use derive_builder::Builder;
use std::collections::{BTreeMap, HashMap};
use std::collections::BTreeMap;
use std::env;
use std::ffi::OsString;
use std::fmt;
Expand Down Expand Up @@ -109,7 +109,7 @@ pub use messages::{
ArtifactBuilder, ArtifactProfileBuilder, BuildFinishedBuilder, BuildScriptBuilder,
CompilerMessageBuilder,
};
use serde::{Deserialize, Serialize, Serializer};
use serde::{Deserialize, Serialize};

mod dependency;
pub mod diagnostic;
Expand All @@ -128,7 +128,7 @@ pub struct PackageId {
pub repr: String,
}

impl std::fmt::Display for PackageId {
impl fmt::Display for PackageId {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Display::fmt(&self.repr, f)
}
Expand All @@ -139,18 +139,6 @@ fn is_null(value: &serde_json::Value) -> bool {
matches!(value, serde_json::Value::Null)
}

/// Helper to ensure that hashmaps serialize in sorted order, to make
/// serialization deterministic.
fn sorted_map<S: Serializer, K: Serialize + Ord, V: Serialize>(
value: &HashMap<K, V>,
serializer: S,
) -> std::result::Result<S::Ok, S::Error> {
value
.iter()
.collect::<BTreeMap<_, _>>()
.serialize(serializer)
}

#[derive(Clone, Serialize, Deserialize, Debug)]
#[cfg_attr(feature = "builder", derive(Builder))]
#[non_exhaustive]
Expand Down Expand Up @@ -323,8 +311,7 @@ pub struct Package {
/// Targets provided by the crate (lib, bin, example, test, ...)
pub targets: Vec<Target>,
/// Features provided by the crate, mapped to the features required by that feature.
#[serde(serialize_with = "sorted_map")]
pub features: HashMap<String, Vec<String>>,
pub features: BTreeMap<String, Vec<String>>,
/// Path containing the `Cargo.toml`
pub manifest_path: Utf8PathBuf,
/// Categories as given in the `Cargo.toml`
Expand Down Expand Up @@ -432,7 +419,7 @@ impl Source {
}
}

impl std::fmt::Display for Source {
impl fmt::Display for Source {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Display::fmt(&self.repr, f)
}
Expand Down Expand Up @@ -615,7 +602,7 @@ pub struct MetadataCommand {
other_options: Vec<String>,
/// Arbitrary environment variables to set when running `cargo`. These will be merged into
/// the calling environment, overriding any which clash.
env: HashMap<OsString, OsString>,
env: BTreeMap<OsString, OsString>,
/// Show stderr
verbose: bool,
}
Expand Down