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

go/build: regression in go 1.7rc1, cannot vendor golang.org/x/net/http2 #16333

Closed
vanackere opened this issue Jul 12, 2016 · 14 comments
Closed
Milestone

Comments

@vanackere
Copy link
Contributor

Please answer these questions before submitting your issue. Thanks!

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

go 1.7rc1

  1. What operating system and processor architecture are you using (go env)?
OARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/vince/dev"
GORACE=""
GOROOT="/home/vince/go"
GOTOOLDIR="/home/vince/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build541667125=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
  1. What did you do?

Put a copy of golang.org/x/net/http2 at $GOPATH/src/vendor/golang.org/x/net/http2

Try to build the following program using go build :

package vendoring17

import _ "golang.org/x/net/http2"
  1. What did you expect to see?

Everything was working fine using go 1.6

  1. What did you see instead?

With go 1.7 I get :

package github.com/vanackere/bugs/vendoring17
imports golang.org/x/net/http2: use of vendored package not allowed

This bug appears at 0104a31 where golang.org/x/net/http2/hpack moves from internal to vendor

From a first look it appears that (*Context) Import is confused by this change and assumes that vendor/golang.org/x/net/http2 is a package located in GOROOT. I don't have a proper fix for the moment (still looking into it) but I believe that this bug should be fixed for go 1.7.

@bradfitz bradfitz added this to the Go1.7 milestone Jul 12, 2016
@bradfitz
Copy link
Contributor

I can reproduce. Thanks for the report.

@ianlancetaylor
Copy link
Contributor

As far as I can tell this only fails when the directory is precisely $GOPATH/src/vendor/golang.org/x/net/http2. It does not fail if the vendor directory is not at top level. And it does not fail if the vendor directory is something other than golang.org/x/net/http2.

@ianlancetaylor
Copy link
Contributor

I don't see a simple fix for the general problem. The go tool only wants to a single import path to denote a single package. I can fix it so that $GOPATH/src/vendor/golang.org/x/net/http2 works, because there is no such package in $GOROOT, but if $GOPATH/src/vendor/golang.org/x/net/http2/hpack exists, then this can't work, because $GOPATH/src/vendor/golang.org/x/net/http2 will import it, and $GOROOT/src/net/http will import $GOROOT/src/vendor/golang.org/x/net/http2/hpack, and now we have two different packages with the import path vendor/golang.org/x/net/http2/hpack. That should logically work, but it doesn't, and I think that fixing that is too risky for 1.7.

The only options I can see are to punt this to 1.8 as unfortunately broken in 1.7, or to move the $GOROOT/src/vendor/golang.org directory to $GOROOT/src/internal as we did in 1.6.

@bradfitz Do you see anything else we can do? Any preferences?

@mdempsky
Copy link
Member

mdempsky commented Jul 13, 2016

Another non-invasive partial* fix would be to relocate $GOROOT/src/vendor/golang.org/... to $GOROOT/src/net/vendor/golang.org/.... That just involves git mv vendor net and a one line fix to go/build/deps_test.go.

(* Agreed with @ianlancetaylor that the correct fix involves making the compiler aware that "vendor" and "internal" packages are workspace-specific.)

@gopherbot
Copy link

CL https://golang.org/cl/24902 mentions this issue.

@bradfitz
Copy link
Contributor

@mdempsky, maybe I don't understand the issue enough, but that still scares me. What does the compiler see then for a user who imports it in their "src/vendor/golang.org/x/net/http2" and their code does import "golang.org/x/net/http2"?

@mdempsky
Copy link
Member

mdempsky commented Jul 13, 2016

@bradfitz If a user creates $GOPATH/src/vendor/golang.org/x/net/http2 and imports it as "golang.org/x/net/http2", the compiler will see that as package at path "vendor/golang.org/x/net/http2". My proposal would make the stdlib's vendored copy appear as "net/vendor/golang.org/x/net/http2" to the compiler, which it will treat as distinct.

Because "net/vendor/..." packages are only visible within the "net/..." package subpath, it's only a risk of collisions if users try to write their code in a $GOPATH/src/net directory, which seems like a bad practice.

If it makes you feel more comfortable, we have $GOROOT/src/cmd/vendor/golang.org/x/arch/... already, without any extra package path mangling.

@bradfitz
Copy link
Contributor

I think it's likely a user would vendor golang.org/x/net/http2 into their $GOPATH/src/net/vendor/golang.org/x/net/http2.

I think the golang_org (with underscore instead of dot) in the CL is a safer option, unlikely to be done by users, since it's an invalid hostname.

@YvanDaSilva
Copy link

YvanDaSilva commented Jul 15, 2016

@bradfitz This causes issues, at least with gb.
When I fetch k8s.io/kubernetes/pkg/api/v1 at tag v1.3.0 it results in:
"golang_org/x/net/http2/hpack" is not a valid import path

@bradfitz
Copy link
Contributor

@YvanDaSilva, that is not a sufficiently complete bug report. What do you mean "When I fetch"? What did you run?

Please file a new bug, in any case, since we tend to lose comments on closed bugs. You can reference this bug number in your new bug and they'll be cross-linked for bug archaeologists later. Be sure to copy @davecheney if it's a gb-specific bug.

@YvanDaSilva
Copy link

YvanDaSilva commented Jul 15, 2016

@bradfitz Sorry for that, gb vendor fetch is somehow equivalent to a go get.
Not sure this is specifc to gb as I couldn't reproduce with "go get -v k8s.io/kubernetes/pkg/api/v1" since that package is not gettable in the current state (go generated code issue). If you know how to get a specific tag with go get I'll gladly make further testing (but as for now I doubt it is possible).

Concerning the command I executed it is : "gb vendor fetch -tag v1.3.0 k8s.io/kubernetes/pkg/api/v1"
which outputs : "golang_org/x/net/http2/hpack" is not a valid import path

Note: Not sure I'll open an issue with this, because I don't have another package where I can test this with "go get" and provide you more output.

@sekimura
Copy link

@YvanDaSilva For me, it looks like gb's regex issue. You might want to open issue on gb repo to allow "_" in the regex below.

https://github.com/constabulary/gb/blob/4ffaea660a4e178e62e94fb8447fb43039ceb1e8/internal/vendor/repo.go#L72

CC: @davecheney

@YvanDaSilva
Copy link

Thank you, will definitively add this to the currently open issue on GB.

davecheney added a commit to constabulary/gb that referenced this issue Jul 24, 2016
Fixes #635

golang/go#16333 renamed $GOROOT/src/vendor/golang.org to
$GOROOT/src/vendor/golang_org. This solved a nasty problem where code in
GOROOT would shadow any other copy of the net/http2 library, but meant
that gb vendor could no longer resolve the package and try to fetch it
(even if that was pointless).

To solve this, include $GOROOT/src/vendor in the set of search paths so
that golang_org/net/http/... is always found (if present).

I'll add a test for this in the integration test repo.
davecheney added a commit to constabulary/gb that referenced this issue Jul 24, 2016
#640)

* cmd/gb-vendor: search $GOROOT/src/vendor when resolving recursive deps

Fixes #635

golang/go#16333 renamed $GOROOT/src/vendor/golang.org to
$GOROOT/src/vendor/golang_org. This solved a nasty problem where code in
GOROOT would shadow any other copy of the net/http2 library, but meant
that gb vendor could no longer resolve the package and try to fetch it
(even if that was pointless).

To solve this, include $GOROOT/src/vendor in the set of search paths so
that golang_org/net/http/... is always found (if present).

I'll add a test for this in the integration test repo.

* internal/vendor: handle missing depset roots

$GOROOT/src/vendor does not exist in Go < 1.6
@gopherbot
Copy link

CL https://golang.org/cl/29086 mentions this issue.

gopherbot pushed a commit to golang/tools that referenced this issue Sep 13, 2016
I've been doing this by hand since Go 1.7rc2.

Updates golang/go#16333

Change-Id: Ib12c013b14210123d48d6ad78922caf1286c20cc
Reviewed-on: https://go-review.googlesource.com/29086
Reviewed-by: Alan Donovan <adonovan@google.com>
@golang golang locked and limited conversation to collaborators Sep 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants