Skip to content
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.

Sign up for GitHub

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

Jump to bottom

x/net: drop unused variables and consts that x/sys/unix has #54035

Open
mengzhuo opened this issue Jul 25, 2022 · 3 comments
Open

x/net: drop unused variables and consts that x/sys/unix has #54035

mengzhuo opened this issue Jul 25, 2022 · 3 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Unreleased

Comments

@mengzhuo
Copy link
Contributor

This is a tracking issue for Ian's request and I quoted here.

In general, where possible, we should be moving this package to using constants defined in x/sys/unix, rather than using its own constants.

In this case as far as I can see the only of the CTL constants that is referenced is CTL_NET, by route.go. I think we should just drop all of the sysCTL constants entirely and change route.go to use unix.CTL_NET.

https://go-review.googlesource.com/c/net/+/419015/comments/6790299e_41c7ad76

I've upload CL for FreeBSD and compiled successful with other OSes by GOOS={darwin,netbsd,dragonfly,openbsd}

cc @golang/netbsd @golang/dragonfly @golang/openbsd @golang/netbsd
@gopherbot gopherbot added this to the Unreleased milestone Jul 25, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/419180 mentions this issue: route: drops const that x/unix has

@cherrymui cherrymui changed the title x/net: drop unused variables and consts that x/unix has x/net: drop unused variables and consts that x/sys/unix has Jul 25, 2022
@cherrymui cherrymui added NeedsFix The path to resolution is known, but the work has not been done. help wanted labels Jul 25, 2022
gopherbot pushed a commit to golang/net that referenced this issue Jul 28, 2022
@mengzhuo @gopherbot
route: drops const that x/unix has
41545e8 
For golang/go#54035

Change-Id: I5163bb1d562f0c7f7de16995fcb42ece894d07bf
Reviewed-on: https://go-review.googlesource.com/c/net/+/419180
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Run-TryBot: Meng Zhuo <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/419937 mentions this issue: route: remove RTM_LOCK on openbsd

gopherbot pushed a commit to golang/net that referenced this issue Jul 28, 2022
@tklauser @gopherbot
route: remove RTM_LOCK on openbsd
f92ba40 
RTM_LOCK is no longer defined on OpenBSD as of version 6.4, see
https://man.openbsd.org/OpenBSD-6.4/route.4 and
https://marc.info/?l=openbsd-tech&m=153018811429193&w=2

CL 419180 broke the build on openbsd/arm64 and openbsd/mips64 due to
RTM_LOCK not being defined on these platforms because these ports were
based on OpenBSD versions newer than 6.4. Earlier OpenBSD versions are
no longer supported since Go 1.15 per https://golang.org/wiki/OpenBSD

For golang/go#54035

Change-Id: I476a60ab2a082c7ddc763b5dbaf279ba47dcc1e1
Reviewed-on: https://go-review.googlesource.com/c/net/+/419937
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Tobias Klauser <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Run-TryBot: Tobias Klauser <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
@dmitshur
Copy link
Contributor

dmitshur commented Aug 4, 2022 •
edited
Loading

For x/net packages that are vendored into std, this appears to be incompatible with the policy set in #32102 that std shouldn't vendor golang.org/x/sys/unix. See #54259.

CC @ianlancetaylor.

@dmitshur dmitshur added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Aug 4, 2022
@dmitshur dmitshur mentioned this issue Aug 5, 2022
Closed
WeiminShang added a commit to WeiminShang/net that referenced this issue Nov 16, 2022
@WeiminShang @gopherbot
route: drops const that x/unix has
1c96223 
For golang/go#54035

Change-Id: I5163bb1d562f0c7f7de16995fcb42ece894d07bf
Reviewed-on: https://go-review.googlesource.com/c/net/+/419180
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Run-TryBot: Meng Zhuo <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
WeiminShang added a commit to WeiminShang/net that referenced this issue Nov 16, 2022
@WeiminShang @gopherbot
route: remove RTM_LOCK on openbsd
4196d10 
RTM_LOCK is no longer defined on OpenBSD as of version 6.4, see
https://man.openbsd.org/OpenBSD-6.4/route.4 and
https://marc.info/?l=openbsd-tech&m=153018811429193&w=2

CL 419180 broke the build on openbsd/arm64 and openbsd/mips64 due to
RTM_LOCK not being defined on these platforms because these ports were
based on OpenBSD versions newer than 6.4. Earlier OpenBSD versions are
no longer supported since Go 1.15 per https://golang.org/wiki/OpenBSD

For golang/go#54035

Change-Id: I476a60ab2a082c7ddc763b5dbaf279ba47dcc1e1
Reviewed-on: https://go-review.googlesource.com/c/net/+/419937
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Tobias Klauser <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Run-TryBot: Tobias Klauser <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@mengzhuo @dmitshur @gopherbot @cherrymui