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

Fixup Macho-O rpath for osx-dynamic #39313

Merged
merged 6 commits into from
Jun 27, 2024
Merged

Fixup Macho-O rpath for osx-dynamic #39313

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
8a3d437
Fix Macho-O rpath for osx-dynamic
m-kuhn Jun 15, 2024
d6501c9
[rpath-test,rpath-test-binaries] Validate rpath fixup
dg0yt Jun 16, 2024
82e282d
Fix rpath for debug tools and add unit tests
m-kuhn Jun 17, 2024
fc9c3a7
Merge remote-tracking branch 'origin/master' into HEAD
BillyONeal Jun 21, 2024
1f94843
Fix regex
m-kuhn Jun 25, 2024
8a79bec
Address review comments
m-kuhn Jun 25, 2024
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
159 changes: 159 additions & 0 deletions scripts/cmake/z_vcpkg_fixup_rpath_macho.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
function(z_vcpkg_calculate_corrected_macho_rpath)
m-kuhn marked this conversation as resolved.
Show resolved Hide resolved
cmake_parse_arguments(PARSE_ARGV 0 "arg"
""
"MACHO_FILE_DIR;OUT_NEW_RPATH_VAR"
"")

m-kuhn marked this conversation as resolved.
Show resolved Hide resolved
if(DEFINED arg_UNPARSED_ARGUMENTS)
message(FATAL_ERROR "${CMAKE_CURRENT_FUNCTION} was passed extra arguments: ${arg_UNPARSED_ARGUMENTS}")
endif()

set(current_prefix "${CURRENT_PACKAGES_DIR}")
set(current_installed_prefix "${CURRENT_INSTALLED_DIR}")
file(RELATIVE_PATH relative_from_packages "${CURRENT_PACKAGES_DIR}" "${arg_MACHO_FILE_DIR}")
if("${relative_from_packages}/" MATCHES "^debug/" OR "${relative_from_packages}/" MATCHES "^(manual-)?tools/.*/debug/.*")
set(current_prefix "${CURRENT_PACKAGES_DIR}/debug")
set(current_installed_prefix "${CURRENT_INSTALLED_DIR}/debug")
endif()

# compute path relative to lib
file(RELATIVE_PATH relative_to_lib "${arg_MACHO_FILE_DIR}" "${current_prefix}/lib")
# remove trailing slash
string(REGEX REPLACE "/+$" "" relative_to_lib "${relative_to_lib}")

if(NOT relative_to_lib STREQUAL "")
set(new_rpath "@loader_path/${relative_to_lib}")
else()
set(new_rpath "@loader_path")
endif()

set("${arg_OUT_NEW_RPATH_VAR}" "${new_rpath}" PARENT_SCOPE)
endfunction()

function(z_vcpkg_fixup_macho_rpath_in_dir)
# We need to iterate through everything because we
# can't predict where a Mach-O file will be located
file(GLOB root_entries LIST_DIRECTORIES TRUE "${CURRENT_PACKAGES_DIR}/*")

# Skip some folders for better throughput
list(APPEND folders_to_skip "include")
list(JOIN folders_to_skip "|" folders_to_skip_regex)
set(folders_to_skip_regex "^(${folders_to_skip_regex})$")

find_program(
install_name_tool_cmd
NAMES install_name_tool
DOC "Absolute path of install_name_tool cmd"
REQUIRED
)

find_program(
otool_cmd
NAMES otool
DOC "Absolute path of otool cmd"
REQUIRED
)

find_program(
file_cmd
NAMES file
DOC "Absolute path of file cmd"
REQUIRED
)
Comment on lines +43 to +62
Copy link
Member

Choose a reason for hiding this comment

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

What are these and what does a user need to do if they aren't present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tools to modify and change dynamic libraries (install_name tool and otool) or inspect files (file tool).
In my experience they come pre-installed on macos systems, so I don't think "not present" is a real world scenario to take into account.


foreach(folder IN LISTS root_entries)
if(NOT IS_DIRECTORY "${folder}")
continue()
endif()

get_filename_component(folder_name "${folder}" NAME)
if(folder_name MATCHES "${folders_to_skip_regex}")
continue()
endif()

