-
Notifications
You must be signed in to change notification settings - Fork 17.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/net/lif, x/net/route: as long as std imports these packages, they can't import golang.org/x/sys/unix because of standard library policy #54259
Comments
Next up:
Reverting CL 419180 is enough to resolve this and avoid x/sys/unix getting vendored into std. |
Change https://go.dev/cl/421334 mentions this issue: |
Go 1.20 development is just beginning. This is a time to update all golang.org/x/... module versions that contribute packages to the std and cmd modules in the standard library to latest master versions. This CL holds back some of the available updates to the x/net module due to go.dev/issue/54259. It'll be updated in a later separate pass. x/tools is also held back a bit to avoid pulling in too new of x/net. For #36905. For #53812. Updates #54259. Change-Id: Iaefe6a343a02cc5ceb85c15125882d64dd372627 Reviewed-on: https://go-review.googlesource.com/c/go/+/421334 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
It may be possible to use syscall rather than x/sys/unix for these constants. |
Change https://go.dev/cl/421419 mentions this issue: |
It happens that everything we need is already defined in syscall. This avoids problems when this package is vendored into the standard library. For golang/go#54259 Change-Id: I999eba5d089a1dfb341e27ebf3651ace0de26947 Reviewed-on: https://go-review.googlesource.com/c/net/+/421419 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Change https://go.dev/cl/421425 mentions this issue: |
It happens that everything we need is already defined in syscall. This avoids problems when this package is vendored into the standard library. For golang/go#54259 Change-Id: I86bfe44f20c9db2ecfdb8dbb2ef798391da2bfa6 Reviewed-on: https://go-review.googlesource.com/c/net/+/421425 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Thanks Ian for finding a resolution to this issue that's better than reverting those earlier x/net CLs. I'm glad I left x/net out of CL 421334 to create an opportunity to take time to think about this (without the pressure of #53812). I've sent CL 421461 to vendor the latest x/net in the main repo, and will close this now because I don't think there's anything left to do. I've considered adding a test to x/net (also suggested by @prattmic), but such a test would either need to hard-code which x/net packages are vendored in std, or extract that dynamically somehow which is probably better suited for #48523. If this happens more often we can revisit it. (If someone has an idea for a better test, please feel free to file an issue.) |
Change https://go.dev/cl/421461 mentions this issue: |
Sorry, I wasn't aware of the policy #32102 when I sent these CLs. Thanks Ian for fixing these! |
CL 421334 updated most of golang.org/x dependencies at the start of the Go 1.20 development cycle. This CL updates x/net and x/tools as well, now that go.dev/issue/54259 is resolved. For #36905. Updates #54259. Change-Id: Ie422b71cba060a4774076eebf3b499cda1150367 Reviewed-on: https://go-review.googlesource.com/c/go/+/421461 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Go 1.20 development is just beginning. This is a time to update all golang.org/x/... module versions that contribute packages to the std and cmd modules in the standard library to latest master versions. This CL holds back some of the available updates to the x/net module due to go.dev/issue/54259. It'll be updated in a later separate pass. x/tools is also held back a bit to avoid pulling in too new of x/net. For golang#36905. For golang#53812. Updates golang#54259. Change-Id: Iaefe6a343a02cc5ceb85c15125882d64dd372627 Reviewed-on: https://go-review.googlesource.com/c/go/+/421334 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
CL 421334 updated most of golang.org/x dependencies at the start of the Go 1.20 development cycle. This CL updates x/net and x/tools as well, now that go.dev/issue/54259 is resolved. For golang#36905. Updates golang#54259. Change-Id: Ie422b71cba060a4774076eebf3b499cda1150367 Reviewed-on: https://go-review.googlesource.com/c/go/+/421461 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
It happens that everything we need is already defined in syscall. This avoids problems when this package is vendored into the standard library. For golang/go#54259 Change-Id: I999eba5d089a1dfb341e27ebf3651ace0de26947 Reviewed-on: https://go-review.googlesource.com/c/net/+/421419 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
It happens that everything we need is already defined in syscall. This avoids problems when this package is vendored into the standard library. For golang/go#54259 Change-Id: I86bfe44f20c9db2ecfdb8dbb2ef798391da2bfa6 Reviewed-on: https://go-review.googlesource.com/c/net/+/421425 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Updating to latest golang.org/x dependencies causes golang.org/x/sys/unix to be vendored into std, which goes against the policy set in #32102:
It's happening because the golang.org/x/net/lif package (vendored into std) has been updated to import and depend on golang.org/x/sys/unix in CL 413275 and CL 414994:
To make progress on #36905, I will likely need to send CLs to revert those golang.org/x/net/lif simplification CLs.
Package
net
seems to only usegolang.org/x/net/lif
in one function callinterfaceAddrTable
, so a path that can be investigated in the future is whether removing that dependency can be simpler overall.CC @tklauser, @ianlancetaylor, @bcmills.
The text was updated successfully, but these errors were encountered: