x/net/http2/h2demo: should not be single-handedly responsible for adding 3 required modules to x/net

[dmitshur]

The golang.org/x/net module holds supplementary Go networking libraries. It currently requires 5 modules:

require (
	cloud.google.com/go v0.36.0
	go4.org v0.0.0-20190218023631-ce4c26f7be8e
	golang.org/x/build v0.0.0-20190307215223-c78805dbabc8
	golang.org/x/crypto v0.0.0-20190228161510-8dd112bcdc25
	golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2
)

The h2demo package is single-handedly responsible for x/net requiring cloud.google.com/go, go4.org, and golang.org/x/build modules (it has a // +build h2demo build constraint, but that doesn't have any effect when computing module requirements):

https://github.com/golang/net/blob/16b79f2e4e95ea23b2bf9903c9809ff7b013ce85/http2/h2demo/h2demo.go#L31-L33

The x/net module should not be requiring x/build (and in turn, the world, due to #29935).

If the h2demo package is removed, the list of required modules in x/net becomes just:

require (
	golang.org/x/crypto v0.0.0-20190228161510-8dd112bcdc25
	golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2
)

I see 2 possible paths to resolve this:

1. move h2demo somewhere else (to x/build?)
2. make h2demo a separate module in x/net by adding a go.mod file to it, in effect cutting it and its required modules off from the rest of x/net

Based on the content of the h2demo directory and the fact it's a part of our infrastructure (i.e., it's the server powering http2.golang.org), I suggest we move h2demo to x/build. It can use the http2 package by requiring x/net module. @bradfitz How does that sound to to you?

/cc @bradfitz @matloob @bcmills

[bcmills]

Either solution SGTM.

[bradfitz]

I'd rather not move it away from the code that it's a demo of.

[bcmills]

Are you ok with carving it out into its own module, keeping it at the same path (and in the same repo)?

[dmitshur]

Ok. If Brad is okay with making it a separate module in x/net instead, I want @matloob and @ianthehat's agreement of introducing a multi-module repository in order to resolve this issue.

I propose making it a separate module but not making any tagged releases yet, which is more in line with @bcmills's plan described in #28136 (comment) that we have been following so far for all subrepos.

Edit: I don't yet know if it's better to include a replace golang.org/x/net => ../.. directive in the h2demo module's go.mod file, or to leave it out. This would need to be figured out.

[bradfitz]

Are you ok with carving it out into its own module, keeping it at the same path (and in the same repo)?

That's fine with me, and shouldn't impact its users, since its users are approximately 1-2 people (us).

[gopherbot]

Change https://golang.org/cl/166337 mentions this issue: http2/h2demo: carve out into a separate module

[dmitshur]

I've created CL 166337 that implements the carving of h2demo out into a separate module. Please feel free to review it.

If CL 162822 (adding a root go.mod to golang.org/x/net module) is rebased on top of it and updated, the x/net root go.mod file becomes just:

module golang.org/x/net

require (
	golang.org/x/crypto v0.0.0-20190228161510-8dd112bcdc25
	golang.org/x/text v0.3.0
)

Before we can proceed, I'm waiting to make sure @matloob and @ianthehat are okay with the decision of creating a new golang.org/x/net/http2/h2demo module (in order to resolve this issue).

[matloob]

I'd be okay with it, as long as we put in a replace directive in net pointing to the local h2demo.

[dmitshur]

a replace directive in net pointing to the local h2demo.

Do you mean a replace directive in h2demo, pointing to the local x/net?

I can add that to CL 166337. Deciding on whether to commit a replace golang.org/x/net => ../.. directive is something I mentioned in #30685 (comment).

[dmitshur]

I can add that to CL 166337.

Actually, I can't, because it requires the go.mod file to exist at ../../go.mod, which will happen after CL 162822 is merged.

Do you want make adding the replace directive a part of your CL instead?

[dmitshur]

Alternatively, it may make more sense to add the root go.mod file first, and carve out h2demo into a separate module second. @matloob Thoughts on that?

[matloob]

Sure, that's okay

[pwaller]

I'd be okay with it, as long as we put in a replace directive in net pointing to the local h2demo.

It would be interesting for those of us following along at home (:wave:) to know what is the effect you desire from this choice. Asking because I want to understand the thought processes and perhaps apply the same ideas elsewhere.

[dmitshur]

The effect of having a replace golang.org/x/net => ../.. directive in the future golang.org/x/net/http2/h2demo module is that when you're doing local development, it will use the version of golang.org/x/net that is on your disk two parent directories up, instead of the version specified in the http2/h2demo/go.mod file.

It also means any local changes to the http2, etc., packages from golang.org/x/net module will be picked up when building or testing h2demo.

[gopherbot]

Change https://golang.org/cl/162822 mentions this issue: all: add go.mod files, carve h2demo into separate module

[gopherbot]

Change https://golang.org/cl/166857 mentions this issue: http2/h2demo: require golang.org/x/net@latest

[gopherbot]

Change https://golang.org/cl/196036 mentions this issue: http2/h2demo: remove h2demo build constraint

[gopherbot]

Change https://golang.org/cl/198917 mentions this issue: [release-branch.go1.13] http2/h2demo: remove h2demo build constraint