tokio-rs / prost

PROST! a Protocol Buffers implementation for the Rust Language
Apache License 2.0
3.65k stars 476 forks source link

feature: Deterministic Serialization #965

Open ParkMyCar opened 6 months ago

ParkMyCar commented 6 months ago

As far as I understand the proto3 specification does not require the serialized data to be stable or deterministic (docs, notes). I'm wondering if prost would be willing to guarantee the following properties do not change, within a semver compatible version, and if they do change to note it in the changelog:

I totally realize other libraries and other languages may not serialize to the exact same wire format at prost, and that's fine! The goal here is to make sure for a given proto message A, prost always serializes it to the same wire format. As far as I can tell prost already does this, but having the library formally guarantee this, within a semver compatible version, would be useful!

DavidBM commented 6 months ago

We are looking at the same in out internal project. We found that, for our use case, this change solves our issue.

diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs
index 908a76a..f9f88be 100644
--- a/prost-build/src/lib.rs
+++ b/prost-build/src/lib.rs
@@ -150,6 +150,7 @@ use crate::extern_paths::ExternPaths;
 use crate::ident::to_snake;
 use crate::message_graph::MessageGraph;
 use crate::path::PathMap;
+use itertools::Itertools;

 /// A service generator takes a service descriptor and generates Rust code.
 ///
@@ -925,7 +926,7 @@ impl Config {
             trace!("Writing include file: {:?}", target.join(include_file));
             let mut file = fs::File::create(target.join(include_file))?;
             self.write_includes(
-                modules.keys().collect(),
+                modules.keys().sorted().collect(),
                 &mut file,
                 0,
                 if target_is_env { None } else { Some(&target) },

More changes might be needed for other use cases.

LucioFranco commented 6 months ago

I think this would be an interesting idea, but before we claim this we should write a test suite that verifies this and keeps track of this. If you're willing to contribute this I see no reason why not.