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 list prints the wrong Origin data for the version resolved from a non-semver query #61423

Closed
bcmills opened this issue Jul 18, 2023 · 5 comments
Assignees
Labels
GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Jul 18, 2023

What version of Go are you using (go version)?

$ go version
go version devel go1.21-18e17e2c Thu Jun 29 23:06:46 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/usr/local/google/home/bcmills/.cache/go-build'
GOENV='/usr/local/google/home/bcmills/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/tmp/tmp.oKChmSObvX/.gopath/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/tmp/tmp.oKChmSObvX/.gopath'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/google/home/bcmills/sdk/gotip'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/google/home/bcmills/sdk/gotip/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.21-18e17e2c Thu Jun 29 23:06:46 2023 +0000'
GCCGO='/usr/bin/gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3913236638=/tmp/go-build -gno-record-gcc-switches'

What did you do?

$ go list -json -m -e golang.org/x/exp@master

What did you expect to see?

A result with either a non-cacheable Origin (currently, an empty Hash, TagSum, and RepoSum), or an Origin with a Ref matching the one we would need to re-resolve in the upstream repo.

What did you see instead?

When a proxy is used, the result may include a cacheable Origin, as if the command had requested the fully-resolved version instead of a query:

$ go list -json -m -e golang.org/x/exp@master
{
        "Path": "golang.org/x/exp",
        "Version": "v0.0.0-20230713183714-613f0c0eb8a1",
        "Query": "master",
        "Time": "2023-07-13T18:37:14Z",
        "GoMod": "/tmp/tmp.oKChmSObvX/.gopath/pkg/mod/cache/download/golang.org/x/exp/@v/v0.0.0-20230713183714-613f0c0eb8a1.mod",
        "GoVersion": "1.20",
        "Origin": {
                "VCS": "git",
                "URL": "https://go.googlesource.com/exp",
                "Hash": "613f0c0eb8a17a98ecdb096a7f9f7d5053c1c963"
        }
}
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go modules labels Jul 18, 2023
@bcmills bcmills added this to the Go1.22 milestone Jul 18, 2023
@bcmills
Copy link
Contributor Author

bcmills commented Jul 18, 2023

(Found while investigating #61415.)

@bcmills bcmills changed the title cmd/go: avoid printing cacheable Origin information for a version resolved from a non-semver query cmd/go: go list should avoid printing a cacheable Origin for a version resolved from a non-semver query Jul 18, 2023
@bcmills bcmills changed the title cmd/go: go list should avoid printing a cacheable Origin for a version resolved from a non-semver query cmd/go: go list prints the wrong Origin data for the version resolved from a non-semver query Jul 18, 2023
@seankhliao
Copy link
Member

if the Version field prints the resolved version, why shouldn't the Origin field print the origin data corresponding to that version?

@bcmills
Copy link
Contributor Author

bcmills commented Jul 19, 2023

Perhaps it should?

But in that case, the -reuse flag will need to know to invalidate it.

@bcmills bcmills self-assigned this Jul 19, 2023
@gopherbot
Copy link

Change https://go.dev/cl/543155 mentions this issue: cmd/go: check that expected Origin fields are present to reuse module info

@gopherbot
Copy link

Change https://go.dev/cl/543216 mentions this issue: cmd/go: use a nil Origin to represent "unchackable"

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 17, 2023
gopherbot pushed a commit that referenced this issue Nov 17, 2023
Previously, we used the presence of individual origin fields
to decide whether an Origin could be checked for staleness,
with a nil Origin representing “use whatever you have”.

However, that turns out to be fairly bug-prone: if we forget
to populate an Origin somewhere, we end up with an incomplete
check instead of a non-reusable origin (see #61415, #61423).

As of CL 543155, the reusability check for a given query
now depends on what is needed by the query more than what
is populated in the origin. With that in place, we can simplify
the handling of the Origin struct by using a nil pointer
to represent inconsistent or unavailable origin data, and
otherwise always reporting whatever origin information we have
regardless of whether we expect it to be reused.

Updates #61415.
Updates #61423.

Change-Id: I97c51063d6c2afa394a05bf304a80c72c08f82cf
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/543216
Auto-Submit: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants