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

x/vgo: unable to resolve submodules for custom import paths #24687

Closed
myitcv opened this issue Apr 4, 2018 · 11 comments
Closed

x/vgo: unable to resolve submodules for custom import paths #24687

myitcv opened this issue Apr 4, 2018 · 11 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented Apr 4, 2018

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

go version go1.10.1 linux/amd64
go version go1.10.1 linux/amd64 vgo:2018-02-20.1

Specifically vgo is at 890b798475a0fc2108fa88d9b2810d5f768f5752.

Does this issue reproduce with the latest release?

Yes, per above.

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.2LGDYmXt1s"
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build814184507=/tmp/go-build -gno-record-gcc-switches"
VGOMODROOT=""

What did you do?

See updated details in #24687 (comment)

I am in the process of trying to migrate myitcv.io/... packages to be vgo-compatible. In the process I want to move away from each having its own repository to use a single repo: https://github.com/myitcv/x.

I have started with myitcv.io/gogenerate. I believe I have the various go-import meta tags set correctly; indeed old go get continues to work:

cd `mktemp -d`
export GOPATH=$PWD
go get myitcv.io/gogenerate
go test myitcv.io/gogenerate

gives:

ok      myitcv.io/gogenerate    0.002s

But vgo get does not:

cd `mktemp -d`
export GOPATH=$PWD
mkdir blah
cd blah/
echo 'module "rubbish.com/blah"' >> go.mod
echo 'package blah' >> blah.go

This vgo get:

vgo get rsc.io/pdf

works as expected:

vgo: finding rsc.io/pdf v0.0.0-20180112171046-225057252246
vgo: downloading rsc.io/pdf v0.0.0-20180112171046-225057252246

But if I now do:

vgo get myitcv.io/gogenerate

I get an error:

vgo get myitcv.io/gogenerate: module root is "myitcv.io"

What did you expect to see?

See updated details in #24687 (comment)

vgo successfully downloading myitcv.io/gogenerate which is a module within the mono-repo at https://github.com/myitcv/x.

Or am I misunderstanding something here within respect to multiple modules in a single repo?

What did you see instead?

Per above.

@gopherbot gopherbot added this to the vgo milestone Apr 4, 2018
@myitcv
Copy link
Member Author

myitcv commented Apr 4, 2018

cc @rsc (just in case)

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 4, 2018
@jeffallen
Copy link
Contributor

jeffallen commented Apr 5, 2018

vgo is correctly fetching the meta-data from your website. It then ends up in this code from domain.go:

	// First look for new module definition.
	for _, imp := range imports {
		println("checking imp", imp.Prefix, imp.VCS, imp.RepoRoot)
                // OUTPUT: checking imp myitcv.io/gogenerate git https://github.com/myitcv/x
		if path == imp.Prefix || strings.HasPrefix(path, imp.Prefix+"/") {
			if imp.VCS == "mod" {
				u, err := url.Parse(imp.RepoRoot)
				if err != nil {
					return nil, fmt.Errorf("invalid module URL %q", imp.RepoRoot)
				} else if u.Scheme != "https" {
					// TODO: Allow -insecure flag as a build flag?
					return nil, fmt.Errorf("invalid module URL %q: must be HTTPS", imp.RepoRoot)
				}
				return newProxyRepo(imp.RepoRoot, imp.Prefix), nil
			}
		}
	}

At this point, your meta-data has VCS set to "git". This is correct, according to "go help importpath".

However, vgo is looking for a VCS of "mod". I don't know why, that's not documented in the importpath help message.

Could you try changing your VCS type to "mod" and we'll see if we can get vgo to go into that "newProxyRepo" call?

@jeffallen
Copy link
Contributor

One time, inexplicably, I got this:

$ vgo get myitcv.io/gogenerate
checking imp myitcv.io/gogenerate git https://github.com/myitcv/x
vgo: finding myitcv.io/gogenerate v0.0.0-20180404133421-640b382e1fbd
checking imp myitcv.io/gogenerate git https://github.com/myitcv/x
panic: mistake

goroutine 1 [running]:
golang.org/x/vgo/vendor/cmd/go/internal/mvs.buildList(0xc420210151, 0x1f, 0x0, 0x0, 0x159a840, 0xc4201f8d20, 0x0, 0xc4201d7620, 0x10129f8, 0x50, ...)
	/Users/jallen/go/src/golang.org/x/vgo/vendor/cmd/go/internal/mvs/mvs.go:80 +0x101c
