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/sys/unix: migrate some definitions from illumos files to solaris files #56000

Closed
nshalman opened this issue Oct 2, 2022 · 3 comments
Closed

x/sys/unix: migrate some definitions from illumos files to solaris files #56000

nshalman opened this issue Oct 2, 2022 · 3 comments
Assignees
@nshalman
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Solaris
Milestone
Backlog

Comments

@nshalman
Copy link

nshalman commented Oct 2, 2022

I have reports from Twitter (see below) that many of my improvements that I've placed in illumos files also work on Solaris systems
(which isn't particularly surprising given how old these APIs are... (the fact that the binaries worked means that even the tuntap driver ioctls match.)

I propose moving these changes from the illumos files to the solaris files. The changes is effectively a noop for illumos and opens up this code to Solaris systems.

I will provide a CR with the work completed. Most of the changes include tests, so between API stability, user test reports and CI builds, if the CI builds pass, the moves I'm proposing should all be safe for Solaris.

Reference Tweets

@nahumshalman If the Lifreq and STREAMS bits are copied into unix/syscall_solaris.go in https://t.co/6brPrhlwMo, Your wireguard-go builds and runs on Solaris 11.4.

— Greg Onufer (@gonufer) September 22, 2022

tailscale v1.30.2-illumos builds on Solaris 11.4 by adding Getsid to x.sys (as in your go-review changes) and “solaris” added to all your “illumos” runtime.GOOS checks, and solaris copies of *illumos.go files.

— Greg Onufer (@gonufer) September 22, 2022

…not only builds, but was also able to connect to tailscale (after tailscaled exited due to watchdog once and was restarted).

Whoa!

— Greg Onufer (@gonufer) September 22, 2022
@gopherbot gopherbot added this to the Proposal milestone Oct 2, 2022
nshalman added a commit to nshalman/sys that referenced this issue Oct 2, 2022
@nshalman
unix: migrate some illumos defintions to solaris
e7f8d0e 
Fixes golang/go#56000

Change-Id: Ica6549385ddb466c4697cc45851e2b59fd6e0f24
@gopherbot
Copy link

Change https://go.dev/cl/437357 mentions this issue: unix: migrate some illumos defintions to solaris

@ianlancetaylor ianlancetaylor changed the title proposal: x/sys/unix: migrate some definitions from illumos files to solaris files x/sys/unix: migrate some definitions from illumos files to solaris files Oct 3, 2022
@ianlancetaylor ianlancetaylor added OS-Solaris NeedsFix The path to resolution is known, but the work has not been done. and removed Proposal labels Oct 3, 2022
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Backlog Oct 3, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 3, 2022
@ianlancetaylor
Copy link
Contributor

This doesn't have to be a proposal, so taking it out of the proposal process. It's always fine to add support in x/sys/unix for simple system call wrappers. Proposals are only required for more complex cases.

@nshalman
Copy link
Author

nshalman commented Oct 3, 2022

This doesn't have to be a proposal, so taking it out of the proposal process. It's always fine to add support in x/sys/unix for simple system call wrappers. Proposals are only required for more complex cases.

Thank you. This felt worth filing some sort of ticket to hold details that don't need to be in the comments or the commit message and I wasn't quite sure which type to file.

nshalman added a commit to nshalman/sys that referenced this issue Oct 6, 2022
@nshalman
unix: migrate some illumos definitions to solaris
8b0bcf9 
Fixes golang/go#56000

Change-Id: Ica6549385ddb466c4697cc45851e2b59fd6e0f24
@golang golang locked and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@nshalman nshalman

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Solaris
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@nshalman @ianlancetaylor @gopherbot