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

Bot upgrades golang packages to Invalid pseudo-version #4310

Closed
Clivern opened this issue Aug 17, 2019 · 20 comments · Fixed by #4386
Closed

Bot upgrades golang packages to Invalid pseudo-version #4310

Clivern opened this issue Aug 17, 2019 · 20 comments · Fixed by #4386
Assignees
Labels
manager:gomod Go Modules priority-2-high Bugs impacting wide number of users or very important features type:bug Bug fix of existing functionality

Comments

@Clivern
Copy link

Clivern commented Aug 17, 2019

What Renovate type are you using?
Renovate GitHub App

Describe the bug
Renovate is picking the wrong timestamp when it upgrades a golang package. It picks the commit timestamp of the main go package not the commit timestamp of the dependency.
So here mgechev/revive#221.

the bot updated at 2019-08-15 the package golang.org/x/sys like this

golang.org/x/sys v0.0.0-20190815002308-fde4db37ae7a

But actually it should pick the date from here golang/sys@fde4db3 which is 2019-08-13. Even if you do the upgrade manually on your laptop it will be

go get -u golang.org/x/sys
go: finding golang.org/x/sys latest
go: downloading golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a
go: extracting golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a

To Reproduce

  1. Create a simple go package with main.go
package main

import (
    "golang.org/x/sys/unix"
)

func main() {
    print(unix.Getpid())
}
  1. And do the following to build go.mod and go.sum
$ export GO111MODULE=on
$ go mod init
$ go run main.go
  1. Check go.mod and go.sum
// go.mod

go 1.12

require golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a // indirect
// go.sum
golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a h1:aYOabOQFp6Vj6W1F80affTUvO9UxmJRx8K0gsfABByQ=
golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
  1. The bot will actually upgrade to the latest commit but with wrong timestamp (mostly the current timestamp) like here https://github.com/mgechev/revive/pull/221/files
golang.org/x/sys v0.0.0-20190815002308-fde4db37ae7a // indirect
golang.org/x/sys v0.0.0-20190815002308-fde4db37ae7a h1:ZtbkXZY7vbjKdcaQck5BbF6wn7cKkfzW9xhtHmXev8A=
golang.org/x/sys v0.0.0-20190815002308-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

I think this line cause the issue

const currentDateTime = DateTime.local().toFormat('yyyyMMddHHmmss');
const newValue = `v0.0.0-${currentDateTime}-${newDigestRightSized}`;
newLine = lineToChange.replace(updateLineExp, `$1$2${newValue}`);
} else {
newLine = lineToChange.replace(updateLineExp, `$1$2${upgrade.newValue}`);

Additional context
This is fine now but once golang 1.13 released which is soon, there will be more troubles since go will validate the timestamp during package installation (See here https://tip.golang.org/doc/go1.13#version-validation) and when i try to use packages like revive that depend on the bot for upgrades with latest go version 1.13, things fail with error

go get github.com/mgechev/revive
go: finding github.com/mgechev/revive latest
go: downloading github.com/mgechev/revive v0.0.0-20190817163854-e8a54f5ffbbb
go: extracting github.com/mgechev/revive v0.0.0-20190817163854-e8a54f5ffbbb
go get: github.com/mgechev/revive@v0.0.0-20190817163854-e8a54f5ffbbb requires
	golang.org/x/sys@v0.0.0-20190815002308-fde4db37ae7a: invalid pseudo-version: does not match version-control timestamp (2019-08-13T06:44:41Z)
Makefile:16: recipe for target 'install_revive' failed
@rarkins rarkins added manager:gomod Go Modules type:bug Bug fix of existing functionality priority-2-high Bugs impacting wide number of users or very important features ready labels Aug 17, 2019
@rarkins
Copy link
Collaborator

rarkins commented Aug 19, 2019

Copied reproduction steps to https://github.com/renovate-tests/gomod6

nono added a commit to nono/cozy-apps-registry that referenced this issue Aug 20, 2019
It allows to use GOPROXY without errors. The bad pseudo versions where
introduced by renovate.

See renovatebot/renovate#4310
nono added a commit to cozy/cozy-apps-registry that referenced this issue Aug 20, 2019
It allows to use GOPROXY without errors. The bad pseudo versions where
introduced by renovate.

See renovatebot/renovate#4310
nono added a commit to nono/cozy-stack that referenced this issue Aug 20, 2019
- Fix the invalid pseudo-versions for go modules
- Set the GOPROXY=https://proxy.golang.org on both travis and appveyor

See renovatebot/renovate#4310
nono added a commit to nono/cozy-stack that referenced this issue Aug 20, 2019
- Fix the invalid pseudo-versions for go modules
- Set the GOPROXY=https://proxy.golang.org on both travis and appveyor

See renovatebot/renovate#4310
nono added a commit to nono/cozy-stack that referenced this issue Aug 20, 2019
- Fix the invalid pseudo-versions for go modules
- Set the GOPROXY=https://proxy.golang.org on both travis and appveyor

See renovatebot/renovate#4310
nono added a commit to nono/cozy-stack that referenced this issue Aug 20, 2019
- Fix the invalid pseudo-versions for go modules
- Set the GOPROXY=https://proxy.golang.org on both travis and appveyor

See renovatebot/renovate#4310
@bcmills
Copy link

bcmills commented Aug 20, 2019

Moreover, note that the code there is always using a v0.0.0- pseudo-version. v0.0.0- pseudo-versions are valid, but they have very low precedence and as such are likely to be overridden by transitive dependencies (see golang/go#27171).

A more robust approach would be to invoke go list $MODULE@$COMMIT as a subprocess and use whatever resulting version it reports, or replace all of the versions with just the desired commits and then run go mod tidy as a final pass (to resolve those commits to versions and clean up any redundant lines).

@rarkins rarkins self-assigned this Aug 20, 2019
@rarkins
Copy link
Collaborator

rarkins commented Aug 20, 2019

Thanks for the suggestion. Currently, if a dependency was required using a pseudo version and has since adopted versioning, Renovate should detect that and update it from v0.0.0 pseudo version to a "real" version. Is that what you mean, or something else?

Also, running go mod tidy after updates is already supported by setting "postUpdateOptions": ["gomodTidy"] so I don't know if that can already help or not?

@bcmills
Copy link

bcmills commented Aug 20, 2019

if a dependency was required using a pseudo version and has since adopted versioning, Renovate should detect that and update it from v0.0.0 pseudo version to a "real" version.

Pseudo-versions can end up with a greater-than-v0.0.0- prefix even if there are no canonical version tags. (For example, as part of the resolution for golang/go#31713 we ended up resolving tags with metadata to pseudo-versions derived from the base version of the tag.)

Moreover, if a module is at a major version above 0, the pseudo-version will start with the corresponding major version (e.g. v2.0.0-). See https://github.com/golang/go/blob/723852388eed2b023c7a47219ebebf722b3a7ced/src/cmd/go/internal/modfetch/coderepo_test.go#L367-L382.

So it's true that most untagged repos will have v0.0.0- pseudo-versions, but it's not 100% guaranteed.

@bcmills
Copy link

bcmills commented Aug 20, 2019

Also, running go mod tidy after updates is already supported […]

go mod tidy will take you from a commit hash to a valid pseudo-version, but won't take you from an invalid pseudo-version to a valid one. (I tried; it was too complicated.)

So go mod tidy could help, but you'd have to start by generating commit hashes instead of invalid pseudo-versions.

@rarkins
Copy link
Collaborator

rarkins commented Aug 21, 2019

@bcmills thanks again for the insights. So if we can implement a fix to generate the v0.0.0- pseudo version using the timestamp of the latest commit, and then run go mod tidy, would that solve the majority of cases?

@bcmills
Copy link

bcmills commented Aug 21, 2019

The majority? Probably, but there are known edge-cases you'd miss.

I would think that with an automated system like renovatebot you'd want to bias toward something that works 100% of the time — and that would be to use go list to resolve the version, or to write only a commit hash and use go mod tidy to resolve it.

@rarkins
Copy link
Collaborator

rarkins commented Aug 21, 2019

use go list to resolve the version

After being bitten many times in the past, we have a strong aversion to shelling out to binaries for results instead understanding the algorithms well enough to map back to our internal concepts. Of course if it's impossible then I'll reconsider, but this position really comes from painful experience. In this case my blind spot appears to be the need for non-v0.0.0 pseudo versions.

or to write only a commit hash and use go mod tidy to resolve it.

Could you elaborate on this idea? e.g. instead of v2.0.0-20170531160350-a96e63847dc3 we just put a96e63847dc3 ?

@bcmills
Copy link

bcmills commented Aug 21, 2019

instead of v2.0.0-20170531160350-a96e63847dc3 we just put a96e63847dc3?

Exactly. When the go command sees an entry in the main go.mod file that is not a valid semantic-version string, it resolves that entry to the corresponding version (which is usually a pseudo-version).

So if you just write the commit hash into the file and then invoke a go command to canonicalize it, you should get the correct corresponding version.

@bcmills
Copy link

bcmills commented Aug 21, 2019

instead understanding the algorithms well enough to map back to our internal concepts

That's fine, but in this case the inputs to the algorithm are fairly broad (potentially the entire history leading up to the requested commit, including the tags found on any ancestor commits), and even if you're excluding commits that have canonically-versioned parents there are many edge-cases to consider (see https://tip.golang.org/cmd/go/#hdr-Pseudo_versions):

  1. The module's path suffix (/vN, or .vN for gopkg.in paths) may imply a major version above v0.
  2. Pre-release tags may imply a prefix of the form vX.Y.Z-some-tag..
  3. Valid ancestor tags with metadata may imply a prefix of the form vX.Y.(Z+1)-0. — and if the repo lacks a go.mod file, the ancestor may even have a higher major version than what is implied by the module path (allowed with the +incompatible annotation as long as there is no go.mod file).

@rarkins
Copy link
Collaborator

rarkins commented Aug 26, 2019

Exactly. When the go command sees an entry in the main go.mod file that is not a valid semantic-version string, it resolves that entry to the corresponding version (which is usually a pseudo-version).

@bcmills does this work with Go 1.12 or only with Go 1.13?

Also I'm a bit confused because when I browse to https://github.com/golang/sys I see that the last commit was in July, not August. Can someone help clear up my confusion about why we're seeing August timestamps in this example?

@nono
Copy link

nono commented Aug 26, 2019

@rarkins Git tracks two dates in commits: author date and commit date. On https://github.com/golang/sys/commits/master, you can see Commits on Aug 25, 2019 (commit date), with a commit from July (author date). Below, you can see a commit with both the author date and commit date in August.

@rarkins
Copy link
Collaborator

rarkins commented Aug 26, 2019

@nono thanks for that answer - makes sense to base it on the commit date and not author date. GitHub's UI confused me there.

Here's something else I've found:

  • go does successfully convert hashes into pseudo versions as described by @bcmills
  • Unfortunately go get drops the // indirect suffix when it does it
  • go mod tidy also drops the // indirect
  • go run main.go updates the pseudo version AND does not drop // indirect, but it's not feasible for us to go run with the bot.

What should we do?

Edit: Running go 1.12.9 btw

@Clivern
Copy link
Author

Clivern commented Aug 26, 2019

@rarkins thanks for looking into this! Will you create a process that run these command on the background and commit the changes to git repository? or you want to build the latest version programmatically. I see that you started to consider running some commands so I would suggest to let golang take care of the upgrades for you and no need to invest time understanding how go build the version. I see it is a bit complicated to replicate what go do to upgrade and if go fails to upgrade then no automation will work and someone need to fix & upgrade manually.

You can even let users define on renovate.json which go version to use but i think any version will be a backward compatible.

For example this will upgrade golang.org/x/sys

$ go get -u golang.org/x/sys
$ go mod tidy

it will upgrade to the latest version of golang.org/x/sys, would this help or i am missing something.

Ref -> https://github.com/golang/go/wiki/Modules#go-111-modules

@rarkins
Copy link
Collaborator

rarkins commented Aug 26, 2019

@Clivern my strategy is:

  1. Work out programmatically that there's a newer commit
  2. Put the commit into go.mod
  3. Run a go command to let it do its magic on go.mod and go.sum

In theory we could skip (2) and maybe just use go get, but I would prefer to retain control (e.g. then we can easily offer users patch, minor, and major updates separately). So I'm looking for a command (e.g. go mod tidy) that will replace the raw git hash with a correct pseudo-version hash. Unfortunately go get and go mod tidy are updating the pseudo version but stripping the // indirect. Isn't that incorrect?

Clarification: obviously patch/minor/major is not applicable for pseudo version updates, but it is for true modules and we want a similar algorithm for all.

@Clivern
Copy link
Author

Clivern commented Aug 26, 2019

It may strip some // indirect dependencies or it may add new ones. I think we should never worry about what go mod tidy add or remove. It always ensures your current go.mod reflects the dependency requirements for all possible combinations of OS, architecture, and build tags.

So whenever you upgrade a dependency:

  • there may be a new indirect dependencies introduced and go mod tidy will add these indirect dependencies with // indirect flag.
  • or one of the indirect dependencies need to be removed and go mod tidy will remove it.

In general, it make sure that go.mod and go.sum records a precise dependency information whenever you run the command and that is fine. no one wants a redundant indirect dependencies or want to miss a new indirect dependency after upgrade.

@rarkins
Copy link
Collaborator

rarkins commented Aug 26, 2019

In that case I think we are ok to fix the original topic of this issue (v0.0.0 pseudo version with wrong time stamp). I have not verified yet if it also works with the other edge cases identified but hopefully it should.

@Clivern
Copy link
Author

Clivern commented Aug 26, 2019

The majority? Probably, but there are known edge-cases you'd miss.

I would think that with an automated system like renovatebot you'd want to bias toward something that works 100% of the time — and that would be to use go list to resolve the version, or to write only a commit hash and use go mod tidy to resolve it.

hopefully it covers the majority, i think that's also what @bcmills suggested.

@rarkins
Copy link
Collaborator

rarkins commented Aug 26, 2019

We will run “go get” 100% of the time and users can opt into “go mod tidy” 100% of the time too. If there are edge cases still missed then I’ll look into it again

@renovate-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 19.34.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
manager:gomod Go Modules priority-2-high Bugs impacting wide number of users or very important features type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants