cmd/go/internal/modfetch: module path validation inconsistent between repo and proxy fetch paths

[shrajfr12]

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

$ go version
go version go1.11.4 darwin/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
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
...

What did you do?

These are the reproduction steps posted in gocenter gopher slack channel by @thepudds.

mkdir /tmp/scratchpad/hashicorp-vault-1
cd /tmp/scratchpad/hashicorp-vault-1

export GOPROXY='https://gocenter.io'
export GOPATH='/tmp/go-path-for-hashicorp-vault-test-1'

go mod init tempmod

cat < main.go
package x
import (
_ "github.com/hashicorp/vault/api"
)
func main() {}
EOF
go mod tidy

go list -m all | wc -l
go list -m all > list.all.out
find $GOPATH/pkg/mod -name "*.mod" | wc -l
du -sh $GOPATH/pkg/mod

Repeat this in another window with different sets of directories(Replace -1 with -2 for example) but without setting GOPROXY.

You can see the du output in the GOPROXY window to be significantly different in the window without GOPROXY setting and the list of modules (list_all.out) shows difference in the version number for one of the dependencies (github.com/pierrec/lz4)

What did you expect to see?

In the both cases with GOPROXY and without GOPROXY, expect to see the dependencies of modules to be resolved to the same version

What did you see instead?

There is a difference in the du output as well as the content of the list of modules with and without GOPROXY,
With GOPROXY, the modules list shows
github.com/pierrec/lz4 v2.1.1+incompatible // indirect
Without GOPROXY, the modules list shows
github.com/pierrec/lz4 v2.0.5+incompatible // indirect

Please note that the actual dependency based on vendor.json in the project github.com/hashicorp/vault is actually v0.0.0-20181005164709-635575b42742 (go mod init in that project generates this correctly) but go mod tidy overwrites it with a different version.

[thepudds]

I have not looked at this carefully, but my initial suspicion would be a difference in behavior in how v2+ modules with bad go.mod files are handled.

If you do go list -m all in each case outlined above and diff the results:

$ diff gocenter-list-all.out no-gocenter-list-all.out
21,22c21
< github.com/pierrec/lz4 v2.1.1+incompatible
< github.com/pkg/profile v1.2.1
---
> github.com/pierrec/lz4 v2.0.5+incompatible

I checked to see if pierrec/lz4 has an incorrect go.mod, which seems to be the case:
https://github.com/pierrec/lz4/blob/v2.1.1/go.mod

The bad go.mod here is missing the required /v2 at the end of its module path on the first line even though it has a VCS tag of v2.x.x, which violates the requirements of:
https://github.com/golang/go/wiki/Modules#semantic-import-versioning

v2.0.5 is one of the versions selected above. It seems v2.0.5 is the version just before the bad go.mod was introduced. By default in at least some cases, the go tool can pick the version just before the introduction of a go.mod that is "bad" in this way for a v2+ dependency.

I would therefore suspect either the go tool is treating this error situation differently based on whether or not GOPROXY is set, or perhaps instead GoCenter is treating this error situation differently than the go tool, or something along those lines.

[thepudds]

@shrajfr12 I would wonder if you or one of your colleagues could try reproducing this with a local file-based GOPROXY setting? This could help differentiate if the go tool display the same difference in behavior with and without a local file-based GOPROXY setting, which would tend to hint strongly that the difference in behavior is not specific to GoCenter.

Perhaps it would not work for some reason, or perhaps it would not be conclusive, but could be worth trying.

In general, and I suspect this is extremely well known to you at this point:

• The module download cache location is controlled by GOPATH. In particular, go mod download, go build, etc. populate the module cache in GOPATH/pkg/mod.
• In addition, when you want to use a particular module cache, you can tell the go command to use a local module cache by setting GOPROXY=file:///file/path.
• You can put those two things together:

   # Populate a module download cache in /tmp/gopath-for-cache
   $ GOPATH=/tmp/gopath-for-cache  go mod tidy

   # Run go mod tidy using the contents of the module download cache in /tmp/gopath-for-cache
   $ GOPROXY=file:///tmp/gopath-for-cache/pkg/mod/cache/download  go mod tidy

In between those two steps, you could try doing go get to populate the module cache with the full set of versions from the initial report, such as something like:

go get github.com/pierrec/[email protected]
go get github.com/pkg/[email protected]
go get github.com/pierrec/[email protected]

