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

cmd/dist: delete branchtag and its misleading use in findgoversion #42345

dmitshur opened this issue Nov 2, 2020 · 1 comment

cmd/dist: delete branchtag and its misleading use in findgoversion #42345

dmitshur opened this issue Nov 2, 2020 · 1 comment


Copy link

@dmitshur dmitshur commented Nov 2, 2020

The cmd/dist script contains a branchtag helper and the following code that uses it in findgoversion:

// If we're on a release branch, use the closest matching tag
// that is on the release branch (and not on the master branch).
if strings.HasPrefix(branch, "release-branch.") {
	tag, precise = branchtag(branch)

This comments suggests a certain behavior. As far as I can tell, this behavior happens almost never: release branches have always had a VERSION file checked in (going back as far as release-branch.go1 from 8 years ago), so execution inside findgoversion function won't reach that if statement. The only way I see for tag and/or precise to be anything other than their initial values are if one checks out a release branch and intentionally deletes the committed VERSION file, then tries to build Go from source.

(Perhaps this implementation worked better before the Go project transitioned from Mercurial to Git in commit f33fc0e, but I can't make sense of how it would have any effect since then.)

This issue is to investigate if my analysis is missing something. We may need to change this behavior in the future as part of work to make the development version string convey more accurate version information. This issue is would be relevant if we were to decide to change the release process to not keep the VERSION checked in on release branches (i.e., add it in one commit, remove in the next commit), but that seems unlikely.

If I'm missing something, please comment. Otherwise, I think we can delete the misleading nearly-dead code when the tree opens for Go 1.17 development.

CC @golang/release, @mvdan, @jayconrod, @dsymonds (in case you're still familiar this).

@dmitshur dmitshur added this to the Go1.17 milestone Nov 2, 2020
Copy link
Contributor Author

@dmitshur dmitshur commented May 10, 2021

I think we can delete this as a next step, which shouldn't change behavior in meaningful ways, but make the current code easier to maintain and change in the future.

Better to do it not during the freeze, so moving to next cycle.

@dmitshur dmitshur modified the milestones: Go1.17, Go1.18 May 10, 2021
@dmitshur dmitshur changed the title cmd/dist: branchtag and its use in findgoversion are misleading cmd/dist: delete branchtag and its misleading use in findgoversion May 10, 2021
@dmitshur dmitshur self-assigned this May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant