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.

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

Fixed nix build for macos. #1914

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

diasbruno
Copy link

I'm not an expert on nix, so I'd appreciate some help with this.

On macOS, the nix was failing because it was finding the llvm 16 on the nix hash.

I updated the flake, and added the missing piece "compiler-rt" to find the rest of the runtime libraries.

This hash pushes llvm to version 19.

@diasbruno diasbruno force-pushed the fixed-nix-build-for-macos-darwin branch from d7166e5 to a5af82b Compare January 31, 2025 19:21
@diasbruno
Copy link
Author

diasbruno commented Jan 31, 2025

I tested on a Ubuntu x86_64 on WSL and it compiled and run ok.

@lerno
Copy link
Collaborator

lerno commented Feb 2, 2025

This one looks wrong. You'd adding the configurable path yes, but you don't default to the LLVM one, so any build outside of nix breaks.

@diasbruno
Copy link
Author

You are right. I forgot to build with the local llvm. 🤦‍♂️

@diasbruno diasbruno force-pushed the fixed-nix-build-for-macos-darwin branch from a5af82b to 25b0474 Compare February 3, 2025 00:25
@lerno
Copy link
Collaborator

lerno commented Feb 4, 2025

Wouldn't it make more sense to make the override to change from the default if the override is set to a value, rather than have it depend on nix?

So "if LLVM_CRT_LIBRARY_DIR is empty string, use the default, otherwise use the code with LLVM_CRT_LIBRARY_DIR"

@diasbruno
Copy link
Author

This seems to be Apple only so I decided to only set on this if. But I'm ok setting as default.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@lerno
Copy link
Collaborator

lerno commented Feb 5, 2025

I meant to get rid of the C3_IS_NIX_BUILD entirely and only use LLVM_CRT_LIBRARY_DIR if it is set to a non-empty value.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
nix/default.nix Outdated Show resolved Hide resolved
@lerno
Copy link
Collaborator

lerno commented Feb 5, 2025

Hmm.. that failed

@lerno
Copy link
Collaborator

lerno commented Feb 5, 2025

Maybe this is better:

option(LLVM_CRT_LIBRARY_DIR "Use custom llvm's compiler-rt directory" "")
...
find_file(RT_ASAN_DYNAMIC NAMES libclang_rt.asan_osx_dynamic.dylib PATHS ${LLVM_CRT_LIBRARY_DIR} "${LLVM_LIBRARY_DIR}/clang/${LLVM_MAJOR_VERSION}/lib/darwin")
find_file(RT_TSAN_DYNAMIC NAMES libclang_rt.tsan_osx_dynamic.dylib PATHS ${LLVM_CRT_LIBRARY_DIR} "${LLVM_LIBRARY_DIR}/clang/${LLVM_MAJOR_VERSION}/lib/darwin")
find_file(RT_UBSAN_DYNAMIC NAMES libclang_rt.ubsan_osx_dynamic.dylib PATHS ${LLVM_CRT_LIBRARY_DIR} "${LLVM_LIBRARY_DIR}/clang/${LLVM_MAJOR_VERSION}/lib/darwin")
find_file(RT_LSAN_DYNAMIC NAMES libclang_rt.lsan_osx_dynamic.dylib PATHS ${LLVM_CRT_LIBRARY_DIR} "${LLVM_LIBRARY_DIR}/clang/${LLVM_MAJOR_VERSION}/lib/darwin")

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.

2 participants