-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[scripts|cmake] Fix up install name and rpath on osx dynamic #32200
[scripts|cmake] Fix up install name and rpath on osx dynamic #32200
Conversation
…install-name-rpath-osx-dynamic
…install-name-rpath-osx-dynamic
…install-name-rpath-osx-dynamic
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.
I like this direction.
if(VCPKG_FIXUP_MACHO_RPATH) | ||
include("${SCRIPTS}/cmake/z_vcpkg_fixup_install_name_rpath.cmake") | ||
endif() |
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.
if(VCPKG_FIXUP_MACHO_RPATH) | |
include("${SCRIPTS}/cmake/z_vcpkg_fixup_install_name_rpath.cmake") | |
endif() | |
z_vcpkg_fixup_install_name_rpath_in_dir() |
and include unconditionally at the top. The function should always be available.
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.
Does that imply that linux rpath fixup mechanism should also be included unconditionally?
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.
So let z_vcpkg_fixup_install_name_rpath_in_dir()
check the running triplet like when hitting *-osx-dynamic
to run the fixup?
endforeach() | ||
endfunction() | ||
|
||
z_vcpkg_fixup_install_name_rpath_in_dir() |
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.
z_vcpkg_fixup_install_name_rpath_in_dir() |
# │ ├── libicudata.72.dylib -> libicudata.72.1.dylib | ||
# │ ├── libicudata.dylib -> libicudata.72.1.dylib |
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.
Note that this pattern is already broken as symlinks aren't cachable. If that's necessary then yet another problem with osx-dyanamic :(
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.
It's not necessary, just showing a build tree from icu
.
The script just tries to resolve a install name
for the underlying dylib non-link file while keeping orginal symlinks, See at:
# Get the major version
string(REGEX REPLACE "(lib[^\\.]+\\.[0-9]+).*\\.(dylib|so)" "\\1.\\2" filename_major "${filename}")
set(install_name_file "${file_dir}/${filename_major}")
# Try the major version file
if(NOT EXISTS "${install_name_file}")
# Get the major.minor version if major version file is missing
string(REGEX REPLACE "(lib[^\\.]+\\.[0-9]+.[0-9]+).*\\.(dylib|so)" "\\1.\\2" filename_major_minor "${filename}")
set(install_name_file "${file_dir}/${filename_major_minor}")
# Fall back to if the major version file not found
if(NOT EXISTS "${install_name_file}")
set(install_name_file "${file}")
endif()
endif()
DOC "Absolute path of otool cmd" | ||
) | ||
|
||
if(NOT otool_cmd) | ||
message(WARNING "otool not found!") | ||
return() | ||
endif() |
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.
DOC "Absolute path of otool cmd" | |
) | |
if(NOT otool_cmd) | |
message(WARNING "otool not found!") | |
return() | |
endif() | |
DOC "Absolute path of otool cmd" | |
REQUIRED | |
) |
This can't depend on what happens to be on the box, it needs to be done, a warning isn't sufficient.
Otherwise we end up binary caching broken bits from a broken machine and restoring them on a hypothetically working machine.
DOC "Absolute path of install_name_tool cmd" | ||
) | ||
|
||
if(NOT install_name_tool_cmd) | ||
message(WARNING " install_name_tool not found!") | ||
return() | ||
endif() |
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.
DOC "Absolute path of install_name_tool cmd" | |
) | |
if(NOT install_name_tool_cmd) | |
message(WARNING " install_name_tool not found!") | |
return() | |
endif() | |
DOC "Absolute path of install_name_tool cmd" | |
REQUIRED | |
) |
ditto.
# - exclude system libraries in `/usr/lib/` and `/System/Library`! | ||
# - fix rpath to `@loader/../../../` to be relative with `lib/` | ||
function(z_vcpkg_fixup_install_name_rpath_in_dir) | ||
find_program( |
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.
Can we do better than find_program
, e.g. fetching this for the user if possible?
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.
Two feasible ways to install otool
and install_name_tool
tools, bundled in Xcode Command Line Tools
:
- Run
xcode-select --install
in terminal. - Goto
developer.apple.com/downloads
but you have to login first.
The 1st
seems more reasonable.
) | ||
|
||
if(NOT set_install_name_rv EQUAL 0) | ||
message(WARNING "Failed, install_name_tool -id ${id_name} ${file}, \n${set_install_name_ov}") |
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.
message(WARNING "Failed, install_name_tool -id ${id_name} ${file}, \n${set_install_name_ov}") | |
message(FATAL_ERROR "Failed, install_name_tool -id ${id_name} ${file}, \n${set_install_name_ov}") |
) | ||
|
||
if(NOT set_rpath_rv EQUAL 0) | ||
message(WARNING "Failed, install_name_tool -add_rpath ${rpath} ${file}, \n${set_rpath_ov}") |
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.
message(WARNING "Failed, install_name_tool -add_rpath ${rpath} ${file}, \n${set_rpath_ov}") | |
message(FATAL_ERROR "Failed, install_name_tool -add_rpath ${rpath} ${file}, \n${set_rpath_ov}") |
message(WARNING "Failed, install_name_tool -change ${dep_install_name} ${id_name} ${file}, \n${change_dep_install_name_ov}") | ||
continue() | ||
else() | ||
message(DEBUG "Fix ${file}: ${dep_install_name} to ${id_name}") | ||
break() |
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.
message(WARNING "Failed, install_name_tool -change ${dep_install_name} ${id_name} ${file}, \n${change_dep_install_name_ov}") | |
continue() | |
else() | |
message(DEBUG "Fix ${file}: ${dep_install_name} to ${id_name}") | |
break() | |
message(FATAL_ERROR "Failed, install_name_tool -change ${dep_install_name} ${id_name} ${file}, \n${change_dep_install_name_ov}") |
Can I help to move this forward? |
Can anything be done to revive this? It's an immense quality of life improvement for |
message(DEBUG "${dep_install_name} match with ${dep_file} of install name ${id_name}") | ||
|
||
execute_process( | ||
COMMAND "${install_name_tool_cmd}" -change "${dep_install_name}" "${id_name}" "${file}" |
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.
This does not work here (in every case).
E.g. for tools/python3.11 it will not replace libpython3.11.dylib
because the tool will already load the library from the install path but we replace the package path. Wouldn't it be better to use the name as detected in dep_install_name
(without modification on as done here: https://github.com/microsoft/vcpkg/pull/32200/files#diff-20a469d007bd17c131ed093757f539dffc2fe3d47e89e154d349a8854899f9ceR376)?
Closing this PR since it seems that no progress is being made. Please ping us to reopen if work is still being done. |
Was also requested on Slack: https://cpplang.slack.com/archives/C7BFF7RCJ/p1716381580159229 |
@autoantwort continued in #36556 I still need to go over the comments by @Neumann-A , help welcome. |
It's supposed to uniform dynamic libraries and executable
install name
andrpath
on osx ofarm64-osx-dynamic
triplet. Due to many reasons, some individual ports may set the wrongrpath
andinstall name
on osx, or not set them. Here's hoping to manage them in a standard way invcpkg
.It is supposed to solve general problems like #31719, continuing on #31720, also referring #23035
Here's the rule which may be quite different than fixing
rpath
mentioned at #23035 as this following:After building and installing a package into
/opt/vcpkg/packages/xxxx_arm64-osx-dynamic
, it'll do the fix on Mach-O files inlib/
,debug/lib/
,tools/xxx/
, following:lib/
anddebug/lib/
:install name
on itself to be with@rpath
prefixinstall name
in its dependent shared libraries to be with@rpath
prefix/usr/lib/
and/System/Library
!rpath
on itself to be@loader_path
currently (Or using absolute pathCURRENT_INSTALLED_DIR
, like/opt/vcpkg/installed/arm64-osx-dynamic/lib
or/opt/vcpkg/installed/arm64-osx-dynamic/debug/lib
?)tools/{package}/bin/
install name
in its dependent shared libraries to be with@rpath
prefixlib/
, assuming other libraries from other managed ports by vcpkg are already fixed!/usr/lib/
and/System/Library
!rpath
to be@loader_path/../../../
to be relative withlib/
(Or using absolute pathCURRENT_INSTALLED_DIR
, like/opt/vcpkg/installed/arm64-osx-dynamic/lib
?)Have done a test on [qtbase] and others
Before:
After fix:
❯ ~/Documents/dlopen_test.out /opt/vcpkg/installed/arm64-osx-dynamic/tools/Qt6/bin/rcc dlopen sample% ❯ otool -L /opt/vcpkg/installed/arm64-osx-dynamic/debug/lib/libQt6Core_debug.6.dylib /opt/vcpkg/installed/arm64-osx-dynamic/debug/lib/libQt6Core_debug.6.dylib: @rpath/libQt6Core_debug.6.dylib (compatibility version 6.0.0, current version 6.5.1) /System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0) /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1971.0.0) /System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1971.0.0) /System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration (compatibility version 1.0.0, current version 1.0.0) /System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 2299.50.120) /System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices (compatibility version 1.0.0, current version 61.0.0) /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 1228.0.0) /System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 60420.101.2) @rpath/libb2.1.dylib (compatibility version 2.0.0, current version 2.4.0) @rpath/libicui18n.73.dylib (compatibility version 73.0.0, current version 73.1.0) @rpath/libicuuc.73.dylib (compatibility version 73.0.0, current version 73.1.0) @rpath/libicudata.73.dylib (compatibility version 73.0.0, current version 73.1.0) /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11) @rpath/libdouble-conversion.3.dylib (compatibility version 3.0.0, current version 3.2.0) @rpath/libpcre2-16.0.dylib (compatibility version 12.0.0, current version 12.2.0) @rpath/libssl.3.dylib (compatibility version 3.0.0, current version 3.0.0) @rpath/libcrypto.3.dylib (compatibility version 3.0.0, current version 3.0.0) @rpath/libzstd.1.dylib (compatibility version 1.0.0, current version 1.5.5) /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1500.65.0) /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1319.100.3) /usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)