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

Keep serialized protobuf binaries for testing #4570

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kzys
Copy link
Member

@kzys kzys commented Jan 20, 2024

These serialized binaries can be used to test changes like #4422.

@kzys
Copy link
Member Author

kzys commented Jan 20, 2024

Hmm, this may not work.

https://protobuf.dev/programming-guides/encoding/#implications

  • Do not assume the byte output of a serialized message is stable. This is especially true for messages with transitive bytes fields representing other serialized protocol buffer messages.
  • By default, repeated invocations of serialization methods on the same protocol buffer message instance may not produce the same byte output. That is, the default serialization is not deterministic.

@tonistiigi
Copy link
Member

Deterministic encoding is needed for LLB encoding(at least for same version of buildkit) as the format is content-addressable and uses checksums of encoded messages are used for deduplication.

@kzys
Copy link
Member Author

kzys commented Feb 1, 2024

Let me set Deterministic on MarshalOptions. This may be the best we could do here.

	// Deterministic controls whether the same message will always be
	// serialized to the same bytes within the same binary.
	//
	// Setting this option guarantees that repeated serialization of
	// the same message will return the same bytes, and that different
	// processes of the same binary (which may be executing on different
	// machines) will serialize equal messages to the same bytes.
	// It has no effect on the resulting size of the encoded message compared
	// to a non-deterministic marshal.
	//
	// Note that the deterministic serialization is NOT canonical across
	// languages. It is not guaranteed to remain stable over time. It is
	// unstable across different builds with schema changes due to unknown
	// fields. Users who need canonical serialization (e.g., persistent
	// storage in a canonical form, fingerprinting, etc.) must define
	// their own canonicalization specification and implement their own
	// serializer rather than relying on this API.
	//
	// If deterministic serialization is requested, map entries will be
	// sorted by keys in lexographical order. This is an implementation
	// detail and subject to change.
	Deterministic [bool](https://pkg.go.dev/builtin#bool)

@kzys kzys force-pushed the proto-test branch 4 times, most recently from cad2c8e to 4918e71 Compare February 1, 2024 03:46
@kzys
Copy link
Member Author

kzys commented Feb 1, 2024

MarshalOptions is only available in google.golang.org/protobuf, which can't handle existing gogo-generated protobuf messages.

gogo's equivalent is stable_marshaler_all, which we already have in LLB-related files such as solver/pb/ops.proto.

These serialized binaries can be used to test changes like moby#4422.

Signed-off-by: Kazuyoshi Kato <kato.kazuyoshi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants