Go: x/sync/errgroup: should use context.Context from stdlib

[sbinet]

The x/sync/errgroup/errgroup.go file reads:

// Package errgroup provides synchronization, error propagation, and Context
// cancelation for groups of goroutines working on subtasks of a common task.
package errgroup

import (
    "sync"

    "golang.org/x/net/context"
)

// [...]

// WithContext returns a new Group and an associated Context derived from ctx.
//
// The derived Context is canceled the first time a function passed to Go
// returns a non-nil error or the first time Wait returns, whichever occurs
// first.
func WithContext(ctx context.Context) (*Group, context.Context) {
    ctx, cancel := context.WithCancel(ctx)
    return &Group{cancel: cancel}, ctx
}

it should probably import and use "context" instead of "golang.org/x/net/context" (once AppEngine jumps to Go1.8)

[bradfitz]

We'll do this when Go 1.9 is released. (We generally try to support the past few Go releases in the golang.org/x/* repos)

[skabbes]

I ran into this today on Go 1.9, any chance to get this fixed?

[ianlancetaylor]

Yes, I think we can make this change now. Want to send a patch?

[skabbes]

What are the compatibility requirements, should I use conditional compilation? or can I simply just use "context". Additionally, where is the repository for this? I suppose I need to set up gerrit as a go contributor, but is the x/sync packaged hosted somewhere else?

[ianlancetaylor]

I think we can just use "context". At least, we can see if anyone complains.

You can fetch the package via go get golang.org/x/sync. That will give you a git repo that should work with Gerrit.

[skabbes]

First go commit, I think I did everything correctly. Changeset here:

https://go-review.googlesource.com/c/sync/+/66651

[gopherbot]

Change https://golang.org/cl/84481 mentions this issue: x/sync/errgroup, x/sync/semaphore: use std context instead of x/net/context

[furdarius]

https://golang.org/cl/84481 replace context without complexity. So, it can be merged when App Engine will move to Go1.8 ("_the policy they decided upon is to wait until January 2nd_" - by Joe Tsai)

[agnivade]

Update on the issue from the CL thread. If anyone wants to follow -

the App Engine team at Google has decided that Go 1.6 users need to be supported for longer, so the decision was made to continue to use golang.org/x/net/context for some time in all the golang.org/x/* (non-Google) repos. We don't have test coverage of them at https://build.golang.org/, but we're relying on them being tested via other testing dashboards.

It'd be nice if the App Engine team made an official statement somewhere about the policy Go now needs to follow, even if it's not tested.

[agnivade]

@jba - Should this be pushed to 1.13 ? Doesn't look like there has been any change to AppEngine policy.

[jba]

AppEngine has changed. This can be fixed now.

[agnivade]

Great, thanks !

[agnivade]

This is done now. The CL was just marked as merged as the change was already in effect.