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

Wrong update version for go module in subdirectory #4589

Closed
hairyhenderson opened this issue Oct 5, 2019 · 25 comments
Closed

Wrong update version for go module in subdirectory #4589

hairyhenderson opened this issue Oct 5, 2019 · 25 comments
Labels
help wanted Help is needed or welcomed on this issue manager:gomod Go Modules priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality

Comments

@hairyhenderson
Copy link

What Renovate type are you using?

Hosted

Describe the bug

See hairyhenderson/gomplate#603 (comment) for the error.

Looks like renovate picked up the github.com/hashicorp/consul v1.6.1 tag, however the github.com/hashicorp/consul/api module is at v1.2.0 still. This is reflected by the presence of an api/v1.2.0 tag.

Did you see anything helpful in debug logs?

Nope!

To Reproduce
Steps to reproduce the behavior:

  1. Create repository with a dependency on the github.com/hashicorp/consul/api package
  2. Watch renovate get confused and try to bump the version for that module to v1.2.0!

Expected behavior

Renovate should understand the api/v1.2.0 tag.

Screenshots

See hairyhenderson/gomplate#603

@rarkins
Copy link
Collaborator

rarkins commented Oct 6, 2019

Here's the update:
image

It's basing this on the raw hashicorp/consul releases:
image

But actually it should be following the api/* releases:
image

@hairyhenderson I don't suppose you already know where this Go behaviour is specified? I want to make sure there's nothing else we're missing.

@rarkins rarkins added manager:gomod Go Modules type:bug Bug fix of existing functionality help wanted Help is needed or welcomed on this issue needs-requirements labels Oct 6, 2019
@rarkins
Copy link
Collaborator

rarkins commented Oct 6, 2019

@viceice I'm thinking we might need to try to leverage our "compatibility" versioning concept here, like we do with Docker suffixes

@viceice
Copy link
Member

viceice commented Oct 7, 2019

What about to have prefix and suffix options?

@rarkins
Copy link
Collaborator

rarkins commented Oct 7, 2019

I'm trying to locate some Go specs on this first before making a decision. For example I want to see if this is always how the versions work, if there is any fallback to non-prefixed releases, and if there is possibility to nest more than one deep. I don't suppose @bcmills can point us in the right direction documentation/specification-wise? Thanks

@hairyhenderson
Copy link
Author

I don't suppose you already know where this Go behaviour is specified? I want to make sure there's nothing else we're missing.

There is some mention here in the Go Modules wiki: https://github.com/golang/go/wiki/Modules#faqs--multi-module-repositories:

With some somewhat-cryptic statements like:

The tag for version 1.2.3 of module "my-repo/foo/rop" is "foo/rop/v1.2.3".

I haven't found anything more "official" than that though...

@rarkins
Copy link
Collaborator

rarkins commented Oct 8, 2019

Thanks for that. Sounds like we need to support arbitrary depth then.

@bcmills
Copy link

bcmills commented Oct 8, 2019

I don't suppose @bcmills can point us in the right direction documentation/specification-wise?

I've said it before and I'll say it again: I recommend that you use go list -m instead of reimplementing the go command's logic to resolve the version string for a given commit.

@rarkins
Copy link
Collaborator

rarkins commented Oct 8, 2019

You're specifically referring to go list -m -versions, right?

@rarkins
Copy link
Collaborator

rarkins commented Oct 8, 2019

dummy example:

❯ cat go.mod
module arkins.net/test1

go 1.13

require rsc.io/quote v1.5.2

Get all deps:

❯ go list -m all
arkins.net/test1
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c
rsc.io/quote v1.5.2
rsc.io/sampler v1.3.0

Get all updates:

❯ go list -m -versions all
arkins.net/test1
golang.org/x/text v0.1.0 v0.2.0 v0.3.0 v0.3.1 v0.3.2
rsc.io/quote v1.0.0 v1.1.0 v1.2.0 v1.2.1 v1.3.0 v1.4.0 v1.5.0 v1.5.1 v1.5.2 v1.5.3-pre1
rsc.io/sampler v1.0.0 v1.2.0 v1.2.1 v1.3.0 v1.3.1 v1.99.99

Get one update at a time:

❯ go list -m -versions rsc.io/sampler
rsc.io/sampler v1.0.0 v1.2.0 v1.2.1 v1.3.0 v1.3.1 v1.99.99

Try to get an update for a module not in the go.mod:

❯ go list -m -versions github.com/hashicorp/consul/api
go: finding github.com/hashicorp/consul/api v1.2.0
github.com/hashicorp/consul/api v1.0.0 v1.0.1 v1.1.0 v1.2.0

Attempting to look up versions from a random directory:

❯ mkdir /tmp/gomodtest2 && cd /tmp/gomodtest2 && go list -m -versions github.com/hashicorp/consul/api
go list -m: not using modules

In conclusion: if we have a "dummy" modules project to run the child process from, it seems we can indeed shell out to go for the lookups.

The main thing I'd been worried about was that we wouldn't get back the full list of versions, but it appears that's not a problem here. So I think this approach is indeed compatible with Renovate's logic.

One more concern: Can this go list command be run concurrently (same working directory, but for different modules), or must they be run sequentially?

@bcmills
Copy link

bcmills commented Oct 8, 2019

You're specifically referring to go list -m -versions, right?

-versons, but also go list -m $MODULE@$COMMIT, which will resolve $COMMIT to the pseudo-version the go command would use for that commit.

go list -m: not using modules

You can set GO111MODULE=on explicitly to activate module mode when outside of a module.

@bcmills
Copy link

bcmills commented Oct 8, 2019

Can this go list command be run concurrently (same working directory, but for different modules), or must they be run sequentially?

go list in module mode has been safe for concurrent use since Go 1.12.

@bcmills
Copy link

bcmills commented Oct 8, 2019

For example:

$ GO111MODULE=on go1.13.1 list -m github.com/hashicorp/consul/api@6439af8
go: finding github.com/hashicorp/consul/api 6439af8
github.com/hashicorp/consul/api v1.2.1-0.20191007211523-6439af86ebe3

@rarkins
Copy link
Collaborator

rarkins commented Oct 8, 2019

It just occurred to me that the list all command is including indirect dependencies, which I don’t think Renovate should be updating. Is there any lost command that lists all direct dependencies but no indirect?

@bcmills
Copy link

bcmills commented Oct 8, 2019

There isn't a simple one at the moment, no.
(But there is probably a complicated one: see golang/go#27887 (comment).)

@tbpg
Copy link

tbpg commented Oct 15, 2019

What about iterating through the "Require" list of go mod edit -json, passing each "Path" to go list -m -versions $path?

@bcmills
Copy link

bcmills commented Oct 21, 2019

Yes, you could do that — just be sure to check the Indirect field.

@vvakame
Copy link
Contributor

vvakame commented Nov 26, 2019

@HonkingGoose HonkingGoose added priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others and removed priority-5-triage labels Apr 1, 2021
@HonkingGoose
Copy link
Collaborator

Pinging @rarkins to take a look at this, as the last movement on this issue has been in November of 2019. So I think it's due a slight nudge/update. 😉

I've marked the issue priority-3-normal as it does seem that this is something that @rarkins wants to implement.

@nabeelsaabna
Copy link
Contributor

nabeelsaabna commented May 20, 2022

Hi @rarkins,

  1. Any reason why we are not simply implementing a smart lookup for Go ?
    In case of github.com/org-name/my-repo/foo/rop to check the tags starting with foo/rop/
  2. Is there a consideration to actually using the go command ?

@viceice
Copy link
Member

viceice commented May 20, 2022

Is there a consideration to actually using the go command ?

Yes, we only use those binaries for artifacts updating (performance)

@nabeelsaabna
Copy link
Contributor

Then we are left with only the first option 😃

@nabeelsaabna
Copy link
Contributor

@rarkins is there any reason to not implement the first suggested solution ?

@rarkins
Copy link
Collaborator

rarkins commented May 23, 2022

No reason. We should try to mimic Go's logic (including if it falls back to using plain tags)

@PhilipAbed
Copy link
Collaborator

@rarkins @nabeelsaabna i cant reproduce the issue, i tried debugging apparently @zharinov already fixed it in his refactor of go back in 27 October 2021 : #12359

here:

// Filter the releases so that we only get the ones that are for this submodule
// Also trim the submodule prefix from the version number
const submodReleases = res.releases
.filter((release) => release.version?.startsWith(prefix))
.map((release) => {
const r2 = release;
r2.version = r2.version.replace(`${prefix}/`, '');
return r2;
});
logger.trace({ submodReleases }, 'go.getReleases');

@rarkins rarkins closed this as completed May 23, 2022
@nabeelsaabna
Copy link
Contributor

screen-short confirmation - each go module updated correctly to it's latest version/tag
image

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
help wanted Help is needed or welcomed on this issue manager:gomod Go Modules priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:bug Bug fix of existing functionality
Projects
None yet
Development

No branches or pull requests

9 participants