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.

Sign up for GitHub

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

Jump to bottom

Fix MacOS build on 11.x SDK and Catalyst build #54506

Merged
merged 6 commits into from
Jun 27, 2021
Merged

Fix MacOS build on 11.x SDK and Catalyst build #54506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
efd0663
Fix MacOS build on 11.x SDK and Catalyst build
janvorli Jun 21, 2021
edfc263
Add comment explaining why we set the macOS options via CMAKE_XXX_FLAGS
janvorli Jun 21, 2021
581ca38
Add diagnostic printing of the configuration
janvorli Jun 25, 2021
2bc930c
May fix the problem
janvorli Jun 25, 2021
7ab601f
Hopefully the right fix
janvorli Jun 25, 2021
c1ca06f
Attempt to fix the coreclr issue
janvorli Jun 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions eng/native/configurecompiler.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,30 +393,33 @@ if (CLR_CMAKE_HOST_UNIX)
# replaced with a default value, and always gets expanded to an OS version.
# https://gitlab.kitware.com/cmake/cmake/-/issues/20132
# We need to disable the warning that -tagret replaces -mmacosx-version-min
add_compile_options(-Wno-overriding-t-option)
set(DISABLE_OVERRIDING_MIN_VERSION_ERROR -Wno-overriding-t-option)
add_link_options(-Wno-overriding-t-option)
if(CLR_CMAKE_HOST_ARCH_ARM64)
add_compile_options(-target arm64-apple-ios14.2-macabi)
set(MACOS_VERSION_MIN_FLAGS "-target arm64-apple-ios14.2-macabi")
add_link_options(-target arm64-apple-ios14.2-macabi)
elseif(CLR_CMAKE_HOST_ARCH_AMD64)
add_compile_options(-target x86_64-apple-ios13.5-macabi)
set(MACOS_VERSION_MIN_FLAGS "-target x86_64-apple-ios13.5-macabi")
add_link_options(-target x86_64-apple-ios13.5-macabi)
else()
clr_unknown_arch()
endif()
# These options are intentionally set using the CMAKE_XXX_FLAGS instead of
# add_compile_options so that they take effect on the configuration functions
# in various configure.cmake files.
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${MACOS_VERSION_MIN_FLAGS} ${DISABLE_OVERRIDING_MIN_VERSION_ERROR}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${MACOS_VERSION_MIN_FLAGS} ${DISABLE_OVERRIDING_MIN_VERSION_ERROR}")
set(CMAKE_ASM_FLAGS "${CMAKE_ASM_FLAGS} ${MACOS_VERSION_MIN_FLAGS} ${DISABLE_OVERRIDING_MIN_VERSION_ERROR}")
else()
if(CLR_CMAKE_HOST_ARCH_ARM64)
# 'pthread_jit_write_protect_np' is only available on macOS 11.0 or newer
set(MACOS_VERSION_MIN_FLAGS -mmacosx-version-min=11.0)
set(CMAKE_OSX_DEPLOYMENT_TARGET "11.0")
add_compile_options(-arch arm64)
elseif(CLR_CMAKE_HOST_ARCH_AMD64)
set(MACOS_VERSION_MIN_FLAGS -mmacosx-version-min=10.13)
set(CMAKE_OSX_DEPLOYMENT_TARGET "10.13")
add_compile_options(-arch x86_64)
else()
clr_unknown_arch()
endif()
add_compile_options(${MACOS_VERSION_MIN_FLAGS})
add_linker_flag(${MACOS_VERSION_MIN_FLAGS})
endif(CLR_CMAKE_TARGET_MACCATALYST)
endif(CLR_CMAKE_HOST_OSX OR CLR_CMAKE_HOST_MACCATALYST)

Expand Down
14 changes: 11 additions & 3 deletions src/libraries/Native/Unix/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,23 @@ endif()
if(CLR_CMAKE_TARGET_MACCATALYST)
# -target overrides -mmacosx-version-min so suppress warning about that
# https://gitlab.kitware.com/cmake/cmake/-/issues/20132
add_compile_options(-Wno-overriding-t-option)
set(DISABLE_OVERRIDING_MIN_VERSION_ERROR -Wno-overriding-t-option)
add_link_options(-Wno-overriding-t-option)
if (CLR_CMAKE_TARGET_ARCH_AMD64)
add_compile_options(-target x86_64-apple-ios13.5-macabi)
set(MACOS_VERSION_MIN_FLAGS "-target x86_64-apple-ios13.5-macabi")
add_link_options(-target x86_64-apple-ios13.5-macabi)
elseif (CLR_CMAKE_TARGET_ARCH_ARM64)
add_compile_options(-target arm64-apple-ios14.2-macabi)
set(MACOS_VERSION_MIN_FLAGS "-target arm64-apple-ios14.2-macabi")
add_link_options(-target arm64-apple-ios14.2-macabi)
endif()

# These options are intentionally set using the CMAKE_XXX_FLAGS instead of
# add_compile_options so that they take effect on the configuration functions
# in various configure.cmake files.
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${MACOS_VERSION_MIN_FLAGS} ${DISABLE_OVERRIDING_MIN_VERSION_ERROR}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${MACOS_VERSION_MIN_FLAGS} ${DISABLE_OVERRIDING_MIN_VERSION_ERROR}")
set(CMAKE_ASM_FLAGS "${CMAKE_ASM_FLAGS} ${MACOS_VERSION_MIN_FLAGS} ${DISABLE_OVERRIDING_MIN_VERSION_ERROR}")

endif()

if(CLR_CMAKE_TARGET_TVOS)
Expand Down
4 changes: 4 additions & 0 deletions src/libraries/Native/Unix/System.Native/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ if (NOT CLR_CMAKE_TARGET_MACCATALYST AND NOT CLR_CMAKE_TARGET_IOS AND NOT CLR_CM
add_definitions(-DHAS_CONSOLE_SIGNALS)
endif ()

if (CLR_CMAKE_TARGET_OSX)
add_definitions(-D_DARWIN_C_SOURCE)
endif ()

include_directories("${CLR_SRC_NATIVE_DIR}/common")

set(NATIVE_SOURCES
Expand Down
6 changes: 5 additions & 1 deletion src/libraries/Native/build-native.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,11 @@ fi

if [[ "$__TargetOS" == OSX ]]; then
# set default OSX deployment target
__CMakeArgs="-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 $__CMakeArgs"
if [[ "$__BuildArch" == x64 ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

(not for this PR but just curious) Does it always eventually get overwritten (with same value) in src/libraries/Native/Unix/CMakeLists.txt? If so, we can delete the if block.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not. The src/libraries/Native/Unix/CMakeLists.txt doesn't set the variable nor the underlying option for macOS x64/arm64. I actually wonder why don't we use the settings from the eng/native/configurecompiler.cmake here. I have thought that we have unified all three subrepos to use those, but I cannot see it getting included. Do you remember when you were unifying this stuff if there was a reason to not to use the configurecompiler.cmake here? Or was it being used before and some change has removed it? Maybe the fact that mono uses this stuff too was the reason?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, you are right. I was confusing MACOS_VERSION_MIN_FLAGS with CMAKE_OSX_DEPLOYMENT_TARGET.

Around the time when we were consolidating native configurations, mono builds were being added (Android/iOS etc.), so the progress stopped to avoid conflicts with other PRs. We had cleaned up most of the initial redundancies from scripts and cmake, but this part of libraires' cmake configurations was left alone because mono targets were frequently updating these files. Now that we have relatively a stable build matrix, that work can be continued to accomplish DRY.

__CMakeArgs="-DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 $__CMakeArgs"
else
__CMakeArgs="-DCMAKE_OSX_DEPLOYMENT_TARGET=11.0 $__CMakeArgs"
fi
elif [[ "$__TargetOS" == Android && -z "$ROOTFS_DIR" ]]; then
if [[ -z "$ANDROID_NDK_ROOT" ]]; then
echo "Error: You need to set the ANDROID_NDK_ROOT environment variable pointing to the Android NDK root."
Expand Down