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

switch from deprecated gogo to google.golang.org/protobuf #38

Merged
merged 2 commits into from
Jan 11, 2023

Conversation

marten-seemann
Copy link
Contributor

Needed for libp2p/go-libp2p#1976.

While this is a straightforward change, this will break users, since the gogo proto.Message is different from the Google protobuf proto.Message. Sometimes, this will lead to compile time errors, sometimes to runtime panics. Once people update go-libp2p, they'll be forced onto the new version of this package that includes this PR (unless we go with (3), see below).

Unfortunately, this package is used by a number of repos: https://cs.github.com/?scopeName=All+repos&scope=&q=github.com%2Flibp2p%2Fgo-msgio%2Fprotoio+language%3AGo+NOT+repo%3Alibp2p%2Fgo-libp2p#. (Lots of them are forks that we don't have to care about, but there are some actual users among there as well).

We now have the following options:

  1. Do nothing. This will be released as a version v0.3.0 (current: v0.2.0), which according to semver signals breaking changes. Forcing users to migrate away from GoGo might be annoying for them, but justifiable, since GoGo is officially deprecated since October last year.
  2. Copy the protoio package to protoiogoogle (naming suggestions welcome). protoio would continue to support GoGo, whereas protoiogoogle would use the Google package. We could then deprecate protoio and remove it in a couple of months. Downside: protoio is a nice name, protoiogoogle isn't.
  3. Release a version v2.0.0 of this repo, and use that one in go-libp2p. This would allow people to keep using v0.2.0 and v2.0.0 in parallel. Downside: we'll have to update all of the import paths, and we have to release major versions (requiring updating the import paths again and again) every time we make a breaking API change.

@marten-seemann
Copy link
Contributor Author

Thinking about this a bit more, I think I'm in favor of (2). This way, we don't break people's build (even if it might be justified to break them in this case). I asked ChatGPT for some naming suggestions. Maybe pbio would be an acceptable name?

@MarcoPolo
Copy link

2 or 3 is fine by me. With a slight preference towards 3 since I feel like that makes more sense than forking a repo for an update.

@marten-seemann
Copy link
Contributor Author

With a slight preference towards 3 since I feel like that makes more sense than forking a repo for an update.

It's just the protoio package, not the entire package. Not really sure why protoio is in this package at all. Maybe we should move it out of go-msgio (into go-protoio?). That would also solve the problem.

@MarcoPolo
Copy link

Ah okay. Then I agree on 2

@marten-seemann marten-seemann force-pushed the google-protobuf branch 2 times, most recently from b267c29 to 15148fb Compare January 8, 2023 04:37
@marten-seemann
Copy link
Contributor Author

Introduced a new pbio package using Google's compiler, and deprecated the protoio package.

@marten-seemann
Copy link
Contributor Author

The plan is to create a v0.3.0 release once this PR is merged. I'll interpret approval for this PR as approval for this plan.

@marten-seemann marten-seemann merged commit a7103c0 into master Jan 11, 2023
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.

2 participants