Skip to content

Add support for macOS and BSD#72

Merged
DHowett merged 7 commits intomicrosoft:mainfrom
factormystic:macos
May 20, 2025
Merged

Add support for macOS and BSD#72
DHowett merged 7 commits intomicrosoft:mainfrom
factormystic:macos

Conversation

@factormystic
Copy link
Contributor

@factormystic factormystic commented May 19, 2025

Fixes #66. Still doesn't have color in Terminal.app, though.


This PR was vibe coded with VS Code & Copilot Agent mode. It's fine to close if there's a concurrent effort or something more official. I just wanted to demonstrate it could be done. That said, cargo test passes locally 👍

@factormystic
Copy link
Contributor Author

@microsoft-github-policy-service agree

@kmarekspartz
Copy link

Cross-posting here: #30 (comment)

src/sys/unix.rs Outdated
Comment on lines +20 to +22
unsafe extern "C" {
fn __error() -> *mut libc::c_int;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use libc::__error() instead, I believe. Also, this should be similarly abstracted away:

#[cfg(target_os = "macos")]
pub fn get_errno() -> libc::c_int {
    unsafe { *libc::__error() };
}

#[cfg(not(target_os = "macos"))]
pub fn get_errno() -> libc::c_int {
    unsafe { *libc::__errno_location() };
}

Then you can use it in place of the other calls in this file. This function should go to the end of the file, near the other errno related functions.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be changed to io::Error::last_os_error() as the cross platform way to do this (sources: https://users.rust-lang.org/t/how-to-get-errno-value-from-rust-libc/85224/9
https://doc.rust-lang.org/std/io/struct.Error.html#method.last_os_error)
Learned about this elsewhere

Copy link
Member

@lhecker lhecker May 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really nice and also compiles nicely in release mode, but under opt-level=s the last_os_error call doesn't get properly inlined and this leaves quite a lot of code around. I'll see if there's a workaround...

@ProxyCC
Copy link

ProxyCC commented May 20, 2025

If wanted I can pick this PR up tomorrow if needed and finish the latest requested changes.

@warpdesign
Copy link

warpdesign commented May 20, 2025

I confirm this is working with only modifications applied to unix.rs:

Screenshot 2025-05-20 at 11 00 46

Search function doesn't seem to work though:

Screenshot 2025-05-20 at 11 01 03

@lhecker
Copy link
Member

lhecker commented May 20, 2025

@factormystic I pushed a few commits into your PR. It'd be nice if you could check them out.

It'd be really kind if someone could test these latest changes.

@warpdesign
Copy link

Works for me (macOS/ARM64). Search function still doesn't work: complains about missing ICU library.

@lhecker lhecker requested review from DHowett and removed request for sarvagnan May 20, 2025 15:44
@factormystic
Copy link
Contributor Author

I pulled your changes and it seems to work fine for me on macOS 👍

@lhecker
Copy link
Member

lhecker commented May 20, 2025

The issue with ICU was also discussed in #52. I've internally requested a Mac for testing this myself, but I don't live in the US so it may be difficult to get one loaned out.

@reynoldskr
Copy link

You should be able to link in icu components if you install libicu from brew

@reynoldskr reynoldskr mentioned this pull request May 20, 2025
@IoIxD IoIxD mentioned this pull request May 20, 2025
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks!

@DHowett DHowett changed the title Add support for macOS Add support for macOS ad BSD May 20, 2025
@DHowett DHowett changed the title Add support for macOS ad BSD Add support for macOS and BSD May 20, 2025
@DHowett
Copy link
Member

DHowett commented May 20, 2025

As a treat, here it is on netbsd

image

@DHowett DHowett merged commit 1ce0716 into microsoft:main May 20, 2025
1 check passed
diabloproject pushed a commit to diabloproject/edit that referenced this pull request May 29, 2025
Fixes microsoft#66
Fixes microsoft#124

Still doesn't have color in Terminal.app on macOS, though.

Co-authored-by: Leonard Hecker <leonard@hecker.io>
@RossSmyth RossSmyth mentioned this pull request Jun 8, 2025
13 tasks
@factormystic factormystic deleted the macos branch December 20, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

macOS build support

8 participants