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

Add type aliases to honour go compatibility promise #3500

Closed
wants to merge 1 commit into from

Conversation

samsalisbury
Copy link

As noted in #3180 (comment) there are different libraries which depend on different versions of this library, within major version 1, that contain API differences. An example pair of these is go.etcd.io/etcd v3.4.7 and cloud.google.com/go v0.56.0 which require v1.23.1 and v1.28.0 of this library, respectively. Between these two versions, non-backwards compatible changes were made to the names of 3 types in 2 packages in this module. This breaks Go's required compatibility for modules sharing an import path (i.e. with the same major version) Other Go modules are hence unable to import these libraries because the Go tool assumes that all code within a major version API-compatible.

This change adds type aliases so that as of the next minor or patch release this module will again be API-compatible with all downstream modules that depend on major version 1 (once they get around to updating their version requirements accordingly).

The type aliases are in their own files so that when releasing major version 2 eventually, they can simply be deleted.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 6, 2020

CLA Check
The committers are authorized under a signed CLA.

@dfawley
Copy link
Member

dfawley commented Apr 6, 2020

The packages containing these types are clearly labeled as experimental, and backward compatibility is not promised in this case. Please see https://github.com/grpc/grpc-go/blob/master/Documentation/versioning.md for gRPC-Go's versioning policy.

Even though it's not an official part of that policy, we try to make sure there is a smooth migration path forward between all sequential MINOR releases, and that was provided in this case -- type aliases were in place for the 1.26 release, and marked as deprecated. If you need a release version that has both, you can use that one.

FYI we plan to move forward with the more invasive parts of this change in the next week or two; we are just waiting on one more major package that was unexpectedly using our experimental API to be updated.

@dfawley dfawley closed this Apr 6, 2020
@samsalisbury
Copy link
Author

Hi @dfawley, I read the versioning policy you referenced, this change does not seem to contravene that policy. Since this is a widely used Go package, and there is no concept of "experimental" in the go versioning scheme, do you think it could be worth following established conventions instead, so that this package behaves well in the Go ecosystem? I know this would not benefit the maintainers of this package directly, but it would greatly benefit the Go community if maintainers did follow these conventions.

In the meantime, we might be able to use 1.26, via a replace in our go.mod, but it's a shame that we would have to special-case this library, would not be able to upgrade to a later version, and would not be able to allow the standard go tools to do their usual job of selecting the minimum required version.

@dfawley
Copy link
Member

dfawley commented Apr 6, 2020

Go's conventions need to change to suit the needs of the community, not the other way around. We need a way to experiment with APIs and allow our users to preview them before we're able to commit to supporting them long-term. If there are more effective ways of doing these things without breaking the existing Go compatibility promise, I'm happy to hear them. Note that major version releases are not a viable option for us, as another goal we have is to eliminate the maintenance burden on others that new major release versions would create.

@samsalisbury
Copy link
Author

I agree that Go's conventions and tooling should make this a non-issue, but sadly that's not where we are at the moment. We're all in the same boat, at the whim of upstream maintainers to ensure their repositories are configured as the tooling expects, or else accepting that our dependencies will get very stale and/or have to be forked or dropped entirely at some point. It's really not good for the ecosystem, but I appreciate that the model also does not match every team's desires.

I would note that any release tagged with a prerelease field, e.g. "-beta" is not subject to the same compatibility promise, so a more conventional approach might be to add a v2 branch, where the go.mod uses the /v2 module path suffix, but only add tags that include a prerelease field until you're happy with the API. That way you could make as many breaking changes as necessary in v2. without breaking the workflow of dependent projects on v1. Those who want to be on the bleeding edge could then opt-in, and the major v1 API could be kept stable.

@dfawley
Copy link
Member

dfawley commented Apr 6, 2020

As I said above, frequent new major release versions is not an option for us, and we have no intention of maintaining a v2-prerelease and a v1 over any long-term timescale (similar to how long these APIs have been experimental and in flux).

What we have works well enough. If other projects DO use our experimental APIs, in violation of our recommendations, then we still provide a graceful upgrade path wherever possible. So as long as everyone is moving forward, things will keep working.

@samsalisbury
Copy link
Author

samsalisbury commented Apr 7, 2020

Out of curiosity what is the problem with incrementing the major version? Doing this enables other projects to knowingly opt-in to new behaviour, and should mean you never break other’s builds. There is no hard requirement for ongoing maintenance of older versions, you could just drop v1 maintenance now, and move on to v2-beta, even on the master branch, the tagged v1 releases would still exist and people could be told to upgrade via documentation. (This would break pre-go-modules projects that depend on this library though, but would be fine for those using modules. The Go team's blog post Go Modules: v2 and Beyond details the various methods of doing major version bumps and introducing unstable APIs via prerelease versions.)

The graceful upgrade path you mentioned works when you have a simple project that just depends on this library and few others. Larger projects with many dependencies may depend on multiple versions of this library transitively, as well as directly (as with some code I am working on). Go modules are designed specifically to help with this problem, allowing application developers to depend on multiple incompatible versions of transitive dependencies when necessary, as long as they follow the versioning rules. This scheme, when followed, makes the go ecosystem work with little friction, allowing disparate teams to use whichever version of a library suits them, without causing conflicts for consumers of those libraries. The design may not be perfect, but it’s a design that can work, if the community opts-in. Being a single, well-publicised rule that could in theory be applied to all go libraries, there is some chance (however remote) that the community reaches a consensus on it and can make forward progress, without needing to synchronise dependency upgrades across teams and different organisations, as the versioning scheme of this library implies (documentation notwithstanding).

Expecting over 20,000 packages that rely on this module, whose authors we do not directly communicate with, nor have any control over, to refrain from using working, public code, even if it is labelled as experimental, seems to not work in practice, at least within Google Cloud and Etcd, since they both rely different versions of this experimental code in their latest releases (see go.etcd.io/etcd v3.4.7 and cloud.google.com/go v0.56.0 which require v1.23.1 and v1.28.0 of this library, respectively).

The go toolchain provides a mechanism for communicating API instability to consumers programmatically, via prerelease/beta versions and zero-major-versions. This seems preferable to each org coming up with its own different rules, and hoping all consumers read all the docs and follow them.

I had no hand in the design of go modules, and I have to say they are causing problems for me and my team at the moment! It’s not just this library, we have seen some other examples of teams in different orgs coming up with their own unique schemes that simply do not work in practice with the current state of the go ecosystem. It creates a tremendous amount of work for us that seems easily avoidable, but perhaps we are missing something from our understanding that makes following the go versioning scheme much more difficult than it seems?

Thanks for your responses, and for bearing with me on this. I hope that one day we will be able to use the latest version of this library without having to fork it and/or fork a bunch of other things that depend on it.

@dfawley
Copy link
Member

dfawley commented Apr 7, 2020

Out of curiosity what is the problem with incrementing the major version?

As I mentioned earlier, frequent major version releases causes a maintenance burden for our users, and we would be on at least version 5 or 6 by now if every experimental API change we made was a new major release.

I'm fully aware of the problems caused by breaking backward compatibility. The APIs we change are very clearly documented as experimental. If any project uses these APIs in a non-experimental package, then they are violating our intended usage, and their usage should be treated as a bug in their project.

"Backward compatibility" is not strictly a behavior-level or AST-level promise. The documentation must be considered as well. Otherwise any behavior change -- including the addition of a new feature or even a bug fix -- would be a backward-compatibility breaking change. I'm sorry you don't agree, but this policy is not up for debate at this time.

@sdbaiguanghe
Copy link

etcd fixed this problem.

etcd-io/etcd#11564

@samsalisbury
Copy link
Author

Not sure what you mean @sdbaiguanghe? That PR you referenced allows etcd to compile but does not address this issue. This issue is about this module (google.golang.org/grpc) poisoning the Go modules ecosystem in general by advertising itself to the Go toolchain as having always had the same API, since v1.0.0, but in reality having changed it. This doesn't cause an issue for projects that just depend on GRPC directly, because they can choose whether or not to upgrade, and if they do upgrade, they can choose to change their calling code to work with the new API, not a huge fuss. However, for projects that depend on projects that depend on GRPC, the situation is dire. Unless those middle dependencies that depend on GRPC all happen to magically depend on an API-compatible v1.x.x versions of GRPC, then projects which depend on them may not be able to build at all using standard Go tools, because the Go tools cannot differentiate them. No magic would be necessary if this project instead incremented its major version to indicate breaking changes to the API, or used pre-release versions strings, or avoided breaking the API in the first place.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants