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: backported module awareness doesn't support list or go/build #25662

Closed
myitcv opened this issue May 31, 2018 · 7 comments
Closed

cmd/go: backported module awareness doesn't support list or go/build #25662

myitcv opened this issue May 31, 2018 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented May 31, 2018

Please answer these questions before submitting your issue. Thanks!

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

28ae82663a1c57c185312b60a2eae8cf06cc24b4

which is on the release-branch.go1.10 branch, and includes https://go-review.googlesource.com/c/go/+/114500.

Does this issue reproduce with the latest release?

n/a

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/tmp/tmp.xwRnLArihb"
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build189190539=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Use the Go 1.10 backport branch, specifically:

$ cd /tmp
$ git clone -q https://github.com/golang/go
$ cd go/src
$ git checkout -q 28ae82663a1c57c185312b60a2eae8cf06cc24b4
$ ./make.bash
Building Go cmd/dist using /usr/local/go.
Building Go toolchain1 using /usr/local/go.
Building Go bootstrap cmd/go (go_bootstrap) using Go toolchain1.
Building Go toolchain2 using go_bootstrap and Go toolchain1.
Building Go toolchain3 using go_bootstrap and Go toolchain2.
Building packages and commands for linux/amd64.
---
Installed Go for linux/amd64 in /tmp/go
Installed commands in /tmp/go/bin
$ export PATH=/tmp/go/bin:$PATH
$ which go
/tmp/go/bin/go
$ go version
go version go1.10.2 linux/amd64

Get vgo:

$ go get -u golang.org/x/vgo

Create a simple module that depends on v2 of a package, and build:

$ cd $(mktemp -d)
$ export GOPATH=$PWD
$ mkdir -p src/example.com/hello
$ cd src/example.com/hello
$ cat <<EOD >hello.go
package main // import "example.com/hello"

import (
	"fmt"
	"github.com/myitcv/vgo_example_compat/v2"
	"github.com/myitcv/vgo_example_compat/v2/sub"
)

func main() {
	fmt.Println(vgo_example_compat.X, sub.Y)
}
EOD
$ echo >go.mod
$ vgo build
vgo: resolving import "github.com/myitcv/vgo_example_compat/v2"
vgo: finding github.com/myitcv/vgo_example_compat/v2 (latest)
vgo: adding github.com/myitcv/vgo_example_compat/v2 v2.0.0
vgo: finding github.com/myitcv/vgo_example_compat/v2 v2.0.0
vgo: downloading github.com/myitcv/vgo_example_compat/v2 v2.0.0
$ ./hello
2 2

Now try to go get v2:

$ go get github.com/myitcv/vgo_example_compat/v2
package github.com/myitcv/vgo_example_compat/v2: cannot find package "github.com/myitcv/vgo_example_compat/v2" in any of:
	/tmp/go/src/github.com/myitcv/vgo_example_compat/v2 (from $GOROOT)
	/tmp/tmp.uBC7919n7v/src/github.com/myitcv/vgo_example_compat/v2 (from $GOPATH)

Note that this works (unsuprisingly):

$ go get github.com/myitcv/vgo_example_compat

Now try and build:

$ go build
$ ./hello
2 2

Now try and list v2 of the package on which we depend:

$ go list github.com/myitcv/vgo_example_compat/v2
can't load package: package github.com/myitcv/vgo_example_compat/v2: cannot find package "github.com/myitcv/vgo_example_compat/v2" in any of:
	/tmp/go/src/github.com/myitcv/vgo_example_compat/v2 (from $GOROOT)
	/tmp/tmp.uBC7919n7v/src/github.com/myitcv/vgo_example_compat/v2 (from $GOPATH)

Note that this works:

$ go list github.com/myitcv/vgo_example_compat
github.com/myitcv/vgo_example_compat

Now verify how go/build behaves with these imports paths:

$ cat <<EOD >run.go
// +build ignore

package main

import (
	"fmt"
	"os"
	"go/build"
)

func main() {
	bpkg, err := build.Import(os.Args[1], ".", 0)
	if err != nil {
		panic(err)
	}
	fmt.Printf("%v\n", bpkg.Dir)
}
EOD

Run for the v2 import path:

$ go run run.go github.com/myitcv/vgo_example_compat/v2
panic: cannot find package "github.com/myitcv/vgo_example_compat/v2" in any of:
	/tmp/go/src/github.com/myitcv/vgo_example_compat/v2 (from $GOROOT)
	/tmp/tmp.uBC7919n7v/src/github.com/myitcv/vgo_example_compat/v2 (from $GOPATH)

goroutine 1 [running]:
main.main()
	/tmp/tmp.uBC7919n7v/src/example.com/hello/run.go:14 +0x10a
exit status 2

Note that this works:

$ go run run.go github.com/myitcv/vgo_example_compat
/tmp/tmp.uBC7919n7v/src/github.com/myitcv/vgo_example_compat

What did you expect to see?

I'm unclear whether we should/would expect go list etc to work with the backported changes for */v2/* import paths?

Also I note that go/build could/should have these resolution powers too?

What did you see instead?

The Go 1.10 backported changes not working for go list or via use of go/build for */v2/* import paths.

/cc @rsc

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 31, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.10.3 milestone May 31, 2018
@ianlancetaylor
Copy link
Contributor

CC @andybons This is a potential blocker for a 1.10.3 release.

@andybons
Copy link
Member

andybons commented Jun 1, 2018

I’m guessing this will also require a backport to 1.9 as well.

@myitcv
Copy link
Member Author

myitcv commented Jun 1, 2018

I’m guessing this will also require a backport to 1.9 as well.

If this issue is confirmed as indeed an issue (i.e. assumptions/expectations aren't off) then yes, I suspect so.

/cc @bcmills too

@rsc
Copy link
Contributor

rsc commented Jun 1, 2018

Thanks very much for trying these out Paul. I really appreciate the scrutiny.

While I admit the behavior here is a little difficult to understand, it's working as intended. The key point about the change for #25069 is that it only applies to new (go.mod-containing) code that is itself imported from new (go.mod-containing) code. It is intentional that if you are compiling code that is outside a go.mod tree, the correct import path is still github.com/myitcv/vgo_example_compat. Only inside a go.mod source tree, an import github.com/myitcv/vgo_example_compat/v2 is read as if it really said github.com/myitcv/vgo_example_compat. For example I just created github.com/rsc/vgotest4 and github.com/vgotest5, which both import your code. vgotest4 has no go.mod so it imports as github.com/myitcv/vgo_example_compat, while vgotest5 does have a go.mod, so it imports as github.com/myitcv/vgo_example_compat/v2.

The other key part is the "is read as if it really said". Just like with vendor, on the go command line you have to use the "real" import paths not the rewritten ones that might appear in your code. For example if you have a GOPATH in which x/y/vendor/z/w exists and you are in the x/y directory, "go list z/w" may well assuming, since GOPATH/src/z/w does not also exist. What you have to say on the command line is the "real" path, x/y/vendor/z/w, even though x/y/y.go might say import "z/w". That import is read as if it really said x/y/vendor/z/w, per golang.org/s/go15vendor.

The same thing is happening here: there is a by-design disconnect between what's in the import paths and what you have to say to the go command. The new v2-containing path is (in the old legacy go command) this one special case essentially limited to the import reader in module-aware source files, not something you can use more generally.

go get github.com/rsc/vgotest4
go get github.com/rsc/vgotest5
go1.10 get github.com/rsc/vgotest4
go1.10 get github.com/rsc/vgotest5
go1.9 get github.com/rsc/vgotest4

all work for me (my go1.10 has the release branch CLs). What doesn't work for me is

go1.9 get github.com/rsc/vgotest5

I have to run out now but I will look into that in a few hours. Filed #25687 for that.

@rsc rsc closed this as completed Jun 1, 2018
@myitcv
Copy link
Member Author

myitcv commented Jun 1, 2018

While I admit the behavior here is a little difficult to understand, it's working as intended.

Thanks, Russ; totally makes sense now. Really appreciate the full explanation.

Regarding the go/build example; am I right in concluding therefore that build.Import will never work with /vX in the import paths? Either with the soon-to-be-released Go 1.9, 1.10 or indeed 1.11?

Do we therefore, with the release of Go 1.11, gain an golang.org/x/**/vgobuild (or similar, vgo equivalent of go/build) package to deal with this new world of modules, that is /vX aware?

@rsc
Copy link
Contributor

rsc commented Jun 4, 2018

Yes, go/build's build.Import does not handle vgo. Nor does it handle some aspects of build caching. We are trying to get a replacement package, tentatively named golang.org/x/tools/go/packages, out soon.

@myitcv
Copy link
Member Author

myitcv commented Jun 4, 2018

Ah great, so this is indeed part of the work Alan mentioned a few weeks ago. Thanks for confirming.

@golang golang locked and limited conversation to collaborators Jun 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants