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.

Sign up for GitHub

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

Jump to bottom

x/tools/cmd/bundle: use caller's module version, not $GOPATH #32031

Closed
bradfitz opened this issue May 14, 2019 · 5 comments
Closed

x/tools/cmd/bundle: use caller's module version, not $GOPATH #32031

bradfitz opened this issue May 14, 2019 · 5 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@bradfitz
Copy link
Contributor

bradfitz commented May 14, 2019 •
edited

There are two bundled packages in $GOROOT/src/net/http:

bradfitz@go:~/go/src/net/http$ git grep go:gener
h2_bundle.go://go:generate bundle -o h2_bundle.go -prefix http2 golang.org/x/net/http2
socks_bundle.go://go:generate bundle -o socks_bundle.go -dst net/http -prefix socks -underscore golang.org/x/net/internal/socks

A bundled package is a a package that gets fused in with another package to avoid an otherwise-circular dependency. (We can't just import golang.org/x/net/http2 from net/http as http2 implements interfaces using types in their signatures defined in net/http)

Currently, to update http2 in std you need to both update the go.mod+vendor files (for some of the http2 subpackages) as well as run the go generate bundle step to generate h2_bundle.go.

The bundle appears to just gets it code from whatever you have on your disk in $GOPATH, though. It should use the same version from the current module ($GOROOT/src/go.mod).

/cc @bcmills @jayconrod @alandonovan @dmitshur @ianlancetaylor
@gopherbot gopherbot added this to the Unreleased milestone May 14, 2019
@gopherbot
Copy link

Change https://golang.org/cl/177037 mentions this issue: net/http: update vendored, bundled x/net/http2

@FiloSottile FiloSottile added the NeedsFix The path to resolution is known, but the work has not been done. label May 14, 2019
@flibustenet flibustenet mentioned this issue May 15, 2019
Closed
63 tasks
@dmitshur
Copy link
Contributor

dmitshur commented May 15, 2019 •
edited

bundle currently uses golang.org/x/tools/go/loader to load packages. At first glance, this issue is about making it use golang.org/x/tools/go/packages instead, which will make it support modules.

Added bundle to #24661.

/cc @ianthehat @matloob

gopherbot pushed a commit that referenced this issue May 16, 2019
@bradfitz
net/http: update vendored, bundled x/net/http2
cb5c82b 
For:

    http2: track reused connections
    https://golang.org/cl/176720 (updates #31982)

Some x/sys/unix updates come along for the ride too.

I filed #32031 for making the bundling process less difficult and
error-prone in the future.

Change-Id: Ic822080991ffa2d50352c5f613e45648a327cf16
Reviewed-on: https://go-review.googlesource.com/c/go/+/177037
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/189818 mentions this issue: cmd/bundle: use caller's module version via go/packages

@gopherbot
Copy link

Change https://golang.org/cl/189897 mentions this issue: src/go.mod: sync golang.org/x/net with h2_bundle.go

@FiloSottile FiloSottile self-assigned this Aug 11, 2019
gopherbot pushed a commit that referenced this issue Aug 12, 2019
@FiloSottile
src/go.mod: sync golang.org/x/net with h2_bundle.go
4550406 
The bundle included changes from a commit after the one referred to by
the go.mod, probably due to cmd/bundle using the GOPATH source.

Identified with the new go/packages based cmd/bundle from CL 189818.

$ go get golang.org/x/net@461777fb6f
$ go mod tidy
$ go mod vendor
$ go generate net/http # with CL 189818

Also, updated the socks_bundle.go generate command to drop obsolete
options and match h2_bundle.go. It caused no output changes.

Updates #32031

Change-Id: I0322d4e842dbfdad749455111072ca4872a62ad4
Reviewed-on: https://go-review.googlesource.com/c/go/+/189897
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@gopherbot
Copy link

Change https://golang.org/cl/209901 mentions this issue: src/go.mod: sync golang.org/x/net with h2_bundle.go

gopherbot pushed a commit that referenced this issue Dec 5, 2019
@FiloSottile
src/go.mod: sync golang.org/x/net with h2_bundle.go
af08822 
CL 209077 updated bundled http2 to x/net git rev ef20fe5d7 without
bumping the go.mod version.

Identified with the new go/packages based cmd/bundle from CL 189818.

$ go get golang.org/x/net@ef20fe5d7
$ go mod tidy
$ go mod vendor
$ go generate -run bundle std # with CL 189818

Updates #32031

Change-Id: I581d35f33e2adafb588b2b0569648039187234a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/209901
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This was referenced Sep 15, 2020
Closed
Closed
@golang golang locked and limited conversation to collaborators Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@bradfitz @FiloSottile @dmitshur @gopherbot