golang.org/x/vgo/vendor/cmd/go/internal/mvs.BuildList(0xc420210151, 0x1f, 0x0, 0x0, 0x159a840, 0xc4201f8d20, 0x1, 0x1, 0x0, 0x0, ...)
	/Users/jallen/go/src/golang.org/x/vgo/vendor/cmd/go/internal/mvs/mvs.go:33 +0x78
golang.org/x/vgo/vendor/cmd/go/internal/mvs.Upgrade(0xc420210151, 0x1f, 0x0, 0x0, 0x159a880, 0xc420392820, 0xc420392800, 0x1, 0x1, 0x1d5c6e8, ...)
	/Users/jallen/go/src/golang.org/x/vgo/vendor/cmd/go/internal/mvs/mvs.go:209 +0x262
golang.org/x/vgo/vendor/cmd/go/internal/vgo.runGet(0x17b7a60, 0xc4200a00b0, 0x1, 0x1)
	/Users/jallen/go/src/golang.org/x/vgo/vendor/cmd/go/internal/vgo/get.go:102 +0x829
golang.org/x/vgo/vendor/cmd/go.Main()
	/Users/jallen/go/src/golang.org/x/vgo/vendor/cmd/go/main.go:155 +0x82e
main.main()
	/Users/jallen/go/src/golang.org/x/vgo/main.go:37 +0x25

I cannot reproduce it.

@myitcv
Copy link
Member Author

myitcv commented Apr 5, 2018

Thanks for helping to look into this @jeffallen

One time, inexplicably, I got this:

Apologies, you caught me mid-test of a change to the go-import meta tags. Now reverted so you should see the original error as reported in my description.

Could you try changing your VCS type to "mod" and we'll see if we can get vgo to go into that "newProxyRepo" call?

I can't do that if I want to retain backwards compatibility with regular go users (added steps in my original post that confirm go get still works)

In any case, my understanding was that the mod "VCS" in a go-import meta tag is used for published modules (and that further more it can be specific in addition to the git or otherwise meta tag because there is fallback logic). i.e. the server that responds to mod needs to understand the download protocol: in this situation I'm not doing that, I'm serving source from Github.

@myitcv
Copy link
Member Author

myitcv commented Apr 5, 2018

It appears there is actually a test for this exact case (but golang.org/x/net/context is used instead of myitcv.io/gogenerate). In this test the response is:

GET https://golang.org/x/net?go-get=1
200 OK
Alt-Svc: hq=":443"; ma=2592000; quic=51303431; quic=51303339; quic=51303338; quic=51303337; quic=51303335,quic=":443"; ma=2592000; v="41,39,38,37,35"
Cache-Control: private
Content-Type: text/html; charset=utf-8
Date: Tue, 30 Jan 2018 03:40:58 GMT
Server: Google Frontend
Vary: Accept-Encoding
X-Cloud-Trace-Context: fd9216519bb8006b586b7fdda695bcd0

<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
<meta name="go-import" content="golang.org/x/net git https://go.googlesource.com/net">
<meta name="go-source" content="golang.org/x/net https://github.com/golang/net/ https://github.com/golang/net/tree/master{/dir} https://github.com/golang/net/blob/master{/dir}/{file}#L{line}">
<meta http-equiv="refresh" content="0; url=https://godoc.org/golang.org/x/net">
</head>
<body>
Nothing to see here; <a href="https://godoc.org/golang.org/x/net">move along</a>.
</body>
</html>

which appears to be identical in form to my meta tags.

The expected output from the test is:

module root is "golang.org/x/net"

so I can only assume this behaviour is intentional for now, i.e. support for submodules is a TODO?

@rsc
Copy link
Contributor

rsc commented Apr 5, 2018

The mod directives are documented at https://research.swtch.com/vgo-module in the "Publishing" section.

If you wrote an import "myitcv.io/generate" then during a subsequent build modfetch.Import would try that path and then back down to "myitcv.io" which would succeed. But "vgo get" assumes it is being given module paths, not arbitrary import paths, and so it doesn't try prefixes. This is another thing to fix as part of the overall vgo get rethink (see also #24105).

@rsc rsc changed the title x/vgo: vgo get myitcv.io/gogenerate fails with "module root is myitcv.io" x/vgo: vgo get only accepts module root as path Apr 5, 2018
@rsc rsc added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Apr 5, 2018
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 5, 2018
@myitcv
Copy link
Member Author

myitcv commented Apr 5, 2018

Edit: also see https://go-review.googlesource.com/c/vgo/+/114819#message-98130f3c63189bdee0a22a3509e56192134fdd8d

Thanks @rsc

The mod directives are documented at https://research.swtch.com/vgo-module in the "Publishing" section.

Noted; I'll look at adding such a meta tag once I've got the regular go-import working with vgo.

If you wrote an import "myitcv.io/generate" then during a subsequent build modfetch.Import would try that path and then back down to "myitcv.io" which would succeed

Ah, ok. So if I now try:

cd `mktemp -d`
export GOPATH=$PWD
mkdir blah
cd blah/
echo 'module "rubbish.com/blah"' >> go.mod
echo 'package blah
import _ "myitcv.io/blah"
' >> blah.go
vgo build

Then I get an error

vgo: resolving import "myitcv.io/blah"
vgo: finding myitcv.io (latest)
vgo: adding myitcv.io v0.0.0-20180405194116-cdcde380fd71
vgo: finding myitcv.io v0.0.0-20180405194116-cdcde380fd71
vgo: downloading myitcv.io v0.0.0-20180405194116-cdcde380fd71
vgo: import "rubbish.com/blah" ->
        import "myitcv.io/blah" [/tmp/tmp.mqlE9gzbST/src/v/[email protected]/blah]: open /tmp/tmp.mqlE9gzbST/src/v/[email protected]/blah: no such file or directory

If I list the contents of GOPATH:

src/v/[email protected]/go.mod
src/v/[email protected]/.travis.yml
src/v/[email protected]/README.md
src/v/[email protected]/_scripts/common.bash
src/v/[email protected]/_scripts/run_tests.sh
src/v/[email protected]/_scripts/check_git_is_clean.sh
src/v/cache/myitcv.io/@v/v0.0.0-20180405194116-cdcde380fd71.mod
src/v/cache/myitcv.io/@v/v0.0.0-20180405194116-cdcde380fd71.zip
src/v/cache/myitcv.io/@v/v0.0.0-20180405194116-cdcde380fd71.info
src/v/cache/myitcv.io/@v/v0.0.0-20180405194116-cdcde380fd71.ziphash

So it seems the blah subdirectory (module) is not being detected?

The go.mod's in the https://github.com/myitcv/x repo are:

./go.mod
./blah/go.mod

I'll do some digging to see if I can work out what's going on.

But "vgo get" assumes it is being given module paths, not arbitrary import paths, and so it doesn't try prefixes.

Got it. This is going to require a bit of a mental shift (potentially only for me?) because I'm used to go get either getting a package or installing a program. My understanding of the latter in vgo speak is the one should use vgo install X (per #24051). But I'll follow #24105 for any updates.

@myitcv
Copy link
Member Author

myitcv commented Apr 6, 2018

I'll do some digging to see if I can work out what's going on.

@rsc I'm going to park my investigation for now because I'm not entirely clear whether you are expecting this scenario (i.e. submodules within a repo) to work or whether it's simply a TODO.

As you said, I can see the fallback logic taking effect. So this first call to try fails with a *ModuleSubdirError. I was half expecting to see something like:

var p string

if err, ok := err.(*ModuleSubdirError); ok {
	p = err.ModulePath
} else {
	p = pathpkg.Dir(path)
}

But even then we lose the original submodule in the subsequent call to try. We almost need to be passing round the submodule path (if there is one, else "") so that the ultimate call to modfetch.Query also knows that we're looking for a submodule.

At this point however I'm honestly guessing as to what's intended, hence the pause!

@myitcv
Copy link
Member Author

myitcv commented Apr 6, 2018

As a further update to this issue, just to note the myitcv.io/gogenerate example will now "work" because I've setup the go-import meta tag with the mod VCS for this package.

I've therefore setup myitcv.io/blah to demonstrate the problem as originally described (I will also update my comments above for completeness)

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/114819 mentions this issue: cmd/go/internal/modfetch: resolve submodules in custom imports

@myitcv myitcv changed the title x/vgo: vgo get only accepts module root as path x/vgo: unable to resolve submodules for custom import paths Jun 18, 2018
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/120042 mentions this issue: cmd/go/internal/modfetch: fix Lookup, Import; add ImportRepoRev

@golang golang locked and limited conversation to collaborators Jun 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

5 participants