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: cannot resolve subpackages of unfetched modules #26602

Closed
bcmills opened this issue Jul 25, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@bcmills
Copy link
Member

commented Jul 25, 2018

In the fix for #26238, @rsc introduced a new +incompatible convention for projects using legacy build tags.

However, that mechanism does not seem to work for resolving packages that are not at the root of the module tree.

$ go version
go version devel +a9e638380f Tue Jul 24 16:59:54 2018 -0400 linux/amd64
$ cd $(mktemp -d)
$ export GOPATH=$(pwd)
$ mkdir -p src/proxy
$ cd src/proxy

src/proxy$ cat > proxy.go
package proxy
import _ "github.com/google/martian/httpspec"

src/proxy$ GO111MODULE=on go mod -init -module example.com/proxy
go: creating new go.mod: module example.com/proxy

src/proxy$ GO111MODULE=on go get github.com/google/martian/httpspec@v2.0.0-beta.2
go: finding github.com/google/martian/httpspec v2.0.0-beta.2
go: finding github.com/google/martian/httpspec latest
go: finding github.com/google/martian v2.0.0-beta.2+incompatible
go: downloading github.com/google/martian v2.0.0-beta.2+incompatible
go get github.com/google/martian/httpspec@v2.0.0-beta.2: cannot find module providing package github.com/google/martian/httpspec

src/proxy$ GO111MODULE=on go list github.com/google/martian/httpspec
go: finding github.com/google/martian/httpspec latest
go: import "github.com/google/martian/httpspec": cannot find module providing package github.com/google/martian/httpspec

src/proxy$ GO111MODULE=off go get github.com/google/martian/httpspec

src/proxy$ GO111MODULE=off go list github.com/google/martian/httpspec
github.com/google/martian/httpspec

@bcmills bcmills changed the title cmd/go: 'go mod -sync' fails to resolve imports of packages with "+incompatible" major versions cmd/go: cannot resolve subpackages of modules with "+incompatible" major versions Jul 25, 2018

@bcmills bcmills added the modules label Jul 25, 2018

@bcmills

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2018

This was found while attempting to produce a module definition for golang.org/x/build.

@bcmills bcmills added this to the Go1.11 milestone Jul 25, 2018

@bcmills bcmills changed the title cmd/go: cannot resolve subpackages of modules with "+incompatible" major versions cmd/go: cannot resolve subpackages of unfetched modules Aug 2, 2018

@bcmills

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2018

I can reproduce this by moving a small block of mod_get_commit.txt to the beginning of the test (before the attempts using the module root path):

# golang.org/x/text/language@commit should not resolve with -m,
# because that's not a module path.
! go get -m golang.org/x/text/language@14c0d48
# ... but it should work without -m.
# because of -d, the compiler should not run
go get -d -x golang.org/x/text/language@14c0d48
! stderr 'compile|cp|gccgo .*language\.a$'

(CC @rsc, but I'm going to see if I can fix it tomorrow.)

@bcmills

This comment has been minimized.

Copy link
Member Author

commented Aug 3, 2018

This turned out to be a complex of compensating bugs and surprises.

  • modload.Import happily imports module roots as if they were packages, even if they aren't.
  • go get uses the output of modload.Import to decide which module to fetch.
  • search.CleanImportPaths returns "." if len(args) == 0.

As a result, a bunch of our existing tests end up compiling and installing empty packages in response to go get -u without arguments. There needs to be some step where we skip installation entirely if we invoked get without args in a module root that is not a package, and I haven't yet figured out how to do that cleanly. (I have a pretty big subset of the change done, but without that the tests fail right and left.)

@gopherbot

This comment has been minimized.

Copy link

commented Aug 4, 2018

Change https://golang.org/cl/127936 mentions this issue: cmd/go: do not rely on Import to find the module for a new package

@gopherbot

This comment has been minimized.

Copy link

commented Aug 4, 2018

Change https://golang.org/cl/127935 mentions this issue: cmd/go: reproduce #26602

@gopherbot

This comment has been minimized.

Copy link

commented Aug 6, 2018

Change https://golang.org/cl/128136 mentions this issue: cmd/go/internal: factor out modload.QueryPackage and use in in modget

@gopherbot gopherbot closed this in a1cbbe0 Aug 9, 2018

gopherbot pushed a commit that referenced this issue Aug 9, 2018

cmd/go/internal: factor out modload.QueryPackage and use in in modget
modload.Import contains a loop that looks for the module containing a package.
Because we overload Import to locate both packages and modules, that loop
contains a bunch of special-cases for modules with empty roots.

In this change, we factor out the loop into a new function (QueryPackage) and
use that directly in modget.getQuery. That restores the invariant that
the paths passed to modload.Import must be importable packages, and fixes 'go
get' lookups for packages that have moved between a module and submodules with
the same path prefix.

Updates #26602.

Change-Id: I8bc8340c17f2df062d03ce720f4dc18b2ba406b2
Reviewed-on: https://go-review.googlesource.com/128136
Reviewed-by: Russ Cox <rsc@golang.org>
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.