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/exp/cmd/gorelease: accept any release version when inferring base version of untagged repo #40267

Closed
Tracked by #46371
qmuntal opened this issue Jul 17, 2020 · 7 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@qmuntal
Copy link
Contributor

qmuntal commented Jul 17, 2020

gorelease should always fallback to none when inferring the base version in repositories without available versions.

The current implementation only fallbacks to none when releaseVersion is x.0.0 | 0.0.1 | 0.1.0 and fails one other situations. This behavior is not a semver requirement and despite being a good practice it may be too strict for modules which are required to start the version with other minor or patch numbers for whatever business, technical or legacy reason.

@gopherbot gopherbot added this to the Unreleased milestone Jul 17, 2020
@qmuntal qmuntal changed the title x/exp/cmd/gorelease: accept any release version when inferring unexisting base version x/exp/cmd/gorelease: accept any release version when inferring base version of untagged repo Jul 17, 2020
@jayconrod
Copy link
Contributor

It seems like it's better to require an explicit -base=none in this situation (the current behavior). If the first version of a module is, for example, v1.5.2, that implies there should be a v1.5.1. If gorelease can't find any previous version, then something is probably wrong, and an error should be reported.

Could you say more about why you wouldn't start with vX.0.0, v0.1.0, or something like that? "Business, technical, or legacy reason" is not really clear.

@jayconrod jayconrod added modules Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jul 17, 2020
@qmuntal
Copy link
Contributor Author

qmuntal commented Jul 18, 2020

Could you say more about why you wouldn't start with vX.0.0, v0.1.0, or something like that? "Business, technical, or legacy reason" is not really clear.

I´m adding experimental support for gorelease in an internal CI/CD pipeline that is being used by multiple teams in my organization. To use this pipeline a repo has to follow the Semantic Version 2.0.0 spec and be in compliance with the go modules requirements. Notice that non of the previous requirements force a module to start with x.0.0 | 0.0.1 | 0.1.0, and as it is not mandatory -despite being a highly recommended good practice- the pipeline allows to define a custom initial version in case the repository has no tags.

I´m afraid I don´t have a good answer for why someone would use this parameter, I´m just a man-in-the-middle, but I guess people don´t like to start a project with a low minor version for the fear of not getting adoption due to the inmaturity connotations associated to low 0.x.0 versions or to denotate some kind of continuity when creating a new spun-off project.

I´ve done a quick manual github search looking for initial version patterns in popular libraries and most of them starts with 0.1.0 or 0.0.1 but there are also some notable exceptions:

It seems like it's better to require an explicit -base=none in this situation (the current behavior)

It is easy for my to add some lines of code in the pipeline to check if the repo doesn´t have any tag and add an explicit -base=none, but I wonder if this restriction really makes sense and compensates the complexity it adds to the code and to its usage.

As a data point, I had to dive into the code to understand why it was failing when using v0.2.0 as first tag but working with v0.1.0, I couldn't find it in the error message nor in the documentation.

@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Oct 14, 2020
@jayconrod jayconrod modified the milestones: Unreleased, gorelease Oct 14, 2020
@jeanbza
Copy link
Member

jeanbza commented May 14, 2021

Sorry for the slow response to this.

My understanding of the bug is this:

deklerk at deklerk1 in ~/workspace/no-base on master
$ git tag
$ # there are no tags (no versions)

deklerk at deklerk1 in ~/workspace/no-base on master
$ ~/workspace/exp/cmd/gorelease/gorelease -version v1.2.3
gorelease: could not find base version: no versions found lower than v1.2.3
$ # this should have succeeded: any version is valid, since there are no prior versions

$ ~/workspace/exp/cmd/gorelease/gorelease -version v1.0.0
Inferred base version: none
v1.0.0 is a valid semantic version for this release.
$ # this succeeded, because for vx.0.0, v0.0.1, and v0.1.0, gorelease chooses "none" as base

$ ~/workspace/exp/cmd/gorelease/gorelease
Inferred base version: none
Suggested version: v0.1.0
$ # this succeeded, because when release version is not specified gorelease chooses "none" as base

As a data point, I had to dive into the code to understand why it was failing when using v0.2.0 as first tag but working with v0.1.0, I couldn't find it in the error message nor in the documentation.

Could you elaborate on this, @qmuntal ? Could you provide the error message you're seeing, and maybe describe what's confusing about it so that we can analyse whether some error messages can be made more explicit? (is it the "could not find base version" message above?)

@jeanbza
Copy link
Member

jeanbza commented May 14, 2021

After a quick chat with Jay, we're going to:

  1. Allow explicitly setting -base=none.
  2. Improve the error message.
  3. Continue the behavior of erroring if user sets -version to a non-"starting" version (vX.0.0, v0.0.1, v0.1.0), since it's weird and we want users to explicitly set -base=none as a way of acknowledging that they're doing a weird thing.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/320229 mentions this issue: cmd/gorelease: support for non-starting first version

@qmuntal
Copy link
Contributor Author

qmuntal commented May 17, 2021

As a data point, I had to dive into the code to understand why it was failing when using v0.2.0 as first tag but working with v0.1.0, I couldn't find it in the error message nor in the documentation.

Could you elaborate on this, @qmuntal ? Could you provide the error message you're seeing, and maybe describe what's confusing about it so that we can analyse whether some error messages can be made more explicit? (is it the "could not find base version" message above?)

@jadekler What I was seeing is the "could not find base version" error message. The change done in CL320229 will improve the situation, thanks for that.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/349997 mentions this issue: cmd/go: 'go get' no longer builds or installs packages

gopherbot pushed a commit that referenced this issue Sep 28, 2021
As part of #40267, 'go install' is now fully responsible for building
and installing executables. 'go get' will only be used to change
versions in go.mod. The -d flag no longer has any effect; its behavior
is the default.

When 'go get' is invoked inside a module on a main package outside of
the main module, it no longer prints any warning. In 1.16-1.17, we
suggested using -d in this situation, but we want
'go get example.com/cmd' to be able to upgrade a tool dependency
without needing -d to suppress the warning.

For #43684

Change-Id: I9daf29c123a5a0e382aa326d62721cb26fc26c19
Reviewed-on: https://go-review.googlesource.com/c/go/+/349997
Trust: Jay Conrod <[email protected]>
Run-TryBot: Jay Conrod <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
@rsc rsc unassigned jeanbza Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants