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/go: go get module/pkg@master doesn't seem to work sometimes #37438

Closed
mvdan opened this issue Feb 25, 2020 · 28 comments
Closed

cmd/go: go get module/pkg@master doesn't seem to work sometimes #37438

mvdan opened this issue Feb 25, 2020 · 28 comments
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Feb 25, 2020

Here is a standalone reproducer:

#!/bin/bash
  
docker run -i golang:1.14rc1 <<-SCRIPT

        mkdir foo
        cd foo
        go mod init test.tld/foo

        go get -d github.com/grpc-ecosystem/go-grpc-prometheus@v1.2.0
        go list -m -f {{.Version}} github.com/grpc-ecosystem/go-grpc-prometheus

        go get -d github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus@master

SCRIPT

It's minified from a real situation at work. You can see that https://github.com/grpc-ecosystem/go-grpc-prometheus/tree/master/packages/grpcstatus exists at master, but it didn't exist when 1.2.0 was tagged, which is the latest release.

The output is:

go get github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus@master: module github.com/grpc-ecosystem/go-grpc-prometheus@latest found (v1.2.0), but does not contain package github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus

This message doesn't make sense to me. If I asked for @master, why is it settling for @latest?

If I replace the commands for go get module@master; go build module/pkg, it works:

go get -d github.com/grpc-ecosystem/go-grpc-prometheus@master
go build github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus

This seems like a bug. I'm not sure what exactly is triggering it. CC @bcmills @jayconrod

@mvdan mvdan added the modules label Feb 25, 2020
@bcmills
Copy link
Contributor

bcmills commented Feb 25, 2020

I can confirm that something is strange here.

go get manages to find the module, but then (for some reason) tries to re-resolve the package without that module.

foo$ go version
go version devel +450d0b2f Tue Feb 25 08:36:15 2020 +0000 linux/amd64

foo$ go mod init test.tld/foo
go: creating new go.mod: module test.tld/foo

foo$ go get -d github.com/grpc-ecosystem/go-grpc-prometheus@v1.2.0
…

foo$ go list -m -f {{.Version}} github.com/grpc-ecosystem/go-grpc-prometheus
v1.2.0

foo$ go get -d github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus@master
go: downloading github.com/grpc-ecosystem/go-grpc-prometheus v1.2.1-0.20191002090509-6af20e3a5340
go: found github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus in github.com/grpc-ecosystem/go-grpc-prometheus v1.2.1-0.20191002090509-6af20e3a5340
go: finding module for package github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus
go get github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus@master: module github.com/grpc-ecosystem/go-grpc-prometheus@latest found (v1.2.0), but does not contain package github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus

CC @matloob

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 25, 2020
@bcmills bcmills added this to the Go1.15 milestone Feb 25, 2020
@mvdan
Copy link
Member Author

mvdan commented Feb 25, 2020

Precisely. Almost like it forgets that I specified @master halfway through its work. This might be an edge case, such as @master doesn't work when go-getting a package path within a module when the module is already at the latest version in go.mod.

@bcmills
Copy link
Contributor

bcmills commented Feb 25, 2020

I can reproduce the same behavior with an explicit commit hash. Having the module already present at v1.2.0 seems to be essential.

foo$ go get -d github.com/grpc-ecosystem/go-grpc-prometheus@v1.2.0
go: downloading github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
go: finding module for package github.com/prometheus/client_golang/prometheus
go: finding module for package golang.org/x/net/context
go: finding module for package google.golang.org/grpc
go: finding module for package google.golang.org/grpc/status
go: finding module for package google.golang.org/grpc/codes
go: downloading github.com/prometheus/client_golang v1.4.1
go: downloading google.golang.org/grpc v1.27.1
go: downloading golang.org/x/net v0.0.0-20200222125558-5a598a2470a0
go: found github.com/prometheus/client_golang/prometheus in github.com/prometheus/client_golang v1.4.1
go: found golang.org/x/net/context in golang.org/x/net v0.0.0-20200222125558-5a598a2470a0
go: found google.golang.org/grpc in google.golang.org/grpc v1.27.1
go: found google.golang.org/grpc/codes in google.golang.org/grpc v1.27.1
go: found google.golang.org/grpc/status in google.golang.org/grpc v1.27.1
go: downloading github.com/golang/protobuf v1.3.2
go: downloading golang.org/x/sys v0.0.0-20200122134326-e047566fdf82
go: downloading github.com/prometheus/common v0.9.1
go: downloading github.com/cespare/xxhash/v2 v2.1.1
go: downloading github.com/beorn7/perks v1.0.1
go: downloading golang.org/x/text v0.3.0
go: downloading github.com/prometheus/procfs v0.0.8
go: downloading github.com/prometheus/client_model v0.2.0
go: downloading google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55
go: downloading github.com/matttproud/golang_protobuf_extensions v1.0.1

foo$ go get -d github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus@6af20e3a5340
go: found github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus in github.com/grpc-ecosystem/go-grpc-prometheus v1.2.1-0.20191002090509-6af20e3a5340
go: finding module for package github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus
go get github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus@6af20e3a5340: module github.com/grpc-ecosystem/go-grpc-prometheus@latest found (v1.2.0), but does not contain package github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus

@ianlancetaylor
Copy link
Contributor

I guess this didn't get fixed for 1.15. Moving milestone to 1.16.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.15, Go1.16 Jun 16, 2020
@ianlancetaylor ianlancetaylor added the GoCommand cmd/go label Jun 16, 2020
@bcmills
Copy link
Contributor

bcmills commented Jul 27, 2020

Almost like it forgets that I specified @master halfway through its work.

I don't think it's quite that. Rather, it is trying to classify the arguments as either packages or modules before it computes the upgrades, and at that point in time github.com/grpc-ecosystem/go-grpc-prometheus/packages/grpcstatus is neither. (It is not yet a package because we haven't upgraded to the version containing that package, and it is certainly not a module either.)

@gopherbot
Copy link

Change https://golang.org/cl/254820 mentions this issue: cmd/go/internal/modget: factor out functions for argument resolution

@gopherbot
Copy link

Change https://golang.org/cl/255054 mentions this issue: cmd/go: test the behavior of 'go get' in module mode with package vs. module arguments

gopherbot pushed a commit that referenced this issue Sep 15, 2020
For #37438
For #41315
For #36460

Change-Id: I17041c35ec91ff6ffb547e0f32572673d191b1ed
Reviewed-on: https://go-review.googlesource.com/c/go/+/254820
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/255938 mentions this issue: cmd/go/internal/modget: consolidate Load entrypoints

gopherbot pushed a commit that referenced this issue Sep 18, 2020
… module arguments

Updates #37438

Change-Id: I5beb380b37532571768a92bea50003f6ff1757e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/255054
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue Sep 22, 2020
This change replaces ImportPaths, ImportPathsQuiet, LoadALL, and
LoadVendor with a single LoadPackages function, with a LoadOpts struct
that more clearly documents the variations in behavior.

It also eliminates the cmd/go/internal/load.ImportPaths function,
which was undocumented and had only one call site (within its own
package).

The modload.LoadTests global variable is subsumed by a field in the
new LoadOpts struct, and is no longer needed for callers that invoke
LoadPackages directly. It has been (temporarily) replaced with a
similar global variable, load.ModResolveTests, which can itself be
converted to an explicit, local argument.

For #37438
For #36460
Updates #40775
Fixes #26977

Change-Id: I4fb6086c01b04de829d98875db19cf0118d40f8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/255938
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/256799 mentions this issue: cmd/go: add another test case for package/module ambiguity in 'go get'

@gopherbot
Copy link

Change https://golang.org/cl/256922 mentions this issue: cmd/go: add yet another test case for ambiguous arguments to 'go get'

@gopherbot
Copy link

Change https://golang.org/cl/258220 mentions this issue: cmd/go/internal/modload: rework replacements in the Query functions

gopherbot pushed a commit that referenced this issue Sep 30, 2020
For #37438

Change-Id: Iae00ef7f97144e85f4f710cdb3087c2548b4b8f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/256799
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue Sep 30, 2020
For #37438

Change-Id: Ie40971ff677d36ddadbf9834bba2d366a0fc34d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/256922
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/258717 mentions this issue: internal/lsp/testdata: remove diagnostic from percent package

gopherbot pushed a commit to golang/tools that referenced this issue Oct 1, 2020
The percent package has an invalid import path and a disallowed
character in a source file. In CL 258298, I am changing cmd/go to
diagnose invalid import paths during loading (instead of during
missing-import resolution), and as a result 'go list' will no longer
attempt to load or enumerate the source files for that package.

It is important that gopls and 'go list' not crash when attempting to
load a package with an invalid path, but gopls should not assume that
'go list' will produce anything more than an error for it.

For golang/go#37438
For golang/go#41576

Change-Id: I8af8896ea7108f1588e0085ddc1bf1b9ff55d5b9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/258717
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/259158 mentions this issue: cmd/go/internal/modfetch: remove error return from Lookup

gopherbot pushed a commit that referenced this issue Oct 13, 2020
We generally don't care about errors in resolving a repo if the result
we're looking for is already in the module cache. Moreover, we can
avoid some expense in initializing the repo if all of the methods we
plan to call on it hit in the cache — especially when using
GOPROXY=direct.

This also incidentally fixes a possible (but rare) bug in Download:
we had forgotten to reset the downloaded file in case the Zip method
returned an error after writing a nonzero number of bytes.

For #37438

Change-Id: Ib64f10f763f6d1936536b8e1f7d31ed1b463e955
Reviewed-on: https://go-review.googlesource.com/c/go/+/259158
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue Oct 16, 2020
'go mod tidy' has been able to use replaced versions since CL 152739,
but 'go get' failed for many of the same paths. Now that we are
recommending 'go get' more aggressively due to #40728, we should make
that work too.

In the future, we might consider factoring out the new replacementRepo
type so that 'go list' can report the new versions as well.

For #41577
For #41416
For #37438
Updates #26241

Change-Id: I9140c556424b584fdd9bdd0a747842774664a7d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/258220
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/266339 mentions this issue: cmd/go/internal/mvs: omit modules at version "none" in BuildList and Req

@gopherbot
Copy link

Change https://golang.org/cl/266340 mentions this issue: cmd/go/internal/modload: ensure that modRoot and targetPrefix are initialized in DirImportPath

@gopherbot
Copy link

Change https://golang.org/cl/266369 mentions this issue: cmd/go: make TestScript/mod_get_patchmod self-contained

@gopherbot
Copy link

Change https://golang.org/cl/266370 mentions this issue: cmd/go/internal/modload: handle NotExist errors in (*mvsReqs).Previous

@gopherbot
Copy link

Change https://golang.org/cl/266657 mentions this issue: cmd/go/internal/modload: return a module-only result from QueryPattern

gopherbot pushed a commit that referenced this issue Oct 30, 2020
For #37438

Change-Id: Icb28035ae4027aa09d8959d4ac2f4b94a6c843a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/266339
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Oct 30, 2020
…tialized in DirImportPath

For #37438

Change-Id: I2e1f47d567842ac5504b7b8ed0b3fba6f92d778b
Reviewed-on: https://go-review.googlesource.com/c/go/+/266340
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Bryan C. Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Oct 30, 2020
I find it pretty difficult to reason about test-dependency modules
when they aren't in the same file as the rest of the test.

Now that 'go get' supports replacements (CL 258220 and CL 266018),
we can localize tests that need 'go get' but don't specifically depend
on module proxy semantics.

For #36460
For #37438

Change-Id: Ib37a6c170f251435399dfc23e60d96681a81eadc
Reviewed-on: https://go-review.googlesource.com/c/go/+/266369
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Oct 30, 2020
Previous is used during downgrading. If the module proxy does not
advertise any versions (for example, because it contains only
pseudo-versions), then Previous should return "none" instead of a
non-nil error.

For #37438

Change-Id: I4edfec19cfeb3ffe50df4979f99a01321c442509
Reviewed-on: https://go-review.googlesource.com/c/go/+/266370
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/266717 mentions this issue: cmd/go/internal/modload: add structured errors for queries matching the main module

@gopherbot
Copy link

Change https://golang.org/cl/266860 mentions this issue: cmd/go/internal/modload: fix (*mvsReqs).Max when the second argument is the empty string

@gopherbot
Copy link

Change https://golang.org/cl/266859 mentions this issue: cmd/go/internal/mvs: test a downgrade where the target explicitly requires itself

@gopherbot
Copy link

Change https://golang.org/cl/266897 mentions this issue: cmd/go/internal/mvs: in Upgrade, pass upgrades to buildList as upgrades

gopherbot pushed a commit that referenced this issue Nov 5, 2020
This allows a single QueryPattern call to resolve a path that could be
either a package or a module. It is important to be able to make a
single QueryPattern call — rather than a QueryPattern followed by a
Query for the specific module path — to provide appropriate fallback
behavior: if the proxy returns package results but does not contain a
module result, we don't want to fall back to the next proxy to look
for the (probably-nonexistent) module.

For #37438

Change-Id: I419b8bb3ab4565f443bb5cee9a8b206f453b9801
Reviewed-on: https://go-review.googlesource.com/c/go/+/266657
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue Nov 5, 2020
…he main module

For #37438

Change-Id: I7df80ae0917b0b4ecad98947da39ddf8554b07c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/266717
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue Nov 5, 2020
…uires itself

Also clean up the test assertions, and add a check for assertions
missing function invocations (there was one).

For #37438

Change-Id: Iafbfeae2c25217eac894181e01480b25b7cffbd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/266859
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopherbot pushed a commit that referenced this issue Nov 5, 2020
This has no impact on the resulting build list, but provides clearer
diagnostics if reqs.Required returns an error for one of the upgraded
modules.

For #37438

Change-Id: I5cd8f72a9b7b9a0b185e1a728f46fefbd2f09b4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/266897
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
gopherbot pushed a commit that referenced this issue Nov 5, 2020
…is the empty string

As far as I can tell, this bug had gone unnoticed because everything
that uses Max so far happened to only ever present the empty string as
the first argument.

For #37438

Change-Id: Ie8c42313157d2c2c17e4058c53b5bb026b95a1c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/266860
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/268117 mentions this issue: cmd/go/internal/modget: fix a typo introduced in CL 263267

gopherbot pushed a commit that referenced this issue Nov 9, 2020
Updates #37438

Change-Id: I78f377afd73dad75aed219836725a27fbaa5b69c
Reviewed-on: https://go-review.googlesource.com/c/go/+/268117
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Dan Peterson <dpiddy@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/272128 mentions this issue: cmd/go/internal/modload: remove a stale comment for EditBuildList

gopherbot pushed a commit that referenced this issue Nov 21, 2020
For #36460
Updates #37438

Change-Id: I1626d40e78b110035a893b1b80dbd2279bf50ffe
Reviewed-on: https://go-review.googlesource.com/c/go/+/272128
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/287415 mentions this issue: content/static/doc: document 'go get' requiring version for @patch

gopherbot pushed a commit to golang/website that referenced this issue Feb 16, 2021
For golang/go#37438

Change-Id: I077f364a4c484de4b5f1554207bfd250947198e6
Reviewed-on: https://go-review.googlesource.com/c/website/+/287415
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Jan 27, 2022
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants