moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
8.03k stars 1.12k forks source link

Migrate away from gogo #3119

Open AndreasBergmeier6176 opened 1 year ago

AndreasBergmeier6176 commented 1 year ago

Since gogo/protobuf is no longer maintained it might make sense to start migrating away from it.

See https://github.com/gogo/googleapis/issues/19#issuecomment-1035690838

crazy-max commented 1 year ago

I took a look in the past to migrate away from gogo/protobuf but any alternative might break the contract unfortunately.

Links

Alternatives

vtprotobuf migrations in the wild

jedevc commented 1 year ago

Since this would likely be such a breaking change, I wonder if we should save this for if/when we hit buildkit 1.0? Without a breaking change, we'd need to support both the old protobuf and the protobuf for a few versions, and somehow determine between them, which sounds messy.

AkihiroSuda commented 1 year ago

I heard of another alternative: https://github.com/CrowdStrike/csproto They have a migration guide from gogo: https://github.com/CrowdStrike/csproto/blob/main/docs/migration_guide.md

@tonistiigi What should we do?

tonistiigi commented 1 year ago

Are the alternatives compatible on the wire? If not then this will be tricky and if we ever change we should reevaluate the whole grpc story. If compatibility is maintained then I don't have any desire to keep gogo. Compatibility on Go API level is not important.

AkihiroSuda commented 1 year ago

https://github.com/CrowdStrike/csproto/blob/main/docs/migration_guide.md says "csproto is designed to, as mush as possible, be a drop-in replacement"

crazy-max commented 1 year ago

https://github.com/CrowdStrike/csproto/blob/main/docs/migration_guide.md says "csproto is designed to, as mush as possible, be a drop-in replacement"

Unfortunately, if you are currently using the more advanced code generation enabled by protoc-gen-gogofast, protoc-gen-gogofaster, or protoc-gen-gogoslick then you will lose functionality as the new protoc-gen-go plug-in does not provide the same features.

https://github.com/moby/buildkit/blob/4410c828d67c0ee420a79ba41a64954a5cef18ed/cache/contenthash/generate.go#L3

https://github.com/moby/buildkit/blob/4410c828d67c0ee420a79ba41a64954a5cef18ed/solver/pb/generate.go#L3

https://github.com/moby/buildkit/search?q=gogoslick

marxarelli commented 1 year ago

I came across this issue while hacking locally to see if I could get buildkit working with xds (specifically I wanted to get it working with Istio's Envoy based service mesh). Long story but the part that's relevant here is that I got quite far but in the end I couldn't get the generated code to properly use the grpc.ServiceRegistrar interface over a *grpc.Server. This seems to be a limitation of the out of date (and FWICT unmaintained?) gogo protoc and its grpc plugin.