build: breaking change in go-github

[kevinburke]

The go-github repository took the opportunity to add context.Context parameters to all API calls that go over the network. Several of the packages in x/build depend on go-github:

• x/build/devapp
• x/build/godash
• x/build/cmd/godash
• x/build/maintner
• x/build/cmd/issuelock

CL 37293 adds Context support.

I'm not sure this is enough, however, since the new go-github can only run on 1.7 or higher, and I'm pretty sure this rules out using Google App Engine. People might also run into problems compiling the library if they have an older version of go-github (or a newer version, with an older version of x/build). We could vendor the old version in the x/build repository to maintain maximum compatibility.

[dmitshur]

What's the version support policy for x/build? According to #17626:

What's the general policy for how many versions back we maintain support on a given subrepo?

In general, the two most recent.

But I'm not sure if that applies directly to x/build since it's a little meta.

If x/build needs to run on 1.6 and older, then vendoring version 2 of go-github (parent commit of google/go-github@796decd) would be necessary.

[kevinburke]

I'm not sure there is one, but:

• The stuff in x/build/cmd is probably "the two most recent Go versions," so fine to use context there.

• dev.golang.org still runs on App Engine, which as of today, still runs on Go 1.6.2. https://cloud.google.com/appengine/docs/go/release-notes. I've been working on fixing dev.golang.org to work outside of App Engine but there are still some CL's outstanding, and I don't have access to the dev.golang.org server to be able to update it.

[bradfitz]

We can move https://dev.golang.org to https://cloud.google.com/appengine/docs/flexible/ which allows any arbitrary code, including any version of Go we want.

[gopherbot]

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

[kevinburke]

Can we reopen this? I believe we can't deploy dev.golang.org, since App Engine is still stuck on Go 1.6.

I'm working on a CL to update the devapp codebase to work with Appengine Flex, which apparently runs Go 1.8, and should hopefully fix the issue.

[kevinburke]

cc @quentinmit

[kevinburke]

Though, sigh, it sounds like "flex" is more like EC2's spot instances and less like a continuously running server.

From https://cloud.google.com/appengine/docs/flexible/go/flexible-for-standard-users:

The flexible environment is intended to be complementary to the standard environment. If you have an existing application running in the standard environment, it’s not usually necessary to migrate the entire application to the flexible environment. Instead, identify the parts of your application that require more CPU, more RAM, a specialized third-party library or program, or that need to perform actions that aren’t possible in the standard environment. Once you’ve identified these parts of your application, create small App Engine services that use the flexible environment to handle just those parts. Your existing service running in the standard environment can call the other services using HTTP, Task Queues, or Cloud Pub/Sub.

Are there any App Engine developers that contribute to the Go codebase? What are the plans for Go 1.8 on App Engine?

Alternatively we could work harder to deploy dev.golang.org on Kubernetes or similar. CL https://go-review.googlesource.com/c/36319/ attempts to load the devapp's static assets outside of an App Engine context.

[bradfitz]

App Engine Classic will update to Go 1.8 soon enough. But we should migrate to Flex sooner.

[quentinmit]

It was a tiny patch to go-github to get it to build on 1.6, so I just did that. Devapp is updated and redeployed.

[dmitshur]

It was a tiny patch to go-github to get it to build on 1.6, so I just did that.

@quentinmit, is that patch public, if so, can you point me to it please?

I ask because we have a PR in go-github that attempts to add compatibility for app engine, but currently isn't successful. I'm not familiar with app engine so I can't help it further. If your fix is a general one, it'd be nice to get it upstream. (If the fix isn't general, then that's fine.)

[kevinburke]

@shurcooL, I'm guessing it was "replace all instances of "context" with "golang.org/x/net/context", and ignore/remove the req.WithContext calls.

[dmitshur]

That's my guess too, but given he said "a tiny patch", I was hoping it might be something else that I overlooked.

[quentinmit]

@shurcooL Yes, @kevinburke is correct. I sed'd the import path to golang.org/x/net/context (which still works just fine on Go 1.8), and used build tags to hide the handful of req.WithContext and req.Context calls. I can send you a PR if you want for that?

[dmitshur]

@quentinmit Thanks for confirming.

I can send you a PR if you want for that?

We already have PR google/go-github#582 that does something like that, so there's no need. I just wanted to see if there was a different approach. Thanks!