-
Notifications
You must be signed in to change notification settings - Fork 149
feat: add FreeBSD support #167
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
Conversation
mitchellh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but we're going to have to do some work so that this works on macOS as well.
|
@mitchellh fixed mach port, all tests are green now |
.github/workflows/test.yml
Outdated
| - name: Install zig | ||
| uses: goto-bus-stop/setup-zig@v2 | ||
| with: | ||
| version: 0.14.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably use Zig 0.14.1 since that's what we use elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... or maybe not. I thought I was reviewing a Ghostty PR...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could bump all jobs, but this might potentially break things for non-FBSD cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do the bump as well.
mitchellh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some changes, most importantly around the mach port state.
Loop wakeups from thread pools on FreeBSD will crash, but Ghostty doesn't use this so I don't think we should block merging on that. But let's fix this, I noted a TODO.
I moved our Zig version to 0.14.1 and CI to Namespace.
.github/workflows/test.yml
Outdated
| - name: Install zig | ||
| uses: goto-bus-stop/setup-zig@v2 | ||
| with: | ||
| version: 0.14.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do the bump as well.
| }; | ||
|
|
||
| /// The struct used for loop wakeup. This is only internal state. | ||
| const Wakeup = if (builtin.os.tag.isDarwin()) struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I extracted our wakeup state to this @charlesrocket. We'll have to implement this using eventfd for FreeBSD for thread pool loop wakeups to work. Until then, we crash.
This isn't used on Ghostty so you didn't run into it.
Take a look at Epoll for examples.
I won't block the merge on this.
| // TODO: error handling | ||
| .freebsd => eventfd( | ||
| 0, | ||
| 0x100000, // EFD_CLOEXEC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@charlesrocket CLOEXEC is very important for this, so I kept to std.posix for Linux and hardcoded the value for FreeBSD.
- [x] Waiting for mitchellh/libxev#167 - [x] Translations - [x] x11 - [x] Wayland - [ ] CI
Fixes #67