renovatebot / renovate

Home of the Renovate CLI: Cross-platform Dependency Automation by Mend.io
https://mend.io/renovate
GNU Affero General Public License v3.0
17.68k stars 2.33k forks source link

Bot upgrades golang packages to Invalid pseudo-version #4310

Closed Clivern closed 5 years ago

Clivern commented 5 years ago

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 https://github.com/mgechev/revive/pull/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 https://github.com/golang/sys/commit/fde4db37ae7ad8191b03d30d27f258b5291ae4e3 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()) }

2.  And do the following to build go.mod and go.sum

$ export GO111MODULE=on $ go mod init $ go run main.go


3. 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=

4. 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
https://github.com/renovatebot/renovate/blob/6f99118f7cfb9a646148455938793c8894d10a01/lib/manager/gomod/update.ts#L60-L64

**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 commented 5 years ago

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

bcmills commented 5 years ago

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 https://github.com/golang/go/issues/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 commented 5 years ago

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 commented 5 years ago

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 https://github.com/golang/go/issues/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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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:

What should we do?

Edit: Running go 1.12.9 btw

Clivern commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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:

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

:tada: This issue has been resolved in version 19.34.5 :tada:

The release is available on:

Your semantic-release bot :package::rocket: