-
Notifications
You must be signed in to change notification settings - Fork 2
Support mac os m architecture #140
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
base: vara-dev
Are you sure you want to change the base?
Changes from 3 commits
13c9cb6
9be219e
e6dbe63
1d5dfec
2dcea09
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,9 +20,35 @@ set(CMAKE_CXX_EXTENSIONS OFF) | |
| set(CMAKE_EXPORT_COMPILE_COMMANDS YES) | ||
|
|
||
| if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") | ||
| add_compile_options(-stdlib=libstdc++) | ||
| if(APPLE) | ||
| # Clang on macOS must use libc++ | ||
| add_compile_options(-stdlib=libc++) | ||
| elseif(UNIX AND NOT APPLE) | ||
| # Clang on Linux can use libstdc++ (usually default) No need to explicitly | ||
| # add this unless required | ||
| add_compile_options(-stdlib=libstdc++) | ||
| endif() | ||
| endif() | ||
|
|
||
| # if(APPLE) execute_process( COMMAND xcrun --show-sdk-path OUTPUT_VARIABLE | ||
| # MACOS_SDK_PATH OUTPUT_STRIP_TRAILING_WHITESPACE ) | ||
| # include_directories("${MACOS_SDK_PATH}/usr/include/c++/v1") endif() | ||
Kallistos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| if(APPLE) | ||
| if(NOT DEFINED CMAKE_OSX_ARCHITECTURES) | ||
| set(CMAKE_OSX_ARCHITECTURES | ||
| "arm64" | ||
| CACHE STRING "" FORCE | ||
| ) | ||
| endif() | ||
| endif() | ||
|
|
||
| if(APPLE) | ||
| set(Python3_ROOT_DIR "/opt/homebrew") # cmake-lint: disable=C0103 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh boy, someone should fix the cmake find package for python 😆 or why is it necessary to specify the actual root beforehand where it's not on other systems? |
||
| endif() | ||
|
|
||
| find_package(Python3 REQUIRED COMPONENTS Interpreter Development) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this always necessary here? If I remember correctly, we only searched for python where we needed it for the bindings but I'm not sure if we did this our selfs or if we just used what pybind found.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? |
||
|
|
||
| # VARA_FEATURE options | ||
| option(VARA_FEATURE_COLORED_OUTPUT "Produce ANSI-colored output" TRUE) | ||
| if(${VARA_FEATURE_COLORED_OUTPUT}) | ||
|
|
@@ -150,11 +176,27 @@ if(NOT VARA_FEATURE_IN_TREE) | |
| # Only search for LLVM if we build out of tree | ||
| if(DEFINED LLVM_REQUESTED_VERSION) | ||
| message(STATUS "Using LLVM ${LLVM_REQUESTED_VERSION}") | ||
| find_package(LLVM ${LLVM_REQUESTED_VERSION} REQUIRED CONFIG) | ||
| find_package( | ||
| LLVM | ||
| ${LLVM_REQUESTED_VERSION} | ||
| REQUIRED | ||
| CONFIG | ||
| HINTS | ||
| "/opt/homebrew/opt/llvm/lib/cmake/llvm" # macOS ARM (Homebrew) | ||
| "/usr/lib/llvm-${LLVM_REQUESTED_VERSION}/lib/cmake/llvm" # Linux (Debian) | ||
| "/usr/lib/llvm/lib/cmake/llvm" # Generic fallback | ||
| ) | ||
| else() | ||
| message(STATUS "Using LLVM (default)") | ||
| set(CMAKE_FIND_PACKAGE_SORT_ORDER NATURAL) | ||
| find_package(LLVM REQUIRED CONFIG) | ||
| find_package( | ||
| LLVM | ||
| REQUIRED | ||
| CONFIG | ||
| HINTS | ||
| "/opt/homebrew/opt/llvm/lib/cmake/llvm" | ||
| "/usr/lib/llvm/lib/cmake/llvm" | ||
| ) | ||
| endif() | ||
|
|
||
| if(NOT "${LLVM_VERSION_MAJOR}" GREATER_EQUAL "${MIN_LLVM_REQUIRED}") | ||
|
|
||
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.
Could you make this optional or maybe don't set it at all, as it is currently not necessary to be specified for UNIX systems where the system default gets chosen.
PS: I compile with libc++ on some UNIX setup 🥲
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.
Yes, I do not need it, was not sure if that should be in it.
Should I remove it or what would be the way to make it optional to be helpful for Linux?
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.
There are two options, either remove the line for unix and only keep it for apple or introduce a cmake options that let's one "choose" one of the two libs. something like
VARA_FEATURE_USE_STD_LIBwhere one then can insert the lib wanted.