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 -m' fails to follow dependencies through older versions of the main module #29773

Closed
myitcv opened this issue Jan 16, 2019 · 11 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Jan 16, 2019

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

$ go version
go version go1.12beta2 linux/amd64

Does this issue reproduce with the latest release?

Testing with 1.12 pre the release.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPROXY=""
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/tmp.Z9hWoyAQkk/x/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build313729444=/tmp/go-build -gno-record-gcc-switches"

What did you do?

cd $(mktemp -d)
git clone https://github.com/myitcv/x
cd x
git checkout da37b5e
go mod tidy
git status --porcelain
go list honnef.co/go/js/util

This gives the output:

honnef.co/go/js/util

as expected.

However if we then:

cat <<EOD >> tools.go
import _ "github.com/myitcv/gobin"
EOD
go mod tidy

followed by:

go list honnef.co/go/js/util
go list honnef.co/go/js/util
go list honnef.co/go/js/util
...

any number of times, the output is always:

go: finding honnef.co/go/js/util latest
honnef.co/go/js/util

This despite the requirement being part of the requirements graph.

$ go mod graph | grep util
myitcv.io@v0.0.0-20181102104800-990e6f9524a1 honnef.co/go/js/util@v0.0.0-20150216223935-96b8dd9d1621

What did you expect to see?

Not to see see the "finding" message.

What did you see instead?

The "finding" message as part of each go list.

Could this be something to do with the now circular module requirement myitcv.io -> github.com/myitcv/gobin -> myitcv.io? And the fact:

$ go mod why honnef.co/go/js/util
myitcv.io/react/examples
honnef.co/go/js/xhr
honnef.co/go/js/util

where both the honnef.co requirements are not actual modules?

The addition of the the requirement on github.com/myitcv/gobin has removed the indirect honnef.co/go/js/util from myitcv.io's go.mod, but I think this is expected.

cc @bcmills. I note #27123 which links to #27102 and #27063; I don't think this is precisely an instance of any of those.

Apologies; I don't think this can be reproduced in a testscript test because this appears to rely on the remote import path resolution code path.

cc @rogpeppe as an FYI given his report in #27123

@bcmills

This comment has been hidden.

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 23, 2019

And go mod download -json exits with success without actually printing anything:

x$ go mod download -json honnef.co/go/js/util

x$

@bcmills

This comment has been hidden.

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 23, 2019

I think that the problem here is a circular dependency via an older version of the main module. Probably has something to do with a hard-coded bypass for loading the target module. Minimal example forthcoming.

@bcmills

This comment has been hidden.

@bcmills bcmills changed the title cmd/go: circular module requirement means non-module requirements are never satisfied cmd/go: 'go list -m' fails to follow dependencies through older versions of the main module Jan 24, 2019
@bcmills bcmills added this to the Go1.13 milestone Jan 24, 2019
@bcmills

This comment has been hidden.

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 24, 2019

Found a simpler repro.

@bcmills
Copy link
Member

@bcmills bcmills commented Jan 24, 2019

root$ mkdir -p ./mirror-v0.1.0 ./pkg-v0.1.0 ./root-v0.1.0

root$ cat >./go.mod <<EOF
module golang.org/issue/root

go 1.12

replace (
        golang.org/issue/mirror v0.1.0 => ./mirror-v0.1.0
        golang.org/issue/pkg v0.1.0 => ./pkg-v0.1.0
        golang.org/issue/root v0.1.0 => ./root-v0.1.0
)

require golang.org/issue/mirror v0.1.0
EOF

root$ cat >./root.go <<EOF
package root

import _ "golang.org/issue/mirror"
EOF

root$ cat >./mirror-v0.1.0/go.mod <<EOF
module golang.org/issue/mirror

require golang.org/issue/root v0.1.0
EOF

root$ cat >./mirror-v0.1.0/mirror.go <<EOF
package mirror

import _ "golang.org/issue/pkg"
EOF

root$ cat >./pkg-v0.1.0/go.mod <<EOF
module golang.org/issue/pkg
EOF

root$ cat >./pkg-v0.1.0/pkg.go <<EOF
package pkg
EOF

root$ cat >./root-v0.1.0/go.mod <<EOF
module golang.org/issue/root

require golang.org/issue/pkg v0.1.0
EOF

root$ go list -f '{{with .Module}}{{.Path}} {{.Version}}{{end}}' all
golang.org/issue/mirror v0.1.0
golang.org/issue/pkg v0.1.0
golang.org/issue/root

root$ go list -m all
golang.org/issue/root
golang.org/issue/mirror v0.1.0 => ./mirror-v0.1.0

root$ go mod graph
golang.org/issue/root golang.org/issue/mirror@v0.1.0
golang.org/issue/mirror@v0.1.0 golang.org/issue/root@v0.1.0
golang.org/issue/root@v0.1.0 golang.org/issue/pkg@v0.1.0

myitcv added a commit to myitcv/gobin that referenced this issue Jan 25, 2019
Because of golang/go#29773, circular module
requirements create a problem with Go 1.11 and 1.12beta2 (at least).

The fix for this is scheduled for Go 1.13, but we need to add a
dependency from myitcv.io -> github.com/myitcv/gobin now.

Given that the dependency on myitcv.io is only a development-time tool
dependency, the easiest thing to do for now is to drop that requirement,
thereby breaking the cycle.
@myitcv
Copy link
Member Author

@myitcv myitcv commented Jan 25, 2019

@bcmills - thanks very much for taking a look at this.

Just an FYI - it looks from the above like you have a standalone repro, correct? Also that this is going to be fixed in the 1.13 cycle?

If so, I'm going to break the cycle in this instance. The requirement github.com/myitcv/gobin -> myitcv.io is a development-time tool dependency... and not a critical one at that so I can live with this for now.

Just wanted to flag that I'll be doing that in case you're attempting any repros against master on either.

myitcv added a commit to myitcv/gobin that referenced this issue Jan 25, 2019
Because of golang/go#29773, circular module
requirements create a problem with Go 1.11 and 1.12beta2 (at least).

The fix for this is scheduled for Go 1.13, but we need to add a
dependency from myitcv.io -> github.com/myitcv/gobin now.

Given that the dependency on myitcv.io is only a development-time tool
dependency, the easiest thing to do for now is to drop that requirement,
thereby breaking the cycle.
@bcmills
Copy link
Member

@bcmills bcmills commented Jan 25, 2019

Yes, I have a standalone repro and I plan to have it fixed in 1.13. (I'm hoping that the fix will be straightforward, but as the Google SREs say, “hope is not a strategy”.)

Took me a while to distill it out, but the steps you gave did reproduce the problem exactly as described. Thanks for the detailed report!

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 17, 2019

Change https://golang.org/cl/186537 mentions this issue: cmd/go/internal/mvs: retain modules required by older versions

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

Successfully merging a pull request may close this issue.

None yet
6 participants