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

x/build: use new name for release branches in golang.org/x repos starting with Go 1.16 #36882

Open
rittneje opened this issue Jan 29, 2020 · 14 comments
Assignees
Milestone

Comments

@rittneje
Copy link

@rittneje rittneje commented Jan 29, 2020

See #36842 for background. I asked this question on that thread, but didn't get an answer. If the release branches in x/crypto are not intended for public consumption, they simply should not exist. This is especially confusing because this repo (golang/go) does use release branches for their standard purpose. If clients are really just supposed to use latest master regardless of their actual Go version, that should be stated clearly in the README. Also, I don't understand why changes are getting merged to release branches instead of just master.

Note: We do not use go mod because we have found it to be incredibly cumbersome. Instead, we use git submodules. So not having a proper release branch to track is rather annoying.

@gopherbot gopherbot added this to the Unreleased milestone Jan 29, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 30, 2020

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Jan 30, 2020

Yes, we should document that users are supposed to track @latest (which currently is master, as we don't have tags, see #21324) just like any other module.

The branches need to exist so we have something to backport commits to when we need to vendor them in a previous Go release, without importing everything that happened on master. See #36851 for an example of the use case.

I don't understand how submodules make tracking master any harder than tracking a release branch.

@FiloSottile FiloSottile changed the title x/crypto: stop creating release branches x/all: document release branches Jan 30, 2020
@rittneje
Copy link
Author

@rittneje rittneje commented Jan 30, 2020

The branches need to exist so we have something to backport commits to when we need to vendor them in a previous Go release, without importing everything that happened on master.

I would suggest naming them something like "internal/release-branch.go1.13" to prevent consumers from thinking they are proper release branches.

I don't understand how submodules make tracking master any harder than tracking a release branch.

It's just because master moves ahead a lot, and afaik submodules cannot really "lock" to a specific commit when you do git submodule update --remote. We can of course get around this problem either by accepting constant changes (not so feasible for us), or by making our own branch that rarely moves.

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Jul 27, 2020

As a data point, k8s was also using the release branches of the subrepos, presumably misunderstanding them for what you are supposed to use with a certain Go version. kubernetes/kubernetes#93264 (comment)

@golang/osp-team, does any subrepo have a different policy, or are they all using the release branches for vendoring into the go tree, and not for public consumption?

@jbeisswenger-cetitec
Copy link

@jbeisswenger-cetitec jbeisswenger-cetitec commented Oct 6, 2020

The Go 1 compatibility promise's section about Sub-repositories says that

Code in sub-repositories of the main go tree, such as golang.org/x/net, may be developed under looser compatibility requirements. However, the sub-repositories will be tagged as appropriate to identify versions that are compatible with the Go 1 point releases.

I always assumed that these release branches in the golang.org/x/* repos were these compatibility tags.

Even if no common policy for the subrepos currently exists, the section should be updated if it is not current anymore.

Since this is also what is linked to on the Go website its not surprising that people expect these release branches to be compatible releases.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 6, 2020

@jbeisswenger-cetitec I think you're right: we have not been adding the tags that the compatibility document promises. But rather than update the doc, I think we should change the release process to add those tags. CC @dmitshur for release process update.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Oct 6, 2020

I agree with @jbeisswenger-cetitec that the text at https://golang.org/doc/go1compat#subrepos is either confusing or misleading, and doesn't seem to correspond to reality.

As a minor side-note, we've been moving in the direction of preferring to use the term "golang.org/x repositories" instead of "sub-repositories" as of a few years ago.

I think we should change the release process to add those tags.

@ianlancetaylor Do you have something specific in mind; if so, can you please elaborate? Or is your suggestion that we should do the work of investigating what a concrete strategy for creating tags should look like?

(I imagine that the right time to create a tag indicating "this version is the last one known to support Go 1.N" is when Go 1.N+2 is released, as that's when we stop testing golang.org/x repos with Go 1.N. But I'm not sure what a good tag name would be.)

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 6, 2020

My concrete suggestion is that when we cut a release 1.N, we add a a tag go1.N at the tip of each supported subrepo. That will indicate a location at that subrepo that supports release 1.N. We aren't promising to tag the last version of each subrepo that supports that release, just some version. (I would be fine with moving the tags forward if we know that a later version works with a particular release, but I don't think that is essential.)

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Oct 7, 2020

@ianlancetaylor Thanks to clarifying. I think that is a very reasonable strategy to consider.

Making the change to the release process that would cause those tags to get created (and never updated) should be fairly easy and inexpensive to do, but I think it's worth considering these questions before we proceed with implementing it:

  • Do we think that starting to create this tag will not have harmful side-effects? For example, people may misunderstand their purpose and begin to prefer using excessively old versions instead of newer ones.
  • Does the meaning of these tags need to be documented somewhere? If so, it may help to write that text first before making a definite decision to proceed here.
  • How much work will it be to keep this strategy up? The answer may depend on whether we do this once (as suggested), or if we end up needing to update tags to be less out-of-date.
  • How do we expect these "go1.N" tags to intersect with the semantic version releases (issues #28136 and #21324)? Hopefully they don't intersect poorly, but we should confirm that.

Once we create tags, especially ones we expect people to use and rely on, it's not something we can undo easily, so I think we need to be fairly confident the new strategy will be more helpful than harmful, and also sustainable to keep up, before we commit to doing it.

I plan to bring this issue up and discuss it with @cagedmantis and @toothrot tomorrow.

This issue had NeedsFix and Documentation labels applied back when @FiloSottile suggested documenting that users should use @latest in #36882 (comment). I'll change it to NeedsDecision.

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Oct 14, 2020

I don't think we should create these tags.

First, git tags shouldn't move but we can't recommend the same commit forever, both because in general we should not encourage using old versions of the modules, and because specifically when there is a security issue we need to steer people towards the updated version. In practice, implementing this would require branches.

Second, developers look at the golang.org/x modules for guidance on how to manage their own modules, and I don't think tagging per-Go version releases is something we want to bless as a best practice. The Go compatibility promise is there to keep developer updating rather than supporting them on older and older versions.

Finally, I don't see the utility: all our modules should support at latest all supported Go versions. Once a Go version is out of support, we don't support it. People's code is not going to break spontaneously, at worst they will try to update a module, and notice that they need to update Go as well. This is true of any module they depend on, so not something that should come as a surprise.

I think we should amend that sub-repositories paragraph, and do our best to communicate that golang.org/x modules are just any other modules, and discourage people from using the release branches which are just implementation details of how we do vendoring in the standard library.

(Ideally, we'd also rename them to something else than release-branch since they are not releases of the module.)

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Oct 20, 2020

Here's another example of release branch misuse: the Homebrew formula for Go builds golang.org/x/tools/cmd/godoc at release-branch.go1.15. It's very unlikely what they want is "cmd/godoc at the version of x/tools/go/analysis vendored in cmd/go in Go 1.15" but that's what they are getting.

https://github.com/Homebrew/homebrew-core/blob/5c03267153c911cb76df6a4d81af1213f7d26c4e/Formula/go.rb#L14

Users think these branches are what they should use with the associated Go version, or at least they think they are maintained like the main repo release branches. This is a common and reasonable misconception that I don't think we are going to fix with documentation alone. It's also a real concern for security fixes, as those are only committed to master.

I suggest starting with Go 1.16 we rename them to internal-vendor.go1.X.

This will not break any current users but hopefully will cause anyone using them to migrate to master as they should.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Oct 22, 2020

I'm in favor of renaming our release branches in golang.org/x repos starting with Go 1.16. I think that solution has good properties. It will require some changes in testing/release infrastructure and access controls, but they should be small and easy to do.

I suggest starting with Go 1.16 we rename them to internal-vendor.go1.X.

We've had the occasional need to make additional release branches in golang.org/x repos:

  • In #36851 (comment), we made release-branch.go1.14-std and release-branch.go1.14-cmd to account for a deviation in go.mod versions across modules.
  • In #41375 (comment), we made release-branch.go1.15-bundle to account for a deviation in bundle version and go.mod version.

(With #36852, #41409 being marked as release blockers, I expect those exact needs will not reoccur, but we may still run into a new reason to need additional internal branches.)

How about a minor adjustment to your suggestion for a more scaleable pattern:

internal-branch.go1.X-vendor
internal-branch.go1.X-{purpose}  # If another internal branch needs to be made for a new reason.

Then the access controls can be updated so that only release managers can submit CLs to branches matching the regexps ^refs/heads/internal-branch.+ (for Go 1.16 and newer) and ^refs/heads/release-branch.+ (for Go 1.15 and older).

(I initially considered internal.go1.X-vendor but figured a longer and more ugly name will only help deter the misuse of these internal branches for other reasons.)

@FiloSottile Does this minor adjustment to the naming pattern still have the properties you're looking for?

@toothrot, @cagedmantis, @ianlancetaylor Does the plan above sound good to you, so we can move this issue to NeedsFix state?

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Oct 24, 2020

I like internal-branch.go1.X-vendor even better!

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Oct 27, 2020

Discussed this in a release meeting, moving to NeedsFix and taking. If there are newly found concerns or suggestions, we can still adjust the plan before Go 1.16 RC 1, when release branches in golang.org/x repos will be cut.

@dmitshur dmitshur self-assigned this Oct 27, 2020
@dmitshur dmitshur added the NeedsFix label Oct 27, 2020
@dmitshur dmitshur changed the title x/all: document release branches x/build: use new name for release branches in golang.org/x repos starting with Go 1.16 Oct 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.