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: module loader looks for the wrong tags if the "go-import" prefix includes the major-version suffix #30647

Closed
gdamore opened this issue Mar 6, 2019 · 17 comments

Comments

Projects
None yet
4 participants
@gdamore
Copy link

commented Mar 6, 2019

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

$ go version
1.11.4 windows/amd64
1.11.1 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

Windows, Linux, 64-bit

go env Output
$ go env

PS C:\Users\garre\go> go env
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\garre\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\garre\go
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\garre\AppData\Local\Temp\go-build888955782=/tmp/go-build -gno-record-gcc-switches

What did you do?

The easiest example is Russ Cox's rsc.io/quote package.

cd ~/go/
go get rsc.io/quote/v2

package rsc.io/quote/v2: cannot find package "rsc.io/quote/v2" in any of:
        C:\Go\src\rsc.io\quote\v2 (from $GOROOT)
        C:\Users\garre\go\src\rsc.io\quote\v2 (from $GOPATH)

Also with v3, different error:

go get rsc.io/quote/v3

can't load package: package rsc.io/quote/v3: code in directory C:\Users\garre\go\src\rsc.io\quote\v3 expects import "rsc.io/quote"

I'm trying to enable a package (nanomsg.org/go/mangos/v2 - with numerous sub package) for both modules and legacy mode. When I followed the example of rsc.io/quote for how to do this, I wound up breaking all my legacy users.

It might be possible to use vanity import paths, v2+ modules, in a way that works with both GO111MODULES=on and GO111MODULES=off, but after numerous days of effort trying to make this work, I am not convinced that it actually is possible. If it is possible, a working example would be helpful. If it isn't, it would be good to fix it, because this represents huge friction in adoption of go modules for me and my downstreams.

What did you expect to see?

go get works in $GOPATH

What did you see instead?

See above errors, go get fails differently, depending on whether Major Branches (my preferred approach!) or Major directories are used.

Note that things do work fine with GO111MODULES=on (or auto if not in $GOPATH).

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

rsc.io/quote is not at all a good example for GOPATH compatibility. That module exists to illustrate module-mode semantics, and by design, module-mode includes behaviors that GOPATH mode cannot replicate (such as using modules from two different versions of a repository rooted at the same path).

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

The issue template asks, “What did you do?” — please describe the concrete problem that you're trying to solve, and what you've actually tried so far. (Presumably you are not trying to use rsc.io/quote in production code.)

In particular, the “major subdirectory” convention described in https://research.swtch.com/vgo-module should work in both module mode and GOPATH mode without any modification of existing <meta> tags, so if that wasn't working for you it would be helpful to know exactly how you had things set up.

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

I note the following behavior from 'https://nanomsg.org` today:

$ curl -sL https://nanomsg.org/go/mangos?go-get=1 | grep 'go-import'
<meta name="go-import" content="nanomsg.org/go/mangos git https://github.com/nanomsg/mangos">

$ curl -sL https://nanomsg.org/go/mangos/v2?go-get=1 | grep 'go-import'
<meta name="go-import" content="nanomsg.org/go/mangos/v2 git https://github.com/nanomsg/mangos-v2">

The result of that second query seems to indicate that you're pointing the /v2 path at a different repository from the non-/v2 path, although it seems to redirect back to the same repository. That's a bit surprising, but I suppose it should be ok.

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

Actually, I think that second tag is the problem. I suspect that the go command is expecting content="nanomsg.org/go/mangos […]" rather than content="nanomsg.org/go/mangos/v2 […]".

example.com$ go1.12 get nanomsg.org/go/mangos/v2@v2.0.2
go: finding nanomsg.org/go/mangos/v2 v2.0.2
go: finding nanomsg.org/go/mangos v2.0.2
go: downloading nanomsg.org/go/mangos v0.0.0-20181201150740-63f66a65137b
go get nanomsg.org/go/mangos/v2@v2.0.2: go.mod has post-v0 module path "nanomsg.org/go/mangos/v2" at revision 63f66a65137b

@bcmills bcmills self-assigned this Mar 7, 2019

@bcmills bcmills added this to the Go1.13 milestone Mar 7, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

example.com$ go1.12 get -m nanomsg.org/go/mangos/v2@v2.0.2
go: finding nanomsg.org/go/mangos/v2 v2.0.2
go get nanomsg.org/go/mangos/v2@v2.0.2: unknown revision nanomsg.org/go/mangos/v2.0.2

That error message comes from here:

return nil, fmt.Errorf("unknown revision %s", rev)

That seems to imply that, for some reason, we're looking for a tag with the wrong prefix.

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

There seem to be a couple of issues here:

codeRev := r.revToRev(rev)
if semver.IsValid(codeRev) && r.codeDir != "" {
codeRev = r.codeDir + "/" + codeRev
}

First, revToRev has already expanded r.codeDir here, so we probably shouldn't expand it again:

return r.codeDir + "/" + rev

Second, the computed r.codeDir for that repository looks just plain wrong: it's the full module path, when it should be empty. Still trying to figure out why.

@bcmills bcmills changed the title vanity import, semver, and go get incompatible cmd/go: module loader looks for the wrong tags if the "go-import" prefix includes the major-version suffix Mar 7, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

As a workaround, you can probably remove the /v2 suffix from the go-import tag, and rely on the “minimal module support” fallback code to resolve the /v2 suffix in GOPATH mode.

@gdamore

This comment has been minimized.

Copy link
Author

commented Mar 7, 2019

So the reason I called out rsc.io/quote was that this was used as an example to me. It would be helpful to have a working example. I want to use branch mode. The reason the repos for v2 are different is that I split for semver reasons before modules were a thing. I can merge back to one repo, once I'm certain that it works. The other thing is that v1 users have a totally different import path, not just a suffix that's different.

I had originally planned to do a v3 version update, which would support both modules and go get, and could have a revised import path, with minimal disruptions ideally.

It would still be helpful to me to have a demonstration repo (including vanity support) that showed both module and go get support combined with a v2+ semver, ideally using branching rather that directories to distinguish major versions.

@thepudds

This comment has been minimized.

Copy link

commented Mar 7, 2019

It would still be helpful to me to have a demonstration repo (including vanity support) that showed both module and go get support combined with a v2+ semver

I think you might be asking for an example that works with a module consumer and non-module consumer? (Modules use go get).

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

I have some good news. I have a proof-of-concept fix that successfully fetches nanomsg.org/go/mangos/v2 v2.0.2 using its existing go-import tags, and if it is correct I think it will be small enough to backport to Go 1.12.1.

@gdamore

This comment has been minimized.

Copy link
Author

commented Mar 7, 2019

This would be excellent news!

@gopherbot

This comment has been minimized.

Copy link

commented Mar 7, 2019

Change https://golang.org/cl/166117 mentions this issue: cmd/go/internal/modfetch: handle codeRoot == path for paths with major-version suffixes

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

@gopherbot, please backport to 1.12: this fixes a regression from GOPATH mode and the patch is small.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 7, 2019

Backport issue(s) opened: #30665 (for 1.12).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gdamore

This comment has been minimized.

Copy link
Author

commented Mar 7, 2019

It would still be helpful to me to have a demonstration repo (including vanity support) that showed both module and go get support combined with a v2+ semver

I think you might be asking for an example that works with a module consumer and non-module consumer? (Modules use go get).

Correct. I'm not sure how we refer to "non modules"... ;-p

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 8, 2019

I'm not sure how we refer to "non modules"...

“GOPATH mode”.

@gopherbot gopherbot closed this in 1ab9f68 Mar 8, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Mar 8, 2019

It would be helpful to have a working example.

As of CL 166117, nanomsg.org/go/mangos/v2 is now a working example.

I tried to think of what sorts of examples we needed to illustrate, but there really isn't that much to it: a module with path example.com/foo/v2 should work with any of:

  • prefix example.com with tag foo/v2.X.Y and a file at either ./foo/go.mod or ./foo/v2/go.mod,
  • prefix example.com/foo with tag v2.X.Y and a file at either ./go.mod or ./v2/go.mod,
  • or (fixed by CL 166117), prefix example.com/foo/v2 with tag v2.X.Y and a file at ./go.mod.

The bug affected the last of those cases in module mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.