proposal: cmd/go: add pin keyword to go.mod

[benbuzbee]

I would like to have a discussion, or receive guidance, on how to deal with packages which make breaking changes within a minor version which break dependencies via MVS.

Example of the problem

If one were to include the pkg github.com/cloudflare/cloudflared/token they would find that its module includes github.com/coreos/go-oidc v0.0.0-20171002155002-a93f71fdfe73

If they were also to include github.com/containerd/containerd, they would find its module includes k8s.io/[email protected] which includes github.com/coreos/[email protected]+incompatible

My understanding is MVS will select [email protected] to be used by all parties; however 2.1.0 has deleted its jose package which cloudflare/token depends on.

In other words, I do not know a way, without changing one of my dependencies, to make this program work:

package main

import (
	"fmt"

	"github.com/cloudflare/cloudflared/token"
	"github.com/containerd/containerd"
)

func main() {
	var e *token.Encrypter
	var c *containerd.Client
	fmt.Println(e, c)
}

github.com/cloudflare/cloudflared/token imports
        github.com/coreos/go-oidc/jose: cannot find module providing package github.com/coreos/go-oidc/jose

If you would like a contrived example see:
https://github.com/benbuzbee/module_example

Impact of this problem

I have seen this problem occur in my team at least once a month, and it has led to

1. creating insane hacks, like replacing versions in our go.mod just to make our dependencies compile
2. increasing development lead time by weeks while we work to submit pull requests to update our dependencies' dependencies just so we can continue a feature
3. giving up on trying to include the package, leading to suboptimal outcomes and frustration with go

I have not yet thought of a workaround except for going so far as to compile separate binaries linked by RPC.

If this problem is believed to be rare, or only caused by misuse of the module system, please let me know. I would love to share more examples and be educated in how I am incorrectly using the module system. I have seen this happen very often.

Discussion I would like to have here

First let me start by saying I am quite naïve in areas of compilers and language design, so please forgive me if that naivety is on display here and I welcome growth in this area. I do, however, consider myself a competent developer on a competent team. My team has encountered this problem many times and, as mentioned above, I have yet to come up with a satisfactory solution.

I would like to discuss the severity and frequency of this problem and the advice golang developers would give to work around it. I believe the problem to be severe as it has completely hampered my teams ability to write effective go on many occassions.

I would like to discuss if the belief that developers would correctly update semver was a bet that appears to not be paying off.

And I would like to discuss potential solutions to the problem. Either ones which exist but of which I am not aware, or which do not yet exist but could. I do not believe it is a valid answer that either "developers should more correctly update semver" or that "you can fork the code to separate the packages" or that "you should update your dependencies' dependencies so that they work".

An uneducated proposal to get the discussion started

Again noting that I am naïve in compilers and language design. I would suggest adding to go.mod a pin keyword

pin github.com/coreos/go-oidc v0.0.0-20171002155002-a93f71fdfe73

Would prevent MVS from updating this module version in the graph, and instead create a fork containing 2 versions of modules go-oidc.

I am aware this may pose certain challenges, such as requiring separate compilation units to prevent name collisions, and interop issues for exported APIs with differing type definitions, but I am not familiar enough to know how severe these challenges may be.

Thank you for listening

[ianlancetaylor]

The "pin" keyword that you suggest would mean, as I understand it, that a single program would contain two different packages that use the same import path. The go tooling can't support that. It's also rather confusing for anybody looking at the program: the same import path would denote different packages, and how would people know which one they need? It's pretty fundamental in the design that a single import path always denotes a specific package.

I haven't looked at the packages, but it sounds like github.com/coreos/go-oidc is making breaking changes without changing to a new major version. Why are they doing that? You say that this happens often: why is that?

[seankhliao]

I looked, go-oidc did bump a major version v1 -> v2, though at that point they hadn't yet adopted modules (they're currently on v3 and are using modules).
Looking at the cloudflared codebase, they use both v1 and v2, through the paths github.com/coreos/go-oidc and gopkg.in/coreos/go-oidc.v2, which is more or less equivalent to what would have been required (different module paths) of them if modules had been present at that point.

[earthboundkid]

1. creating insane hacks, like replacing versions in our go.mod just to make our dependencies compile

Why is that an "insane hack"? ISTM, that's exactly the reason go.mod supports version replacement. Your upstream is relying on broken code. The only reasonable things to do are to get the upstream to fix its dependencies and/or tell your app to ignore the broken dependency until upstream is fixed. "Pinning" just seems like a second, non-orthogonal way to do version replacement.

[zigo101]

The only reasonable things to do are to get the upstream to fix its dependencies and/or tell your app to ignore the broken dependency until upstream is fixed.

How about two different upstreams use two different versions of a module? The two different versions are not compatible (it is really a problem of that module) and each version only satisfies one upstream. In such cases, the two upstreams are both innocent.

[ericchiang]

Hey author of go-oidc.

Absolutely nothing should be depending on v1. It's been depreciate for years now and has not received any security attention since. Please do not depend on the go-oidc JOSE logic.

We would have updated the path when v2 landed, but go modules didn't exist then. That was corrected for v3.

[ericchiang]

For context, we nuked the v1 branch years ago (2017) and switched to go-jose to use a better reviewed JOSE implementation.

We created the v2 branch before Go modules existed, but kept the path as github.com/coreos/go-oidc since the pattern of updating the path with a version wasn't widely adopted. When we tried creating a go.mod for v2, it broke users who could no longer import as github.com/coreos/go-oidc coreos/go-oidc#230.

v3 uses the expected path github.com/coreos/go-oidc/v3/oidc, allowing users to include both v2 and v3.

If you're depending on v1, I think the solution is to upgrade to either v2, v3, or a better JOSE implementation like github.com/square/go-jose.

Either way, probably a better issue for https://github.com/coreos/go-oidc/issues than Go?

[benbuzbee]

Thank you everyone for your contributions! 

@ericchiang I appreciate your input for go-oidc. You are correct, ideally cloudflared would not depend on go-oidc v1, but I want want to avoid focusing on the specifics of the go-oidc example. I just wanted to share one example of how a go module author could be bitten by transitive dependencies, even though they themselves might depend on two perfectly recent modules.

Fundamentally I believe the go ecosystem could provide a better way to avoid these problems by providing more control over the dependency graph, specifically by allowing developers to tell go when a minor semver change is actually breaking, which is why I have opened the discussion here.

Let me share a better example below that removes the go-oidc questions.

Why replace isn't enough

@carlmjohnson good point, let me share another example to illustrate why replace alone is insufficient.

package main

import (
    "fmt"

    "https://github.com/containerd/containerd"
    "https://github.com/hashicorp/nomad/drivers/shared/executor"
)

func main() {
    var d *executor.ExecCommand
    var c *containerd.Client
    fmt.Println(d, c)
}

../go/pkg/mod/[github.com/hashicorp/[email protected]/drivers/shared/executor/executor_linux.go:792:15](https://github.com/hashicorp/[email protected]/drivers/shared/executor/executor_linux.go:792:15): undefined: configs.Device
../go/pkg/mod/[github.com/hashicorp/[email protected]/drivers/shared/executor/executor_universal_linux.go:124:36](https://github.com/hashicorp/[email protected]/drivers/shared/executor/executor_universal_linux.go:124:36): cannot use groups (type *configs.Cgroup) as type *configs.Resources in argument to freezer.Set

Here, containerd requires github.com/opencontainers/[email protected] and executor indirectly requires github.com/opencontainers/[email protected]. I actually am not sure why it indirectly requires this, my go mod graph skills failed me here)
(Note: Nomad has updated their dependencies but only very recently and I can still repro this with 1.2.5

In this case I depend on two post-release modules, but nomad's dependency chains pull in an RC version of runc that is broken by v1.0.2

I am able to work around this like so: replace github.com/opencontainers/runc v1.0.2 => github.com/opencontainers/runc v1.0.0-rc93

This solution works! However it will also downgrade everything to the least common denominator, the version before the breaking change.

1. I am depriving containerd of any bug fixes, including security, between v1.0.0-rc93  and v1.0.2
2. Though this works here, it would not have worked in many cases. For example, If containerd had used the APIs which changed in 1.0.2
3. I can no longer include any modules, or versions of modules, which truely depend on the newer runc

The source of this limitation seems mostly to do with the fact that, today, go can only upgrade all or none of the dependencies' runc (as ianlancetaylor correctly points out, they are all a single import path shared in the entire graph)

The pin I suggest differs in that it would target only the users of the breaking change: allow containerd to use version 1.0.2, but provide a facility to lock the Nomad executor package to 1.0.0-rc93. Create a version barrier at 1.0.0-rc93, after which go will create a fork in the dependency graph rather than attempt to upgrade.

Solving problems of two modules per import path

@ianlancetaylor indeed I think much of the limitation is that go will treat a package by its import path as equivalent and upgrade all nodes in the dependency graph which reference that package name.

Perhaps this problem could be mitigated by allowing modules to "alias" or "rename" packages.
For example, 

• alias github.com/coreos/go-oidc v0.0.0-20171002155002-a93f71fdfe73 => github.com/coreos/go-oidc-fork-for-cloudflared
  or
• rename github.com/opencontainers/runc v1.0.0-rc93 => github.com/opencontainers/runc-locked-for-nomad

The pre-process step could replace the import path of packages in modules with explicit dependencies on the left package with the package on the right. This is similar to how replace works, but it adds the ability to cheat MVS by separating the names in the dependency graph. (also cc @carlmjohnson, another way to explain the difference I am proposing)

This could be explicit as I have shown, or an implementation detail of pin.

What's interesting is that renaming like this also solves API surface/ABI compat problems, since type names would only match for these versions if the caller and callee both agreed on the version.

More examples

I compiled a few more examples by browsing my company chat. Some of these examples are v0.* or experimental packages or -rc, but they are causing real world problems consistently. If I can come up with this many in a few minutes, I imagine there must be many more.

1. Between go.opentelemetry.io/otel/exporters/trace/jaeger v0.18.0 and v0.19.0 jaeger changed its API https://github.com/open-telemetry/opentelemetry-go/pull/1673/files

otel was in pre-release so long that there are many packages depending on various v0.* versions

2. Google's protobuf moved its example package in https://github.com/protocolbuffers/protobuf/pull/7217/files

I do not know why people are importing things from the example package but apparently they are, as it broke a real application

3. Google's gRPC library removed a package - the package was marked experimental at the time but still caused grief
   using go mod tidy , there is error: google.golang.org/grpc/naming: module google.golang.org/grpc@latest found (v1.43.0), but does not contain package google.golang.org/grpc/naming grpc/grpc-go#5072

[ianlancetaylor]

I think the lengthy discussion in #44550 is relevant here, particularly #44550 (comment).

[earthboundkid]

Okay, I think I’ve gotten the problem now. It’s just the name “pinning” has nothing to do with what’s really going on here. It’s a name being used because it is used in other version managers, but what is really wanted is a way to pick versions to coexist (“Roman ride” is jargon I heard on a podcast) other than “semantic import versioning”. Basically today, you can use foo 1 and foo 2 at the same time if and only if foo 1 and 2 have different URLs (counting /v2 as part of the URL). This means library authors can decide what coexists and what doesn’t, but often application developers want to make that decision for themselves.

It’s a good idea, and probably something should be done in that direction, but I don’t like the name “pin” for it, and as ianlancetaylor pointed out, there are other discussions approaching the question already.

[bcmills]

Perhaps this problem could be mitigated by allowing modules to "alias" or "rename" packages.

See #26904. I'm still actively (but slowly) working on that problem space.

[seankhliao]

#26904 is allowing multiple names to reference a single module version. I think this is instead asking for a name that will fork a single module to use different versions.

If you're going so far as to rename a module and handle coordinating between multiple external modules to match the import name, then there's already a well understood mechanism for it: a hard fork.
This becomes a natural point of coordination, there's no subtleties in needing both a name + version match and works today.

[benbuzbee]

@ianlancetaylor thanks much for that link it was very informative.

Specifically @rsc's linked GopherConSG talk was extremely relevent, mostly beginning here where breaks like these are addressed.

I know a lot of thought has gone into this design and philosophy and I am extremely respectful of it. The talk makes me think in general this issue was explicitly not addressed owing to the philosophy point of "co-operation" in the ecosystem, which I appreciate.

Allow me to respectfully submit my opinion after consuming all of this as a data point for consideration:

As Russ points out here here programming is increasingly organized by piecing libraries together, and libraries of libraries. This leads to go.mod files of extreme sizes and complexity. Even with the healthiest culture of cooperation amongst members of the open-source community, this complexity will cause many breaks. Making them more impactful is the fact that many of these breaks will not be discovered by the authors of the related libraries, but by the author of the service that is just trying to piece them together in novel ways.

The cost of these breaks to the service author, and the cost to fix them, can be quite high. Take my example of runc and nomad - it takes quite a level of familiarity with go modules to diagnose the issue to begin with - someone just adding containerd to a program would be shocked to find that suddenly nomad doesn't build and the errors seem to have nothing to do with containerd. If they were familiar enough to use go tools to determine the root of the break, they would be distressed to realize that to fix it they have to update nomad's repo to jump 3 major docker versions (17 to 20) and fix dependencies with libraries they may or may not be familiar with and know how to test. And even if they did all of that, they would have to wait on the nomad team to accept the pull request and publish a tagged version. The nomad developers would be right to be very wary and slow to accept such a change - who knows the far reaching risk of upgrading docker this much.

@seankhliao your workaround of creating a repo fork and renaming the version to the new repo may indeed work, and it is good that we have some workaround, but this cost is high and I think creates huge fragmentation undermining the community that go seeks to create. Fixing the issue by forking the module does seem like the most straight forward and potentially correct fix, but I really wish there were a better answer than true repo fork.

Now that I have said that, hopefully it will be considered and feel free to close if this thread does not add more value. I appreciate all the information I have gotten from it, though I am sad the workarounds remain painful.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

I added this to active yesterday without reading the entire conversation (with a note to myself to come back and reread it carefully when I could devote more time to it, which I have just done). Upon a close reading, looks like it has resolved itself, so we should move it to retracted at the next proposal review.

Ultimately software is only as good as the cooperation of the overall ecosystem. The initial draft of Go modules did include functionality like pin to allow fine-grained control of "X means this in one place but this other thing in a different place", and it quickly became clear that the results would be very difficult for people to understand in practice.

Unsurprisingly, I agree with the conclusion that the best temporary fix is a fork of the module containing the stale requirement (in this case, cloudflared/token depending on a 5-year-old go-oidc) combined with a replace to redirect uses of that module with the fork. And the best permanent fix is to work with cloudflared/token's owner to get the dependency updated.

[rsc]

This proposal has been declined as retracted.
— rsc for the proposal review group