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

make Protobuf generation deterministic #2429

Closed
marten-seemann opened this issue Jul 14, 2023 · 7 comments
Closed

make Protobuf generation deterministic #2429

marten-seemann opened this issue Jul 14, 2023 · 7 comments
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful kind/maintenance Work required to avoid breaking changes or harm to project's status quo

Comments

@marten-seemann
Copy link
Contributor

go generate ./... run locally should produce the same Protobuf output as when run on CI.

@sukunrt
Copy link
Member

sukunrt commented Aug 8, 2023

Looking at the protobuf version doc
and the diffs here

-//     protoc        v3.21.12
+//     protoc        v4.23.3

It looks like your machine's protoc version changed between the runs. The latest protc version is v23.3(on homebrew) which generates files with v4.23.3.

Should we mention the protobuf compiler version to be used somewhere in the repo? I don't see it mentioned anywhere currently.

@marten-seemann
Copy link
Contributor Author

I was hoping we could do something like this:
https://github.com/quic-go/quic-go/blob/05db808f72b9183653cfce97d6c235108bb18ee6/mockgen.go#L5

By using go run github.com/golang/mock/mockgen the version defined in go.mod is used. No need to worry about versions, it's all automatic. And updates are handled by bumping the version in go.mod.

It seems like this would be more difficult to accomplish with the protobuf compiler, since protoc has to be installed separately.

@marten-seemann marten-seemann added kind/maintenance Work required to avoid breaking changes or harm to project's status quo exp/intermediate Prior experience is likely helpful effort/hours Estimated to take one or several hours labels Aug 9, 2023
@marten-seemann
Copy link
Contributor Author

Maybe the IPDX team has some ideas here. @galargh and @laurentsenta, have you encountered this before? Is there a good solution for this problem?

@galargh
Copy link
Contributor

galargh commented Aug 10, 2023

Some assume the risk and remove the version information from the generated files - golang/protobuf#1185

Off the top of my head, one could also do go generate in docker with pinned deps - e.g https://github.com/moby/moby/blob/master/hack/dockerfiles/generate-files.Dockerfile

@marten-seemann
Copy link
Contributor Author

I'd like to avoid using Docker for code generation. It would be nice if go generate ./... just worked.

Some assume the risk and remove the version information from the generated files - golang/protobuf#1185

This sounds reasonable. I assume we'd still commit the files with the version information, but remove it in CI before comparing?

@galargh
Copy link
Contributor

galargh commented Aug 18, 2023

This sounds reasonable. I assume we'd still commit the files with the version information, but remove it in CI before comparing?

That's an interesting idea! From what I've seen people generally strip the versions before commit but I think doing it for compare only is more clever. We could even do one check with stripped version and if it fails do another one without stripping so that the version diff shows up in the output.

I'm going to track the implementation here: ipdxco/unified-github-workflows#26

@marten-seemann
Copy link
Contributor Author

Closing since @galargh fixed this issue by adjusting the GHA workflows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours exp/intermediate Prior experience is likely helpful kind/maintenance Work required to avoid breaking changes or harm to project's status quo
Projects
None yet
Development

No branches or pull requests

3 participants