(Those versions are taken from the diff output).

[thepudds]

Also, regarding this (with italics added):

What did you expect to see?

In the both cases with GOPROXY and without GOPROXY, expect to see same output for du command and same content for modules list with version number and module name matching exactly.

Personally, I would expect the du command to show different file sizes. One of the benefits of GOPROXY-based module mirrors is that individual go.mod files can be downloaded via a more efficient HTTPS request, and without always requiring all of the code to be downloaded or fetched from git just to get a single go.mod file.

In other words, the hope would be that setting GOPROXY would show a smaller on-disk size for the module download cache, in addition to also executing faster.

[bcmills]

perhaps instead GoCenter is treating this error situation differently than the go tool, or something along those lines.

This sounds like a GoCenter bug. In general, GOPROXY must not adjust the latest version to exclude non-matching module paths, because it is possible that those versions are intended for use only with replace directives — so you should get the same versions (and the same errors) with or without a proxy.

It's tempting for proxy owners to try to “fix” module path errors by culling out mismatched versions, but as you've seen that can lead to surprising behavior — and can break modules that use replace statements to redirect to other versions.

There is probably also a go command bug involved: if the existing version cannot be resolved, go mod tidy should not paper over the error by choosing a different version instead. That issue is already tracked in #31411.

CC @marwan-at-work @heschik @hyangah @katiehockman @jayconrod

[bcmills]

The go bug is tracked in #31411, and the likely GoCenter bug should be filed against GoCenter. I don't think there is anything remaining to do here, so closing.

[elioengcomp]

@bcmills I'm not sure if I understood what do you mean by "GOPROXY must not adjust the latest version to exclude non-matching module paths". it seems to me that what is happening is the other way around.

On GoCenter, v2.1.1 is the latest available version of github.com/pierrec/lz4. GoCenter adds the +incompatible suffix to the version to make it resolvable since the author is not following the semantic versioning best practices. On GitHub, v2.1.1 seems to be the latest tag as well. As a user, I would expect a tidy command to resolve to v2.1.1 from both sources, however, when resolving from GitHub, we are getting v2.0.5 instead.

Can you provide more details on why do you think it might be a GoCenter bug?

[elioengcomp]

Btw, the same behavior is happening for a basic go get command

With GOPROXY:

→ go get github.com/pierrec/lz4
go: finding github.com/pierrec/lz4 v2.1.1+incompatible
go: finding github.com/pkg/profile v1.2.1
go: downloading github.com/pierrec/lz4 v2.1.1+incompatible

Without GOPROXY:

→ go get github.com/pierrec/lz4
go: finding github.com/pierrec/lz4 v2.0.5+incompatible
go: downloading github.com/pierrec/lz4 v2.0.5+incompatible

[bcmills]

Hmm, that's an interesting observation. Whatever restrictions we apply to the module path found in the go.mod file, we should apply that restriction equally to code served through a VCS or through the proxy protocol.

[bcmills]

CC @jayconrod

We've been considering relaxing the module path check anyway (to support the replace use-case), but we should make sure the check is consistent between both paths either way.

[elioengcomp]

@thepudds I did the test you suggested and I got the same results I get from GoCenter. Here is what I did and the results:

→ sudo rm -rf $GOPATH

→ unset GOPROXY

→ go get github.com/pierrec/[email protected]+incompatible
go: downloading github.com/pierrec/lz4 v2.1.1+incompatible

→ go get github.com/pierrec/[email protected]+incompatible
go: finding github.com/pierrec/lz4 v2.0.5+incompatible
go: downloading github.com/pierrec/lz4 v2.0.5+incompatible

→ go get github.com/pkg/[email protected]
go: downloading github.com/pkg/profile v1.2.1

→ cp -r $GOPATH $LOCAL_GOPROXY

→ export GOPROXY=file:///$LOCAL_GOPROXY/pkg/mod/cache/download

→ sudo rm -rf $GOPATH

// Remove go.mod file requirements

→ go get github.com/pierrec/lz4
go: finding github.com/pierrec/lz4 v2.1.1+incompatible
go: finding github.com/pkg/profile v1.2.1
go: downloading github.com/pierrec/lz4 v2.1.1+incompatible

[marwan-at-work]

In Athens' case, getting the latest version is simply deferred to go list -m. Here's the result that Athens would get:

GO111MODULE=on go list -m -json -versions github.com/pierrec/lz4@latest
{
	"Path": "github.com/pierrec/lz4",
	"Version": "v2.0.5+incompatible",
	"Versions": [
		"v1.0.1",
		"v2.0.1+incompatible",
		"v2.0.2+incompatible",
		"v2.0.3+incompatible",
		"v2.0.4+incompatible",
		"v2.0.5+incompatible"
	],
	"Time": "2018-08-30T18:52:49Z",
	"Dir": "/Users/me/go/pkg/mod/github.com/pierrec/[email protected]+incompatible",
	"GoMod": "/Users/me/go/pkg/mod/cache/download/github.com/pierrec/lz4/@v/v2.0.5+incompatible.mod"
}

The Version that we'd pick is v2.0.5+incompatible which seems consistent with GOPROXY=off.

On GoCenter, v2.1.1 is the latest available version of github.com/pierrec/lz4. GoCenter adds the +incompatible suffix to the version to make it resolvable since the author is not following the semantic versioning best practices

This seems incorrect behavior to me. @bcmills should GoCenter (or any GOPROXY) be doing this? Shouldn't we just rely on go mod download and go list -m so that proxies and non-proxy are consistent across the board?

[elioengcomp]

@marwan-at-work when resolving a module in this scenario from VCS by running go get $MODULE_PATH@$MODULE_VERSION, Go will place it in the local cache under $GOPATH/pkg/mod/cache/download/$MODULE_PATH/@v/$MODULE_VERSION+incompatible.(mod|zip|ziphash). Also, if we explode the zip archive, the contents will be under $MODULE_PATH@$MODULE_VERSION+incompatible/

In order to promote compatibility between modules resolved from GoCenter and the ones resolved from VCS, we never touch those files and we use them as is.

If we try to fetch module content via GOPROXY without having the +incompatible as part of the module url path in the repository we get the following errors:

• Resolving without using the suffix:

→ go get github.com/elioengcomp/[email protected]

go: finding github.com/elioengcomp/go-module-example v2.0.0
go: downloading github.com/elioengcomp/go-module-example v2.0.0
-> unzip /Users/eliom/workspace/go-modules-example/empty-module/gopath/pkg/mod/cache/download/github.com/elioengcomp/go-module-example/@v/v2.0.0.zip: unexpected file name github.com/elioengcomp/[email protected]+incompatible/README.md
go get: no install location for directory  outside GOPATH
	For more details see: 'go help gopath'

Besides the error, the module is downloaded to the local cache, but as soon as we try to use it we get the following error:

→ go build ./...
go: errors parsing go.mod:
$MY_WORKING_PROJECT/go.mod:3: invalid module: github.com/elioengcomp/go-module-example should be v0 or v1, not v2 (v2.0.0)

If we try to fix this require statement in my working project go.mod file by adding the suffix we get the same error below.

• Resolving using the suffix:

→ go get github.com/elioengcomp/[email protected]+incompatible

go: finding github.com/elioengcomp/go-module-example v2.0.0+incompatible
go: finding github.com/elioengcomp v2.0.0+incompatible
go: finding github.com v2.0.0+incompatible
go get github.com/elioengcomp/[email protected]+incompatible: unexpected status (http://$MY_PROXY_URL/github.com/elioengcomp/go-module-example/@v/v2.0.0+incompatible.info): 404 Not Found

BTW, we get the same errors if we try to use go mod download command:

→ go mod download github.com/elioengcomp/[email protected]

go: finding github.com/elioengcomp/go-module-example v2.0.0
-> unzip /Users/eliom/workspace/go-modules-example/empty-module/gopath/pkg/mod/cache/download/github.com/elioengcomp/go-module-example/@v/v2.0.0.zip: unexpected file name github.com/elioengcomp/[email protected]+incompatible/README.md

The only way we have found to make a basic go get command to work in this scenario was by keeping the suffix to the modules url path in the repository and that is the only thing we are handling on our side. Everything else is using content generated by Go itself when it fetches content from the VCS.

[dmitshur]

This issue is currently labeled as early-in-cycle for Go 1.15. That time is now, so friendly ping. If it no longer needs to be done early in cycle, that label can be removed.

[ianlancetaylor]

This didn't happen for 1.15. Moving milestone to backlog.