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

Open
mvdan opened this issue Feb 25, 2020 · 18 comments
Open

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

mvdan opened this issue Feb 25, 2020 · 18 comments
Assignees
Milestone

Comments

@mvdan
Copy link
Member

@mvdan 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
Member

@bcmills 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 this to the Go1.15 milestone Feb 25, 2020
@mvdan
Copy link
Member Author

@mvdan 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
Member

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

@ianlancetaylor ianlancetaylor commented Jun 16, 2020

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
@bcmills
Copy link
Member

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

@gopherbot gopherbot commented Sep 14, 2020

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

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 15, 2020

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

@gopherbot gopherbot commented Sep 18, 2020

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

@gopherbot gopherbot commented Sep 23, 2020

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

@gopherbot gopherbot commented Sep 23, 2020

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

@gopherbot gopherbot commented Sep 29, 2020

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

@gopherbot gopherbot commented Oct 1, 2020

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

@gopherbot gopherbot commented Oct 2, 2020

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

@gopherbot gopherbot commented Oct 17, 2020

Change https://golang.org/cl/263266 mentions this issue: cmd/go/internal/modload: fix sort condition in (*replacementRepo).Versions

gopherbot pushed a commit that referenced this issue Oct 17, 2020
…sions

In CL 258220 I added replacement versions to the repo versions used in
the modload.Query functions. The versions are computed from a map in
the modfile index, which has a nondeterministic iteration order.

I added a short-circuit condition to skip sorting in the (vastly
common) case where no replacement versions are added. However, while
cleaning up the change I accidentally deleted the line of code that
sets that condition. As a result, the test of that functionality
(mod_get_replaced) has been failing nondeterministically.

This change fixes the condition by comparing the slices before and
after adding versions, rather than by setting a separate variable.
The test now passes reliably (tested with -count=200).

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

Change-Id: I49a66a3a5510da00ef42b47f20a168de66100db6
Reviewed-on: https://go-review.googlesource.com/c/go/+/263266
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
@gopherbot
Copy link

@gopherbot gopherbot commented Oct 28, 2020

Change https://golang.org/cl/266018 mentions this issue: cmd/go: allow 'go get' to downgrade to replacement-only versions

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 28, 2020

Change https://golang.org/cl/263267 mentions this issue: cmd/go/internal/modget: resolve paths at the requested versions

gopherbot pushed a commit that referenced this issue Oct 29, 2020
This fixes a case missed in CL 258220.

For #36460
Updates #26241
Updates #37438

Change-Id: I5e8c2ee1e08e41cc2eb34e54c617cb5e4bf69c5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/266018
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
Copy link

@gopherbot gopherbot commented Oct 29, 2020

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

@gopherbot gopherbot commented Oct 29, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.