Add macOS support #11
Conversation
This allows OpenVAF to compile and run on macOS. The following changes were made: - Use clang as linker on macOS instead of ld - Change -dylib to -dynamiclib for clang compatibility - Add build.rs to dynamically detect LLVM library path via llvm-config - Add rpath to find libLLVM.dylib at runtime Tested on macOS with MacPorts LLVM 18 on Apple Silicon.
Summary of ChangesHello @TBird2001, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces essential modifications to enable OpenVAF to successfully compile and execute on macOS. The changes primarily focus on adapting the linking process to the macOS environment, including switching to the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully introduces macOS support for OpenVAF. The changes correctly configure the linker to use clang on macOS and adjust the dynamic library flag to -dynamiclib for compatibility. Additionally, the new build.rs script effectively handles the dynamic detection and setup of LLVM library rpaths, which is essential for proper linking and execution on macOS. The overall approach is sound and addresses the stated goal.
| if let Ok(output) = std::process::Command::new("llvm-config") | ||
| .arg("--libdir") | ||
| .output() | ||
| { | ||
| if output.status.success() { | ||
| let libdir = String::from_utf8_lossy(&output.stdout).trim().to_string(); | ||
| println!("cargo:rustc-link-arg=-Wl,-rpath,{}", libdir); | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider adding a warning or error message if llvm-config fails to execute or returns a non-successful status. Currently, if llvm-config is not found or fails, the rpath is silently skipped, which might lead to confusing runtime errors later. Explicit feedback here would improve debuggability for users, especially during initial setup or when LLVM is not correctly configured on their system.
|
hmm I must update the mob branch to the arpad master one since that one is more up to date.. however i beleive they have done mac os support there already https://github.com/arpadbuermen/OpenVAF |
|
Ah, interesting. I didn't appreciate that the arpad repo was the most active one. Their macOS support isn't quite right (as best I can tell). I'll put in a similar pull request for that repo. |
2 participants
This allows OpenVAF to compile and run on macOS. The following changes were made:
Tested on macOS with MacPorts LLVM 18 on Apple Silicon.