Golang cmd/go: add (or make explicit) tests for ambiguous imports in module mode

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

$ go version
go version go1.11.2 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"
GOBIN=""
GOCACHE="/Users/deklerk/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/deklerk/workspace/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/lk/zs4m7sv12mq2vzk_wfn2tfvm00h16k/T/go-build259032920=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Create repo foo with:
A foo/go.mod
A foo/somefile.go containing const Version = "foo=0.0.1"
A directory foo/bar/ with a foo/bar/somefile.go containing const Version = "foo=0.0.1"
Commit, tag v0.0.1 (parent encompasses directory), push both

...

In some other library, depend on foo with require foo v0.0.1
Add fmt.Println(foo.Version) // expect "foo=0.0.1"
Add fmt.Println(bar.Version) // expect "foo=0.0.1"

...

Back in repo foo, make a foo/bar its own submodule:
Create foo/bar/go.mod
Update foo/bar/somefile.go to now have const Version = "bar=1.0.0"
Commit, tag bar/v1.0.0, tag v0.0.2 (new version of the parent because a submodule was carved out), and push all

...

In your other library, add a require bar v1.0.0 dependency (keeping the require foo v0.0.1 dependency!)
Now you require "bar" two ways: via require bar v1.0.0 and via require foo v0.0.1, in which foo/bar was still part of the module
fmt.Println(bar.Version) results in bar=1.0.0

EDIT: actually, you get the following error (see @myitcv excellent repro below):

```
unknown import path "github.com/jadekler/module-testing/pkg_b": ambiguous import: found github.com/jadekler/module-testing/pkg_b in multiple modules:
```

What did you expect to see?

I'm not sure. Maybe this is WAI? I vaguely expected to see an error. I suspect there are some hidden subtleties here that could end up breaking people. Maybe since bar has to always be backwards compatible, this is OK?

Anyways, feel free to close if this is unsurprising. I thought it was subtle and maybe worth looking at, but y'all let me know.

EDIT: Ignore this, since my original post was flawed. In fact you cannot do this, which seems to imply carving out submodules after-the-fact is unsupported.

What did you see instead?

Go modules seems to silently replace foo/bar/ in the require foo v0.0.1 with the bar v1.0.0 version.

EDIT: Ignore this, since my original post was flawed. In fact you cannot do this, which seems to imply carving out submodules after-the-fact is unsupported.

Comment From: agnivade

I don't see anything obviously wrong here. It was always part of the design to be able to import to different versions of the same package. A submodule is also just another package.

/cc @myitcv for further comments.

Comment From: myitcv

cc @bcmills too 😄

@jadekler - can you provide a bit more detail? Because I get an error as expected (the import is ambiguous):

...
$ go mod edit -require=github.com/myitcvscratch/foo/bar@v1.0.0
$ cat go.mod
module mod

require (
        github.com/myitcvscratch/foo v0.0.1
        github.com/myitcvscratch/foo/bar v1.0.0
)
$ go run .
go: finding github.com/myitcvscratch/foo/bar v1.0.0
go: downloading github.com/myitcvscratch/foo/bar v1.0.0
main.go:6:2: unknown import path "github.com/myitcvscratch/foo/bar": ambiguous import: found github.com/myitcvscratch/foo/bar in multiple modules:
        github.com/myitcvscratch/foo v0.0.1 (/home/gopher/pkg/mod/github.com/myitcvscratch/foo@v0.0.1/bar)
        github.com/myitcvscratch/foo/bar v1.0.0 (/home/gopher/pkg/mod/github.com/myitcvscratch/foo/bar@v1.0.0)

Full repro


$ cd /tmp
$ mkdir foo usefoobar
$ cd foo
$ git init
Initialized empty Git repository in /tmp/foo/.git/
$ git remote add origin https://github.com/myitcvscratch/foo
$ go mod init
go: creating new go.mod: module github.com/myitcvscratch/foo
$ mkdir bar
$ cat <foo.go
package foo

const Version = "foo v0.0.1"
EOD
$ cat <bar/bar.go
package bar

const Version = "foo v0.0.1"
EOD
$ git add -A
$ git commit -q -am 'Initial commit'
$ git push -q
remote:
remote: Create a pull request for 'master' on GitHub by visiting:
remote:      https://github.com/myitcvscratch/foo/pull/new/master
remote:
$ git tag v0.0.1
$ git push -q origin v0.0.1
$ cd /tmp/usefoobar
$ go mod init mod
go: creating new go.mod: module mod
$ cat <main.go
package main

import (
        "fmt"
        "github.com/myitcvscratch/foo"
        "github.com/myitcvscratch/foo/bar"
)

func main() {
        fmt.Printf("foo: %v\n", foo.Version)
        fmt.Printf("bar: %v\n", bar.Version)
}
EOD
$ go run .
go: finding github.com/myitcvscratch/foo/bar latest
go: finding github.com/myitcvscratch/foo v0.0.1
go: downloading github.com/myitcvscratch/foo v0.0.1
foo: foo v0.0.1
bar: foo v0.0.1
$ cd /tmp/foo/bar
$ go mod init github.com/myitcvscratch/foo/bar
go: creating new go.mod: module github.com/myitcvscratch/foo/bar
$ cat <bar.go
package bar

const Version = "var v1.0.0"
EOD
$ cd ..
$ cat <foo.go
package foo

const Version = "foo v0.0.2"
EOD
$ git add -A
$ git commit -q -am 'Second commit'
$ git tag v0.0.2
$ git tag bar/v1.0.0
$ git push -q origin v0.0.2
$ git push -q origin bar/v1.0.0
$ cd /tmp/usefoobar
$ go mod edit -require=github.com/myitcvscratch/foo/bar@v1.0.0
$ cat go.mod
module mod

require (
        github.com/myitcvscratch/foo v0.0.1
        github.com/myitcvscratch/foo/bar v1.0.0
)
$ go run .
go: finding github.com/myitcvscratch/foo/bar v1.0.0
go: downloading github.com/myitcvscratch/foo/bar v1.0.0
main.go:6:2: unknown import path "github.com/myitcvscratch/foo/bar": ambiguous import: found github.com/myitcvscratch/foo/bar in multiple modules:
        github.com/myitcvscratch/foo v0.0.1 (/home/gopher/pkg/mod/github.com/myitcvscratch/foo@v0.0.1/bar)
        github.com/myitcvscratch/foo/bar v1.0.0 (/home/gopher/pkg/mod/github.com/myitcvscratch/foo/bar@v1.0.0)

But if I then require v0.0.2 of foo all works as expected:

$ go mod edit -require=github.com/myitcvscratch/foo@v0.0.2
$ cat go.mod
module mod

require (
        github.com/myitcvscratch/foo v0.0.2
        github.com/myitcvscratch/foo/bar v1.0.0
)
$ go run .
go: finding github.com/myitcvscratch/foo v0.0.2
go: downloading github.com/myitcvscratch/foo v0.0.2
foo: foo v0.0.2
bar: var v1.0.0

In case it's useful, I use the very hacky egrunner to quickly pull together these repros

Comment From: jeanbza

Ahhh. Thank you @myitcv, in my repro I had incorrectly depended upon too late of a version. When I bump it back down to v0.0.1, I see the same failure. Thanks for the confirmation.

Before I make changes to this issue, let me ask y'all this: this result seems to indicate that it is subtly unsafe to carve out a submodule after-the-fact if anyone depends on your package. Do you agree? In which case I'd like to suggest that the documentation makes this clear.

cc @hyangah @bradfitz re: x/tools, x/oauth2 (maybe y'all have already discovered and discussed this)

Comment From: myitcv

What do you mean by "unsafe", exactly?

I think the situation we've reproduced here is unfortunate (and could perhaps be better flagged/resolved), but I don't think it's unsafe in so far as we end up with a bad build.

Come and join the party over in https://github.com/golang/go/issues/27858#issuecomment-425089484 (linking to @bcmills's thoughts on this specifically) for more discussion on this!

Comment From: jeanbza

Unsafe as in, users' programs may break in the "ambiguous import" manner seen above. That is, you probably need to do major bumping when you carve out a submodule, instead of just a minor bump.

Subtle as in, this is hard to see as a library maintainer without setting up a fairly convoluted check.

SG, will catch up on that discussion and see if I have anything of worth to contribute heh. :)

Comment From: myitcv

Unsafe as in, users' programs may break in the "ambiguous import" manner seen above. That is, you probably need to do major bumping when you carve out a submodule, instead of just a minor bump.

Got it. Just one small nit, the program itself doesn't break, the build does. And the build only breaks in response to us doing something, i.e. go mod edit -require=github.com/myitcvscratch/foo/bar@v1.0.0 (or equivalent). It's conceivable that in such a situation we can do something to try and automatically resolve the situation, thereby side-stepping the error altogether, and hence it's no longer a subtle problem. But that's just a knee-jerk response. @bcmills has given this much more thought.

Comment From: jeanbza

Turns out you can do this in a really funny way. Per https://github.com/golang/go/issues/27858#issuecomment-425516102, you can make the carved-out submodule require the first version of the parent module that does not contain its contents. Although, I don't think you can do this atomically, since the "carve out commit" must be tagged parent=vNEXT as well as child=vNEXT, and in that same commit you need to have the require statement for parent=vNEXT.

Comment From: bcmills

You can do it atomically (by tagging the parent and child vNEXT in the same commit), but you can't test it before you push (because you can't currently resolve unpublished module versions). 😕

Comment From: jeanbza

Ohhh right. I had incorrectly thought there wouldn't be a sha until you push. But, that's obviously wrong.

Comment From: bcmills

in my repro I had incorrectly depended upon too late of a version.

I had thought we had a test that confirmed the ambiguous import error, but git grep ambiguous isn't turning anything up. Since this does seem to be working as intended, I'll retitle the bug to be about making sure it keeps working.

Comment From: hyangah

You can do it atomically (by tagging the parent and child vNEXT in the same commit), but you can't test it before you push (because you can't currently resolve unpublished module versions).

In addition to this, I observed we can't compute the correct go.sum entries. It doesn't cause build failure but verification is skipped.

Comment From: jeanbza

@hyangah Maybe my git noob-ness, but: does the commit sha change when you push? In https://github.com/golang/go/issues/28806#issuecomment-439105041 I had naively thought the local sha would be OK to manually use.

edit: Whoops, ignore this. I just had the realization that go.sum is not comprised of shas.

Comment From: bcmills

we can't compute the correct go.sum entries.

That's true, although omitting a go.sum entry is fairly benign. (Certainly not ideal, but not actively harmful either.)

Comment From: gopherbot

Change https://golang.org/cl/255046 mentions this issue: cmd/go/internal/modload: avoid a network fetch when querying a valid semantic version

Comment From: jeanbza

This is probably stale. Closing.

Golang cmd/go: add (or make explicit) tests for ambiguous imports in module mode

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

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

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