x/net/trace: use std context package #20056

Closed
gyuho opened this Issue Apr 20, 2017 · 8 comments

Comments

Projects
None yet
6 participants
@gyuho
Contributor

gyuho commented Apr 20, 2017

Can we have context here https://github.com/golang/net/blob/master/trace/trace.go#L80?

This golang/time@c06e80d happened. Would the same change in trace break anything?

It's one of our sub-dependency and would like to know when this would happen :)

Thanks in advance!

@gyuho gyuho referenced this issue in coreos/etcd Apr 20, 2017

Closed

*: use Go 1.9+ #6174

21 of 26 tasks complete

@bradfitz bradfitz changed the title from context: 'net/trace' still imports 'golang.org/x/net/context' to x/net/trace: use std context package Apr 20, 2017

@gopherbot gopherbot added this to the Unreleased milestone Apr 20, 2017

@gopherbot

This comment has been minimized.

Show comment
Hide comment

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

@dfawley

This comment has been minimized.

Show comment
Hide comment
@dfawley

dfawley Apr 21, 2017

This is a cascading problem. grpc-go is now broken since this package is a dependency of ours and we support Go 1.6. We are blocked on deprecating 1.6 and updating our imports until generated proto files use the standard context package. AppEngine only supports Go1.6, so I believe the proto compiler can't be changed until that is updated.

Is it possible to revert these context changes and hold off on them until Go1.6 is less widely used?

dfawley commented Apr 21, 2017

This is a cascading problem. grpc-go is now broken since this package is a dependency of ours and we support Go 1.6. We are blocked on deprecating 1.6 and updating our imports until generated proto files use the standard context package. AppEngine only supports Go1.6, so I believe the proto compiler can't be changed until that is updated.

Is it possible to revert these context changes and hold off on them until Go1.6 is less widely used?

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Apr 21, 2017

Member

@dfawley, where is the grpc-go breakage? Got some URLs?

Member

bradfitz commented Apr 21, 2017

@dfawley, where is the grpc-go breakage? Got some URLs?

@dfawley

This comment has been minimized.

Show comment
Hide comment
@dfawley

dfawley Apr 21, 2017

Our tests only run on PRs and merges, so the main page doesn't show this breakage. Here's an example from an in-progress PR:

https://travis-ci.org/grpc/grpc-go/jobs/224274230#L215

dfawley commented Apr 21, 2017

Our tests only run on PRs and merges, so the main page doesn't show this breakage. Here's an example from an in-progress PR:

https://travis-ci.org/grpc/grpc-go/jobs/224274230#L215

@broady

This comment has been minimized.

Show comment
Hide comment
@broady

broady Apr 21, 2017

Member

+1 on being affected by breakage. This breaks Google Cloud client libraries using gRPC on App Engine standard, which is stuck on Go 1.6.

Member

broady commented Apr 21, 2017

+1 on being affected by breakage. This breaks Google Cloud client libraries using gRPC on App Engine standard, which is stuck on Go 1.6.

@menghanl menghanl referenced this issue in grpc/grpc-go Apr 21, 2017

Merged

temporarily disable 1.6 on travis #1198

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Apr 21, 2017

Member

It's been 14 months since Go 1.6. App Engine can't get another release out? Is that really what this is all about?

Go's policy is that we only test and maintain the past two releases. I could add some build tag chicanery into x/net/trace, but our builders aren't testing 1.6 anymore. I don't want to revert the change. We want to make forward progress on cleanups and other people want us to use std context.

Let me see what I can do.

Member

bradfitz commented Apr 21, 2017

It's been 14 months since Go 1.6. App Engine can't get another release out? Is that really what this is all about?

Go's policy is that we only test and maintain the past two releases. I could add some build tag chicanery into x/net/trace, but our builders aren't testing 1.6 anymore. I don't want to revert the change. We want to make forward progress on cleanups and other people want us to use std context.

Let me see what I can do.

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@scottma

This comment has been minimized.

Show comment
Hide comment
@scottma

scottma Apr 23, 2017

yak, getting this error now after this change .

golang.org/x/net/trace/trace.go:67: can't find import: "context"

scottma commented Apr 23, 2017

yak, getting this error now after this change .

golang.org/x/net/trace/trace.go:67: can't find import: "context"

gopherbot pushed a commit to golang/net that referenced this issue Apr 23, 2017

trace: support Go 1.6 again
App Engine is stuck 14 months in the past and doesn't have context.

Updates golang/go#20056

Change-Id: I2cb2a3a75af07728805be8c714e4725f01a17074
Reviewed-on: https://go-review.googlesource.com/41413
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

@dfawley dfawley referenced this issue in grpc/grpc-go Oct 13, 2017

Closed

Add back support for Go 1.6 #1575

@golang golang locked and limited conversation to collaborators Apr 23, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.