http -> http2 -> http import cycle

[Brad Fitzpatrick]

Good news: http2 is in pretty good shape and we started to vendor it into the main Go tree (https://go-review.googlesource.com/#/c/15822/)

Bad news:

##### Building packages and commands for darwin/amd64.

import cycle not allowed

package expvar

imports net/http

imports golang.org/x/net/http2

imports net/http

As background:

* upstream for http2 is golang.org/x/net/http2

* the goal for Go 1.6 was to have http2 transparently enabled for all users, with no APIs changes.

* we'd provide a mechanism in net/http to disable http2, but if you wanted to customize it, you'd need to x/net/http2 yourself.

Because so far we've always enabled http2 (and previously spdy) by registering the http2 package with the http package, I never thought about the import cycle that would result if we made this registration automatic.

Of the two dependency edges in question,

 

1) net/http -> x/net/http2

2) x/net/http2 -> net/http

Edge (2) is fundamental. We can't implement http.Handler etc without the concrete *http.Request type, for instance.

Edge (1) is only so users of net/http get http2 for free.

We have 4 potential paths forward to break the import cycle:

a) Give up the dream.

No automatic http2. Make users import _ "net/http/registerhttp2" (or something. Still no new API, but a new empty, magic package. (and a public package, not "internal"). This is pretty cargo-culty, though, and we can't say automatic http2.

b) Modify cmd/go to make use of net/http imply import "net/http/internal/registerhttp2".

People will object to this one.

The proposal is that packages are allowed to list other packages they require also be loaded, and cmd/go synthesizes an empty import to it, as if you had a new file with import _ "peer/package", similar to how "runtime" is always linked in for all packages. With this, "net/http" would declare with some magic syntax (or just hard-coded into cmd/go) that it needs "net/http/internal/registerhttp2". This would be an internal mechanism only (at least for now).

c) Use cmd/bundle.

Russ wrote a tool previously: https://code.google.com/p/rsc/source/browse/cmd/bundle/main.go

Use cmd/bundle to copy http2 into the http package, combining them all together, with http2 stuff prefixed with "http2" so it's all unexported.

I hoped the "vendor" mechanism would wean us off this tool, though.

d) Merge http2 into the http package and standard library.

This is the most work, but grpc and others depend on http2 being separate.

And I like it separate.

And I fear how much new API would be exposed in net/http, even if we tried to contain it (but even then, we'd still probably need an external http2 package)

All options suck.

Thoughts?

I kinda like (b), modifying cmd/go, the most, even though I know I'll be controversial.

I imagine people will say (c). I think that's Andrew's vote. How mature is cmd/bundle?

- Brad

[Andrew Gerrand]

Yeah, (c) is my vote.

On 14 October 2015 at 15:48, Brad Fitzpatrick <brad...@golang.org> wrote:

How mature is cmd/bundle?

We were at one stage using it in the core for the assemblers IIRC.

[David Symonds]

I vote for (c) too, though isn't (d) the long-term plan anyway?

[Dave Day]

(c) has my vote.

If I understand correctly, (b) and (c) will look the same to developers? (ie. the public API of net/http is the same either way?)

[Liam]

(Hope these are appropriate remarks for the -dev group. It's about development of the language, even if I'm not a developer of the language.)

As just a -nuts type developer, I'd vote a) followed by a well-understood (amongst the wider Go community) transition to d).

Whose dream is it anyway? 

Is the typical user of Go (developers using it to build things in their companies) particularly worried about alignment with Google initiatives like gRPC and such, or even transparent HTTP/2 everywhere? I'm not. I think HTTP/2 is neat but I don't mind the idea of consciously enabling it.

Cheers

[Peter Waldschmidt]

In option (c), how would a user of the package get access to the http2 api? It sounds like if you want to work with the http2 api, you'd have two copies of golang/x/net/http2 included in your app (one imported and another embedded in net/http. Will this cause internal conflicts if both are in use at the same time?

[Ingo Oeser]

(a) sounds good to me, too with a transition scheduled for (d)

Rationale: Less maintenance burden and careful migration to H2 for existing apps.

New features should be opt-in for the developer, not forced down upon them.

Having and running an script adding a
"import _ "golang/x/net/http2" for all web services who need it, doesn't sound like rocket science to me.

[Brad Fitzpatrick]

On Tue, Oct 13, 2015 at 10:39 PM, David Symonds <dsym...@golang.org> wrote:

I vote for (c) too, though isn't (d) the long-term plan anyway?

No, the plan was never to ship the http2 low-level API (https://godoc.org/golang.org/x/net/http2) in net/http. That's way too much API surface.

In Go 1.7 or 1.8 we'll probably have APIs for sending PUSH_PROMISE frames in net/http, but with a Go-like API, not raw frame writing.

[Brad Fitzpatrick]

On Tue, Oct 13, 2015 at 10:52 PM, Dave Day <d...@golang.org> wrote:

(c) has my vote.

If I understand correctly, (b) and (c) will look the same to developers? (ie. the public API of net/http is the same either way?)

Correct.

 

[roger peppe]

My initial feeling is that option a) is best. The others feel like
we're adding magic rather than respecting the language
as it is. We don't need another special package - surely
http2 can register itself with net/http? So import "net/http/http2"
anywhere in the program should be sufficient.

If I was going to suggest some magic somewhere, it might be
a build flag to say "add this/these import(s)", so someone
could take an arbitrary command and add support for http2
to it without needing to add the requisite _ import. Also
good for adding image formats and anything else that
relies on a registration mechanism. But this breaks the
"no flags needed" go build mechanism.

ISTM that c) is just allowing cyclic imports by another name.
Once could actually allow cyclic imports by making _ imports
special - don't count it as cycle if it goes through a _ import.
But that would break any number of tools in the ecosystem.

After a little reflection and a brief look through the code,
I'm inclined to question this statement:

> Edge (2) is fundamental. We can't implement http.Handler etc
> without the concrete *http.Request type, for instance.

Looking at the public API, the only things that fundamentally
require net/http are server.go and transport.go.
Other than that, we have uses of http.Header (aka textproto.MIMEHeader)
and http.StatusText (easy to factor out).

So my suggestion would be to export enough from net/http/internal/http2
that it's possible to implement ConfigureServer in terms of that.
Might that be possible?

cheers,
rog.

> --
> You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Ian Lance Taylor]

How hard would it be to gut the net/http package, while preserving the
API, and move all the real code into net/http/http1?

Then net/http could use both net/http/http1 and net/http/http2 (names
can be changed as appropriate). Users of net/http would get both, and
would import the specific package for specific tweaks.

Ian

[Brad Fitzpatrick]

I don't think it's possible. It's the same problem, but with two import cycles, since both http1 and http2 would need to e.g. implement their servers using *http.Request.

[Brad Fitzpatrick]

On Wed, Oct 14, 2015 at 7:31 AM, roger peppe <rogp...@gmail.com> wrote:

My initial feeling is that option a) is best. The others feel like
we're adding magic rather than respecting the language
as it is. We don't need another special package - surely
http2 can register itself with net/http? So import "net/http/http2"
anywhere in the program should be sufficient.

I only included option (a) for completeness. We really do want http2 to transparently work, in the same way we didn't make http/1.1 support opt-in when we added that.

ISTM that c) is just allowing cyclic imports by another name.

Or you can think of them as the same package.

 

Once could actually allow cyclic imports by making _ imports
special - don't count it as cycle if it goes through a _ import.

But that would break any number of tools in the ecosystem.

An interesting thought.

 

After a little reflection and a brief look through the code,
I'm inclined to question this statement:

> Edge (2) is fundamental. We can't implement http.Handler etc
> without the concrete *http.Request type, for instance.

Looking at the public API, the only things that fundamentally
require net/http are server.go and transport.go.
Other than that, we have uses of http.Header (aka textproto.MIMEHeader)
and http.StatusText (easy to factor out).

So my suggestion would be to export enough from net/http/internal/http2
that it's possible to implement ConfigureServer in terms of that.
Might that be possible?

No, http2 really needs to get at *http.Request. I can put the framing code into its own package, but that stuff is trivial. The main stuff (http2/server.go) needs *http.Request.

[Brendan Tracey]

> On Oct 14, 2015, at 8:32 AM, Ian Lance Taylor <ia...@golang.org> wrote:
>
> How hard would it be to gut the net/http package, while preserving the
> API, and move all the real code into net/http/http1?

We did something similar for BLAS in gonum

We want to have a BLAS implementation registered by default but the different implementations (pure go vs cgo) both need to share common types.

We have:
blas (types and interface definitions)
blas/native (native go implementation)
blas/cgo (cgo implentation)
blas/blas64 (public facing API)

cgo and native both import blas, and blas64 provides registration capabilities (importing blas/native for the default)

>
> Then net/http could use both net/http/http1 and net/http/http2 (names
> can be changed as appropriate). Users of net/http would get both, and
> would import the specific package for specific tweaks.
>
> Ian
>

[Eric Myhre]

While acknowledging the complication of this tactic (esp while preserving api backcompat), I do like this approach.

I don't know if it's idiomatic per se, but it feels familiar to me: every time I write a system with "plug in" implementations, I end up doing essentially what Ian suggests. There's a subpackage for each implementation, a subpackage for the "manager" or "mux" or whatever you call it that imports all the implementations, and the base package has all the essential interfaces and shared types. (Or, swap the mux and shared+types around so the mux is at the base; same thing, season to taste.)

This *seems* like a lot of packages, but so far is working well in practice. (It also seems to make one cognizant of letting too much "helper" code for initializations slip into the mux package, though YMMV with the situation on that; probably not relevant to already well-developed packages like net/http.)

[roger peppe]

On 14 October 2015 at 15:40, Brad Fitzpatrick <brad...@golang.org> wrote:
>
>
> On Wed, Oct 14, 2015 at 7:31 AM, roger peppe <rogp...@gmail.com> wrote:
>>
>> My initial feeling is that option a) is best. The others feel like
>> we're adding magic rather than respecting the language
>> as it is. We don't need another special package - surely
>> http2 can register itself with net/http? So import "net/http/http2"
>> anywhere in the program should be sufficient.
>
>
> I only included option (a) for completeness. We really do want http2 to
> transparently work, in the same way we didn't make http/1.1 support opt-in
> when we added that.
>
>> ISTM that c) is just allowing cyclic imports by another name.
>
>
> Or you can think of them as the same package.

If you can think of them as the same package, they should *be* the
same package. The namespace in Go's packages is global,
and I think that's probably a good thing because it encourages
people to break out genuinely independent units of functionality.

> No, http2 really needs to get at *http.Request. I can put the framing code
> into its own package, but that stuff is trivial. The main stuff
> (http2/server.go) needs *http.Request.

In which case I think it should live in net/http. Is there really
that much namespace collision?

I think you're trying to have your cake and eat it here. Either
http2 is part of net/http or it isn't. If it is, then include the
parts of http2 that are necessary and factor out the rest
into an internal package. If it isn't, then bite the option a)
bullet.

Changing the language to support this use case seems wrong to
me. This is a relatively common Go dilemma, I think - I don't think it's
*that* much of a special case that it justify the means proposed.

cheers,
rog.

[Brad Fitzpatrick]

It wasn't really up for a vote. I included (a) only for completeness. Users shouldn't care about the underlying wire protocol. You don't opt in to http/1.1, for example.

On Wed, Oct 14, 2015 at 11:57 AM, <peter.k...@pressly.com> wrote:

I vote (a). The dream of saying "automatic http2" is just marketing. I'd prefer an explicit import _ "net/http2" or "x/net/http2"

[Brad Fitzpatrick]

On Wed, Oct 14, 2015 at 2:48 AM, Liam <goo...@o172.net> wrote:

(Hope these are appropriate remarks for the -dev group. It's about development of the language, even if I'm not a developer of the language.)

As just a -nuts type developer, I'd vote a) followed by a well-understood (amongst the wider Go community) transition to d).

Whose dream is it anyway? 

The web's. The users'.

Is the typical user of Go (developers using it to build things in their companies) particularly worried about alignment with Google initiatives like gRPC and such, or even transparent HTTP/2 everywhere? I'm not. I think HTTP/2 is neat but I don't mind the idea of consciously enabling it.

HTTP/2 is larger than Google.

Nobody is proposing baking in gRPC to the standard library.

[peter.k...@pressly.com]

[Brad Fitzpatrick]

Conclusion: we ended up bundling it, option (c). The existing cmd/bundle didn't work, but Alan Donovan wrote one super quickly which did the trick (thanks!).

CLs are https://go-review.googlesource.com/#/c/15822/4 (the bundle + vendor) and https://go-review.googlesource.com/#/c/15828/10 (turning it on).

HTTP/2 is now on by default for servers, but not clients yet.

Thanks to Andrew and Alan for assistance.

[Mathieu Lonjaret]

So, out of curiosity, how do you keep on evolving it now? Do you keep
on making changes to golang.org/x/net/http2, and rerun the "bundler"
everytime ?
Or do you work directly on net/http/h2_bundle.go ? If the latter, how
do you keep it in sync with golang.org/x/net/http2 (if that is even
needed anymore) ?

[Brad Fitzpatrick]

On Thu, Oct 15, 2015 at 6:51 AM, Mathieu Lonjaret <mathieu....@gmail.com> wrote:

So, out of curiosity, how do you keep on evolving it now? Do you keep
on making changes to golang.org/x/net/http2,

Yes. That's also where the tests live, and the tests are key to development. (Unfortunately bundle loses tests)

 

and rerun the "bundler"
everytime ?

As needed, occasionally, and before new releases.

 

Or do you work directly on net/http/h2_bundle.go ?

That's a scary thought. :)

 

[roger peppe]

SGTM. Wouldn't this be a good use case for go generate?

[Brad Fitzpatrick]

Perhaps.