x/build/coordinator: make oauth2 build green, support GitHub / non-Go dependencies

[adg]

The oauth2 repo is currently broken on the build dashboard because the coordinator doesn't know how to fetch dependencies external to the Go sub-repos.

../../gopath/src/golang.org/x/oauth2/google/default.go:20:2: cannot find package "google.golang.org/cloud/compute/metadata" in any of:
    /tmp/workdir/go/src/google.golang.org/cloud/compute/metadata (from $GOROOT)
    /tmp/workdir/gopath/src/google.golang.org/cloud/compute/metadata (from $GOPATH)

Options:

1. Teach the coordinator how to fetch packages from repos other than the Go sub-repos.
   • I'm not in favor of this approach due to the complete lack of versioning (that already exists, but could be perceived as "worse" when we're talking about repos outside our control).
2. Use a build tag to exclude packages from continuous build. Something like "gobuildfarm". Then packages that shouldn't be tested can specify // +build !gobuildfarm in their source files.
   • I feel somewhat ambivalent about this approach. It sets an odd precedent. It can be used to make the build green when it's actually not. That seems bad.
3. Skip packages that have dependencies outside the Go project.
   • This seems promising. We already know which packages these are. The question is, how do we keep an eye on the packages we're skipping? We don't want to silently lose test coverage by adding an external dependency.
4. Remove oauth2 from the build dashboard as it'll likely never be able to build using only the sub-repos.
   • This has some appeal, because there was no solid reason to make oauth2 a golang.org/x/ repo in the first place. Maybe it doesn't belong on the build dashboard.

My instinct points to options 3 or 2.

cc @bradfitz @rakyll

[rakyll]

Before answering your question, I wonder whether we should vendor the external packages in the oauth2 package. I think we should have never contributed provider-specific bits with external dependencies to the oauth2 package. They could have lived under a google package elsewhere. But since we have these bits and cannot break the existing users, it is too late to move the google package somewhere else. Vendoring will solve the broken build, right?

[adg]

Yes, option 5 is vendoring the dependencies!

The only dependencies this repo has outside the standard library are:

golang.org/x/net/context
google.golang.org/cloud/compute/metadata
google.golang.org/cloud/internal

We should (and need) not vendor x/net/context, but vendoring the tiny cloud/compute/metadata package (and the dependent cloud/internal package) is probably worthwhile.

[bradfitz]

That would break some people's rule that vendoring should only be done by binaries, and not libraries.

I need to solve this for grpc-go too, since I want to run their tests on our infrastructure. I'm thinking a manifest file of deps for the builder, similar to the .travis.yml file (but not yaml).

[kostya-sh]

Could google.golang.org/cloud/compute/metadata and google.golang.org/cloud/internal be copied to golang.org/x/oauth2/internal with paths rewritten instead of vendoring them?

Alternatively it is possible to remove the dependency completely by copying a few functions (metadata.OnGCE, metadata.Get) that are needed from these external packages to oauth2 repo.

[bradfitz]

@kostya-sh, that's just as bad as vendoring them, or a bit worse. I'd rather fix our test infrastructure rather than mirror that code all over the place.

[rakyll]

Fixing the infra SGTM.

[kostya-sh]

@bradfitz, I suggested copying because:

• Building and vendoring packages with less or no dependencies is simpler
• The amount of code to copy is quite small (l think less than 100 lines)
• I assumed that GCE metadata HTTP API is stable

[dsymonds]

I think option 1 is the best course of action, and we can probably get by for quite a while with a simple list of additional packages to fetch for a given subrepo. That will keep any addition "external" dependencies from quietly slipping in, and it's going to be rare to add/remove these "external" dependencies.

For oauth2, it just needs to run go get google.golang.org/cloud before building.

[adg]

For oauth2, it just needs to run go get google.golang.org/cloud before building.

Rather than running go get we'd probably want to fetch the dependency manually (there's already some code to do this). That way the build would break if the cloud packages add transitive dependencies.

I know @bradfitz has thoughts on how to address this issue more generally.

[bradfitz]

Now that we have modules (and that https://github.com/gomods/athens exists), this got almost trivial.

@bcmills is adding a go.mod file to oauth2 in https://go-review.googlesource.com/c/oauth2/+/157137 and then I'll run an Athens in Kubernetes:

• https://docs.gomods.io/install/install-on-kubernetes/
• https://hub.docker.com/r/gomods/athens

And then just set GOPROXY when we run go test commands. Super trivial.

[gopherbot]

Change https://golang.org/cl/157438 mentions this issue: cmd/coordinator: support testing subrepos that use modules

[gopherbot]

Change https://golang.org/cl/157439 mentions this issue: cmd/buildlet: ignore symlinks for now

[gopherbot]

Change https://golang.org/cl/165779 mentions this issue: cmd/coordinator: add authenticated proxy to internal module proxy

[gopherbot]

Change https://golang.org/cl/165780 mentions this issue: cmd/coordinator: fix authentication to not require a user token

[gopherbot]

Change https://golang.org/cl/165782 mentions this issue: cmd/buildlet: add localhost proxy to farmer.golang.org's module proxy

[gopherbot]

Change https://golang.org/cl/166218 mentions this issue: dashboard: unify post-submit & trybot policy in builders.go, not coordinator

[bradfitz]

This has been done for a number of months.