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

[vcpkg] Meson osx sysroot #21772

Merged
merged 6 commits into from
Feb 24, 2022
Merged

[vcpkg] Meson osx sysroot #21772

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
19d1706
[get_cmake_vars] add -isysroot flag independednt of crosscompiling
daschuer Nov 28, 2021
4c298e5
[meson] add -arch and -isysroot to linker flags in case of osx crossc…
daschuer Nov 28, 2021
267e8e3
[meson] avoid DEPRECATION warning about flags in the [properties] sec…
daschuer Nov 30, 2021
d1f915c
[meson] don't set OSXROOT evironment varibale
daschuer Jan 10, 2022
7e4ecfe
[meson] don't set MACOSX_DEPLOYMENT_TARGET environment variable
daschuer Jan 15, 2022
c56c9c1
[meson/make] Add CMAKE_OSX_DEPLOYMENT_TARGET to VCPKG_DETECTED_CMAKE_…
daschuer Jan 15, 2022
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
30 changes: 14 additions & 16 deletions scripts/cmake/vcpkg_configure_meson.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,17 @@ function(z_vcpkg_meson_generate_flags_properties_string out_var config_type)
set(linker_flags "${VCPKG_DETECTED_CMAKE_STATIC_LINKER_FLAGS_${config_type}}")
endif()
z_vcpkg_meson_convert_compiler_flags_to_list(linker_flags "${linker_flags}")
if(VCPKG_TARGET_IS_OSX)
# macOS - append arch and isysroot if cross-compiling
if(NOT "${VCPKG_OSX_ARCHITECTURES}" STREQUAL "${VCPKG_DETECTED_CMAKE_HOST_SYSTEM_PROCESSOR}")
foreach(arch IN LISTS VCPKG_OSX_ARCHITECTURES)
vcpkg_list(APPEND linker_flags -arch "${arch}")
endforeach()
endif()
if(VCPKG_DETECTED_CMAKE_OSX_SYSROOT)
vcpkg_list(APPEND linker_flags -isysroot "${VCPKG_DETECTED_CMAKE_OSX_SYSROOT}")
endif()
endif()
Comment on lines +182 to +192
Copy link
Contributor

@Neumann-A Neumann-A Jan 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Add it to the linker flags in get_cmake_vars/CMakeLists.txt ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does not work, because it would also affect autotools ports

Copy link
Contributor

Choose a reason for hiding this comment

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

Ping @Neumann-A for reply.

Copy link
Contributor

Choose a reason for hiding this comment

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

This does not work, because it would also affect autotools ports

Can you declare more?

Copy link
Contributor

Choose a reason for hiding this comment

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

This does not work, because it would also affect autotools ports

I also don't see why autotools should break with the additional flags. In general vcpkg should avoid adding additional flags in vcpkg_configure_ scripts.

Copy link
Contributor Author

@daschuer daschuer Jan 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Now we have:

set(VCPKG_DETECTED_CMAKE_SHARED_LINKER_FLAGS_DEBUG "-arch arm64 -isysroot /Applications/Xcode_12.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk")

/bin/sh ../libtool  --tag=CC   --mode=link gcc -fvisibility=hidden -fPIC -mmacosx-version-min=15.0 -arch arm64 -isysroot /Applications/Xcode_12.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -g  -liconv -Wl,-framework -Wl,CoreFoundation  -no-undefined -export-symbols-regex '^([^g]|g[^l]|gl[^w]|glw[^t]|glwt[^h]|glwth[^r]|glwthr[^e]|glwthre[^a]|glwthrea[^d]).*' -version-info 10:0:2 -rpath /Users/runner/mixxx-vcpkg/installed/arm64-osx-min11.0/debug/lib -L/Users/runner/mixxx-vcpkg/installed/arm64-osx-min11.0/debug/lib -arch arm64 -isysroot /Applications/Xcode_12.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -o libintl.la  bindtextdom.lo dcgettext.lo dgettext.lo gettext.lo finddomain.lo hash-string.lo loadmsgcat.lo localealias.lo textdomain.lo l10nflist.lo explodename.lo dcigettext.lo dcngettext.lo dngettext.lo ngettext.lo plural.lo plural-exp.lo localcharset.lo threadlib.lo lock.lo relocatable.lo langprefs.lo localename.lo localename-table.lo log.lo printf.lo setlocale.lo setlocale-lock.lo setlocale_null.lo version.lo xsize.lo osdep.lo intl-compat.lo

With -arch and -isysroot applied twice.

Conclusion: the current state of this PR is correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't share that conclusion.
The reason that is happening is because the common makefile has something along the line $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS). Which of course doubles the flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which common make file do you reference?

What do you propose as an alternative solution?

Copy link
Contributor

Choose a reason for hiding this comment

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

All the flags should be extract from cmake:

vcpkg/scripts/cmake/vcpkg_configure_meson.cmake

Line 383 in 5ef52b5

z_vcpkg_get_cmake_vars(cmake_vars_file)

Therefore, the behavor of cmake, make, qmake and meson etc. should be same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, this is already the case.

The issue here is that cmake and autotools take these flags from the c flags and pass them to the linker. Meson does not do it and require a special handlung.

I have demonstrated that the requested solution does apply the flags twice. The current state of the PR passes the right number of flags as desired.

qmake is out of scope, because it does not use z_vcpkg_get_cmake_vars

If you have an alternative solution in mind that works as well, please propose.

vcpkg_list(APPEND linker_flags "${libpath}")
z_vcpkg_meson_convert_list_to_python_array(linker_flags ${linker_flags})
string(APPEND result "c_link_args = ${linker_flags}\n")
Expand All @@ -188,8 +199,6 @@ endfunction()

function(z_vcpkg_meson_generate_native_file_config config_type) #https://mesonbuild.com/Native-environments.html
set(native_file "[properties]\n") #https://mesonbuild.com/Builtin-options.html
z_vcpkg_meson_generate_flags_properties_string(native_properties "${config_type}")
string(APPEND native_file "${native_properties}")
#Setup CMake properties
string(APPEND native_file "cmake_toolchain_file = '${SCRIPTS}/buildsystems/vcpkg.cmake'\n")
string(APPEND native_file "[cmake]\n")
Expand Down Expand Up @@ -219,6 +228,8 @@ function(z_vcpkg_meson_generate_native_file_config config_type) #https://mesonbu
string(APPEND native_file "VCPKG_CRT_LINKAGE = '${VCPKG_CRT_LINKAGE}'\n")

string(APPEND native_file "[built-in options]\n")
z_vcpkg_meson_generate_flags_properties_string(native_properties "${config_type}")
string(APPEND native_file "${native_properties}")
ras0219-msft marked this conversation as resolved.
Show resolved Hide resolved
if(VCPKG_TARGET_IS_WINDOWS)
if(VCPKG_CRT_LINKAGE STREQUAL "static")
set(crt_type mt)
Expand Down Expand Up @@ -348,9 +359,9 @@ endfunction()

function(z_vcpkg_meson_generate_cross_file_config config_type) #https://mesonbuild.com/Native-environments.html
set(cross_${config_type}_log "[properties]\n") #https://mesonbuild.com/Builtin-options.html
string(APPEND cross_${config_type}_log "[built-in options]\n")
z_vcpkg_meson_generate_flags_properties_string(cross_properties ${config_type})
string(APPEND cross_${config_type}_log "${cross_properties}")
string(APPEND cross_${config_type}_log "[built-in options]\n")
if(VCPKG_TARGET_IS_WINDOWS)
if(VCPKG_CRT_LINKAGE STREQUAL "static")
set(crt_type mt)
Expand Down Expand Up @@ -464,16 +475,6 @@ function(vcpkg_configure_meson)
set(suffix_${buildname} "rel")
endif()

if(VCPKG_TARGET_IS_OSX)
vcpkg_backup_env_variables(VARS SDKROOT MACOSX_DEPLOYMENT_TARGET)

set(ENV{SDKROOT} "${VCPKG_DETECTED_CMAKE_OSX_SYSROOT}")
set(VCPKG_DETECTED_CMAKE_OSX_SYSROOT "${VCPKG_DETECTED_CMAKE_OSX_SYSROOT}" PARENT_SCOPE)

set(ENV{MACOSX_DEPLOYMENT_TARGET} "${VCPKG_DETECTED_CMAKE_OSX_DEPLOYMENT_TARGET}")
set(VCPKG_DETECTED_CMAKE_OSX_DEPLOYMENT_TARGET "${VCPKG_DETECTED_CMAKE_OSX_DEPLOYMENT_TARGET}" PARENT_SCOPE)
endif()

vcpkg_backup_env_variables(VARS INCLUDE)
vcpkg_host_path_list(APPEND ENV{INCLUDE} "${CURRENT_INSTALLED_DIR}/include")
# configure build
Expand Down Expand Up @@ -512,8 +513,5 @@ function(vcpkg_configure_meson)
vcpkg_restore_env_variables(VARS PKG_CONFIG PKG_CONFIG_PATH)
endforeach()

if(VCPKG_TARGET_IS_OSX)
vcpkg_restore_env_variables(VARS SDKROOT MACOSX_DEPLOYMENT_TARGET)
endif()
vcpkg_restore_env_variables(VARS INCLUDE)
endfunction()
5 changes: 4 additions & 1 deletion scripts/get_cmake_vars/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ macro(_vcpkg_adjust_flags flag_var)
foreach(arch IN LISTS CMAKE_OSX_ARCHITECTURES)
string(APPEND ${flag_var} " -arch ${arch}")
endforeach()
string(APPEND ${flag_var} " -isysroot ${CMAKE_OSX_SYSROOT}")
endif()
string(APPEND ${flag_var} " -isysroot ${CMAKE_OSX_SYSROOT}")
if (CMAKE_OSX_DEPLOYMENT_TARGET)
string(APPEND ${flag_var} " -mmacosx-version-min=${CMAKE_OSX_DEPLOYMENT_TARGET}")
endif()
endif()
endif()
Expand Down