file(GLOB_RECURSE macho_files LIST_DIRECTORIES FALSE "${folder}/*")
list(FILTER macho_files EXCLUDE REGEX [[\.(cpp|cc|cxx|c|hpp|h|hh|hxx|inc|json|toml|yaml|man|m4|ac|am|in|log|txt|pyi?|pyc|pyx|pxd|pc|cmake|f77|f90|f03|fi|f|cu|mod|ini|whl|cat|csv|rst|md|npy|npz|template|build)$]])
list(FILTER macho_files EXCLUDE REGEX "/(copyright|LICENSE|METADATA)$")

foreach(macho_file IN LISTS macho_files)
if(IS_SYMLINK "${macho_file}")
continue()
endif()

# Determine if the file is a Mach-O executable or shared library
execute_process(
COMMAND "${file_cmd}" -b "${macho_file}"
OUTPUT_VARIABLE file_output
OUTPUT_STRIP_TRAILING_WHITESPACE
)
if(file_output MATCHES ".*Mach-O.*shared library.*")
set(file_type "shared")
elseif(file_output MATCHES ".*Mach-O.*executable.*")
set(file_type "executable")
else()
debug_message("File `${macho_file}` reported as `${file_output}` is not a Mach-O file")
continue()
m-kuhn marked this conversation as resolved.
Show resolved Hide resolved
endif()

get_filename_component(macho_file_dir "${macho_file}" DIRECTORY)
get_filename_component(macho_file_name "${macho_file}" NAME)

z_vcpkg_calculate_corrected_macho_rpath(
MACHO_FILE_DIR "${macho_file_dir}"
OUT_NEW_RPATH_VAR new_rpath
)

if("${file_type}" STREQUAL "shared")
# Set the install name for shared libraries
execute_process(
COMMAND "${install_name_tool_cmd}" -id "@rpath/${macho_file_name}" "${macho_file}"
OUTPUT_QUIET
ERROR_VARIABLE set_id_error
)
message(STATUS "Set install name id of '${macho_file}' (To '@rpath/${macho_file_name}')")
if(NOT "${set_id_error}" STREQUAL "")
message(WARNING "Couldn't adjust install name of '${macho_file}': ${set_id_error}")
continue()
endif()
endif()

# Clear all existing rpaths
execute_process(
COMMAND "${otool_cmd}" -l "${macho_file}"
OUTPUT_VARIABLE get_rpath_ov
RESULT_VARIABLE get_rpath_rv
)

if(NOT get_rpath_rv EQUAL 0)
message(FATAL_ERROR "Could not obtain rpath list from '${macho_file}'")
endif()
# Extract the LC_RPATH load commands and extract the paths
string(REGEX REPLACE "[^\n]+cmd LC_RPATH\n[^\n]+\n[^\n]+path ([^\n]+) \\(offset[^\n]+\n" "rpath \\1\n" get_rpath_ov "${get_rpath_ov}")
string(REGEX MATCHALL "rpath [^\n]+" get_rpath_ov "${get_rpath_ov}")
string(REGEX REPLACE "rpath " "" rpath_list "${get_rpath_ov}")

foreach(rpath IN LISTS rpath_list)
execute_process(
COMMAND "${install_name_tool_cmd}" -delete_rpath "${rpath}" "${macho_file}"
Copy link
Contributor

@sharadhr sharadhr Jun 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What about the LC_LOAD_DYLIB field? How come we don't use install_name_tool -change here for dependent libraries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tested this approach with a wide variety of ports (qt, qt5, even python via open-vcpkg) and it worked reliably.
Do you have a port which needs attention or do you want to provide a specific alternative implementation?

Copy link
Contributor

Choose a reason for hiding this comment

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

If it works reliably, great! No further questions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @m-kuhn
I've seen the PR merged and I give it a try.
I think it may be needed to take into account the LC_LOAD_DYLIB when it is pointing to other VCPKG dynamic libraries.

Here the output of a just built ffmpeg library, which depends on other ffmpeg libraries:

% otool -L libavcodec.60.31.102.dylib
libavcodec.60.31.102.dylib (architecture arm64):
	@rpath/libavcodec.60.31.102.dylib (compatibility version 60.0.0, current version 60.31.102)
	/Users/megadev/aag/vcpkg/packages/ffmpeg_arm64-osx-dynamic/lib/libswresample.4.dylib (compatibility version 4.0.0, current version 4.12.100)
	/Users/megadev/aag/vcpkg/packages/ffmpeg_arm64-osx-dynamic/lib/libavutil.58.dylib (compatibility version 58.0.0, current version 58.29.100)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.0.0)
	/System/Library/Frameworks/AudioToolbox.framework/Versions/A/AudioToolbox (compatibility version 1.0.0, current version 1000.0.0)
	/System/Library/Frameworks/VideoToolbox.framework/Versions/A/VideoToolbox (compatibility version 1.0.0, current version 1.0.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 2048.1.255)
	/System/Library/Frameworks/CoreMedia.framework/Versions/A/CoreMedia (compatibility version 1.0.0, current version 1.0.0)
	/System/Library/Frameworks/CoreVideo.framework/Versions/A/CoreVideo (compatibility version 1.2.0, current version 1.5.0)
	/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 1226.0.0)

We can see that the dependencies of the library have a path to the packages directory. I guess the LC_LOAD_DYLIBs should be @rpath/<lib>.

Sadly I didn't have time before to test it before merging. So sorry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedback, much appreciated. I can reproduce this here.
Can you open an issue, so it's not forgotten?

Copy link
Contributor

@aabellagm aabellagm Jul 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for the delay. Done #39890

OUTPUT_QUIET
ERROR_VARIABLE delete_rpath_error
)
message(STATUS "Remove RPATH from '${macho_file}' ('${rpath}')")
endforeach()

# Set the new rpath
execute_process(
COMMAND "${install_name_tool_cmd}" -add_rpath "${new_rpath}" "${macho_file}"
OUTPUT_QUIET
ERROR_VARIABLE set_rpath_error
)

if(NOT "${set_rpath_error}" STREQUAL "")
message(WARNING "Couldn't adjust RPATH of '${macho_file}': ${set_rpath_error}")
continue()
endif()

message(STATUS "Adjusted RPATH of '${macho_file}' (To '${new_rpath}')")
endforeach()
endforeach()
endfunction()
6 changes: 5 additions & 1 deletion scripts/ports.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ include("${SCRIPTS}/cmake/z_vcpkg_prettify_command_line.cmake")
include("${SCRIPTS}/cmake/z_vcpkg_setup_pkgconfig_path.cmake")

include("${SCRIPTS}/cmake/z_vcpkg_fixup_rpath.cmake")
include("${SCRIPTS}/cmake/z_vcpkg_fixup_rpath_macho.cmake")

function(debug_message)
if(PORT_DEBUG)
Expand Down Expand Up @@ -190,10 +191,13 @@ target system or to the host system. Use a prefixed variable instead.

include("${CURRENT_PORT_DIR}/portfile.cmake")
if(DEFINED PORT)
# Always fixup RPATH on linux unless explicitly disabled.
# Always fixup RPATH on linux and osx unless explicitly disabled.
if(VCPKG_FIXUP_ELF_RPATH OR (VCPKG_TARGET_IS_LINUX AND NOT DEFINED VCPKG_FIXUP_ELF_RPATH))
z_vcpkg_fixup_rpath_in_dir()
endif()
if(VCPKG_FIXUP_MACHO_RPATH OR (VCPKG_TARGET_IS_OSX AND NOT DEFINED VCPKG_FIXUP_MACHO_RPATH))
z_vcpkg_fixup_macho_rpath_in_dir()
endif()
include("${SCRIPTS}/build_info.cmake")
endif()
elseif(CMD STREQUAL "CREATE")
Expand Down
18 changes: 18 additions & 0 deletions scripts/test_ports/rpath-test-binaries/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
set(VCPKG_POLICY_EMPTY_INCLUDE_FOLDER enabled)
vcpkg_check_linkage(ONLY_DYNAMIC_LIBRARY)
vcpkg_cmake_configure(
SOURCE_PATH "${CURRENT_PORT_DIR}/project"
OPTIONS_RELEASE
-DTEST_STRING=release
OPTIONS_DEBUG
-DTEST_STRING=debug
)
vcpkg_cmake_install()
if(NOT VCPKG_BUILD_TYPE)
vcpkg_copy_tools(TOOL_NAMES rpath-test-tool
SEARCH_DIR "${CURRENT_PACKAGES_DIR}/debug/bin"
DESTINATION "${CURRENT_PACKAGES_DIR}/tools/${PORT}/debug"
)
endif()
vcpkg_copy_tools(TOOL_NAMES rpath-test-tool AUTO_CLEAN)
file(WRITE "${CURRENT_PACKAGES_DIR}/share/${PORT}/copyright" "This test port is part of vcpkg.")
12 changes: 12 additions & 0 deletions scripts/test_ports/rpath-test-binaries/project/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cmake_minimum_required(VERSION 3.7)
project(rpath-test CXX)

set(TEST_STRING "" CACHE STRING "")

add_library(rpath-test-lib lib.cpp)
target_compile_definitions(rpath-test-lib PRIVATE "TEST_STRING=\"${TEST_STRING}\"")

add_executable(rpath-test-tool main.cpp)
target_link_libraries(rpath-test-tool PRIVATE rpath-test-lib)

install(TARGETS rpath-test-lib rpath-test-tool)
4 changes: 4 additions & 0 deletions scripts/test_ports/rpath-test-binaries/project/lib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
const char* getTestString()
{
return TEST_STRING;
}
8 changes: 8 additions & 0 deletions scripts/test_ports/rpath-test-binaries/project/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#include <stdio.h>

extern const char* getTestString();

int main()
{
puts(getTestString());
}
12 changes: 12 additions & 0 deletions scripts/test_ports/rpath-test-binaries/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "rpath-test-binaries",
"version-string": "ci",
"description": "Provides installed binaries for rpath fixup test",
"supports": "native & !windows",
"dependencies": [
{
"name": "vcpkg-cmake",
"host": true
}
]
}
23 changes: 23 additions & 0 deletions scripts/test_ports/rpath-test/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
set(VCPKG_POLICY_EMPTY_PACKAGE enabled)

vcpkg_execute_in_download_mode(
COMMAND "${CURRENT_INSTALLED_DIR}/tools/rpath-test-binaries/rpath-test-tool"
WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}"
OUTPUT_VARIABLE output
OUTPUT_STRIP_TRAILING_WHITESPACE
)
if(NOT output STREQUAL "release")
message(SEND_ERROR "Actual: '${output}', expected: 'release'")
endif()

if(NOT VCPKG_BUILD_TYPE)
vcpkg_execute_in_download_mode(
COMMAND "${CURRENT_INSTALLED_DIR}/tools/rpath-test-binaries/debug/rpath-test-tool"
WORKING_DIRECTORY "${CURRENT_BUILDTREES_DIR}"
OUTPUT_VARIABLE output
OUTPUT_STRIP_TRAILING_WHITESPACE
)
if(NOT output STREQUAL "debug")
message(SEND_ERROR "Actual: '${output}', expected: 'debug'")
endif()
endif()
8 changes: 8 additions & 0 deletions scripts/test_ports/rpath-test/vcpkg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "rpath-test",
"version-string": "ci",
"description": "Test rpath fixup",
"dependencies": [
"rpath-test-binaries"
]
}
1 change: 1 addition & 0 deletions scripts/test_ports/unit-test-cmake/portfile.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ if("fixup-pkgconfig" IN_LIST FEATURES)
endif()
if("fixup-rpath" IN_LIST FEATURES)
include("${CMAKE_CURRENT_LIST_DIR}/test-z_vcpkg_calculate_corrected_rpath.cmake")
include("${CMAKE_CURRENT_LIST_DIR}/test-z_vcpkg_calculate_corrected_rpath_macho.cmake")
endif()

if(Z_VCPKG_UNIT_TEST_HAS_ERROR)
Expand Down
56 changes: 56 additions & 0 deletions scripts/test_ports/unit-test-cmake/test-z_vcpkg_calculate_corrected_rpath_macho.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# z_vcpkg_calculate_corrected_macho_rpath_macho(...)

block(SCOPE_FOR VARIABLES)

set(CURRENT_PACKAGES_DIR "/P")
set(CURRENT_INSTALLED_DIR "/I")

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/lib")
]] out [[@loader_path]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/plugins/group")
]] out [[@loader_path/../../lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/debug/lib")
]] out [[@loader_path]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/debug/plugins/group")
]] out [[@loader_path/../../lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/tools/port")
]] out [[@loader_path/../../lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/tools/port/bin")
]] out [[@loader_path/../../../lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/tools/port/debug")
]] out [[@loader_path/../../../debug/lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/tools/port/debug/bin")
]] out [[@loader_path/../../../../debug/lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/manual-tools/port")
]] out [[@loader_path/../../lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/manual-tools/port/bin")
]] out [[@loader_path/../../../lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/manual-tools/port/debug")
]] out [[@loader_path/../../../debug/lib]])

unit_test_check_variable_equal([[
z_vcpkg_calculate_corrected_macho_rpath(OUT_NEW_RPATH_VAR "out" MACHO_FILE_DIR "/P/manual-tools/port/debug/bin")
]] out [[@loader_path/../../../../debug/lib]])

endblock()
Loading
Loading