Allow uname calls in download mode + Fix handling of files without RPATH (#37129)

... and don't fetch patchelf unless we actually need it. For example cmake helper ports have no need to fetch patchelf. In https://github.com/microsoft/vcpkg/pull/36056 , things were changed to unconditionally patchelf when targeting Linux. Unfortunately that broke --only-downloads a lot of the time. This fixes that. Also contains @Osyotr 's fixes from https://github.com/microsoft/vcpkg/pull/37190 to coalesce world rebuilds a bit, so repeating comments from there: Fixes https://github.com/microsoft/vcpkg/issues/37183 Towards https://github.com/microsoft/vcpkg/issues/25668
2024-11-24 02:19:00 +08:00 · 2024-03-07 15:19:17 -08:00 · 2024-03-07 15:19:17 -08:00 · e38b936343
commit e38b936343
parent 8f542fd07a
5 changed files with 56 additions and 9 deletions
--- a/scripts/cmake/vcpkg_find_acquire_program(NINJA).cmake
+++ b/scripts/cmake/vcpkg_find_acquire_program(NINJA).cmake
@ -15,7 +15,7 @@ elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin")
 elseif(CMAKE_HOST_SYSTEM_NAME STREQUAL "FreeBSD")
    set(paths_to_search "${DOWNLOADS}/tools/${tool_subdirectory}-freebsd")
 else()
-    execute_process(COMMAND "uname" "-m" OUTPUT_VARIABLE HOST_ARCH OUTPUT_STRIP_TRAILING_WHITESPACE)
+    vcpkg_execute_in_download_mode(COMMAND "uname" "-m" OUTPUT_VARIABLE HOST_ARCH OUTPUT_STRIP_TRAILING_WHITESPACE)
    if(HOST_ARCH MATCHES "x86_64|amd64|AMD64")
        set(download_filename "ninja-linux-${program_version}.zip")
        set(download_urls "https://github.com/ninja-build/ninja/releases/download/v${program_version}/ninja-linux.zip")
--- a/scripts/cmake/vcpkg_find_acquire_program(PATCHELF).cmake
+++ b/scripts/cmake/vcpkg_find_acquire_program(PATCHELF).cmake
@ -1,7 +1,7 @@
 set(program_name patchelf)
 set(program_version 0.14.5)
 if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux")
-    execute_process(COMMAND "uname" "-m" OUTPUT_VARIABLE HOST_ARCH OUTPUT_STRIP_TRAILING_WHITESPACE)
+    vcpkg_execute_in_download_mode(COMMAND "uname" "-m" OUTPUT_VARIABLE HOST_ARCH OUTPUT_STRIP_TRAILING_WHITESPACE)
    if(HOST_ARCH STREQUAL "aarch64")
        set(patchelf_platform "aarch64")
        set(download_sha512 "3B5EB4405FAB1D5202728AA390DD9F059CD7AFD582BAD9C50383CAD605127BC77DFCE3F2F26E9714F6BD5CCFFD49D3973BA2F061D2E2931B6E1BD0C263B99E75")
--- a/scripts/cmake/z_vcpkg_fixup_rpath.cmake
+++ b/scripts/cmake/z_vcpkg_fixup_rpath.cmake
@ -43,7 +43,6 @@ function(z_vcpkg_calculate_corrected_rpath)
        # duplication removal
        list(REMOVE_ITEM rpath_norm "\$ORIGIN")
        list(REMOVE_ITEM rpath_norm "\$ORIGIN/${relative_to_lib}")
-        list(REMOVE_DUPLICATES rpath_norm)

        if(NOT X_VCPKG_RPATH_KEEP_SYSTEM_PATHS)
          list(FILTER rpath_norm INCLUDE REGEX "\\\$ORIGIN.+") # Only keep paths relativ to ORIGIN
@ -55,14 +54,13 @@ function(z_vcpkg_calculate_corrected_rpath)
    endif()
    list(PREPEND rpath_norm "\$ORIGIN") # Make ORIGIN the first entry
    list(TRANSFORM rpath_norm REPLACE "/$" "")
+    list(REMOVE_DUPLICATES rpath_norm)
    cmake_path(CONVERT "${rpath_norm}" TO_NATIVE_PATH_LIST new_rpath)

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

 function(z_vcpkg_fixup_rpath_in_dir)
-    vcpkg_find_acquire_program(PATCHELF)
-
    # We need to iterate trough everything because we
    # can't predict where an elf file will be located
    file(GLOB root_entries LIST_DIRECTORIES TRUE "${CURRENT_PACKAGES_DIR}/*")
@ -72,6 +70,12 @@ function(z_vcpkg_fixup_rpath_in_dir)
    list(JOIN folders_to_skip "|" folders_to_skip_regex)
    set(folders_to_skip_regex "^(${folders_to_skip_regex})$")

+    # In download mode, we don't know if we're going to need PATCHELF, so be pessimistic and fetch
+    # it so it ends up in the downloads directory.
+    if(VCPKG_DOWNLOAD_MODE)
+        vcpkg_find_acquire_program(PATCHELF)
+    endif()
+
    foreach(folder IN LISTS root_entries)
        if(NOT IS_DIRECTORY "${folder}")
            continue()
@ -91,6 +95,8 @@ function(z_vcpkg_fixup_rpath_in_dir)
                continue()
            endif()

+            vcpkg_find_acquire_program(PATCHELF) # Note that this relies on vcpkg_find_acquire_program short
+                                                 # circuiting after the first run
            # If this fails, the file is not an elf
            execute_process(
                COMMAND "${PATCHELF}" --print-rpath "${elf_file}"
@ -98,7 +104,7 @@ function(z_vcpkg_fixup_rpath_in_dir)
                ERROR_VARIABLE read_rpath_error
            )
            string(REPLACE "\n" "" readelf_output "${readelf_output}")
-            if(NOT "${read_rpath_error}" STREQUAL "" OR "${readelf_output}" STREQUAL "")
+            if(NOT "${read_rpath_error}" STREQUAL "")
                continue()
            endif()

@ -116,8 +122,12 @@ function(z_vcpkg_fixup_rpath_in_dir)
                ERROR_VARIABLE set_rpath_error
            )

-            message(STATUS "Adjusted RPATH of '${elf_file}' (From '${org_rpath}' -> To '${new_rpath}')")
+            if(NOT "${set_rpath_error}" STREQUAL "")
+                message(WARNING "Couldn't adjust RPATH of '${elf_file}': ${set_rpath_error}")
+                continue()
+            endif()

+            message(STATUS "Adjusted RPATH of '${elf_file}' (From '${readelf_output}' -> To '${new_rpath}')")
        endforeach()
    endforeach()
 endfunction()
--- a/scripts/ports.cmake
+++ b/scripts/ports.cmake
@ -23,6 +23,9 @@ unset(CMAKE_TWEAK_VERSION)

 set(SCRIPTS "${CMAKE_CURRENT_LIST_DIR}" CACHE PATH "Location to stored scripts")
 list(APPEND CMAKE_MODULE_PATH "${SCRIPTS}/cmake")
+
+# Increment this number if we intentionally need to invalidate all binary caches due a change in
+# the following scripts: 1
 include("${SCRIPTS}/cmake/execute_process.cmake")
 include("${SCRIPTS}/cmake/vcpkg_acquire_msys.cmake")
 include("${SCRIPTS}/cmake/vcpkg_add_to_path.cmake")
@ -171,7 +174,8 @@ if(CMD STREQUAL "BUILD")

    include("${CURRENT_PORT_DIR}/portfile.cmake")
    if(DEFINED PORT)
-        if(VCPKG_FIXUP_ELF_RPATH OR VCPKG_TARGET_IS_LINUX)
+        # Always fixup RPATH on linux 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()
        include("${SCRIPTS}/build_info.cmake")
--- a/scripts/test_ports/vcpkg-fix-rpath/portfile.cmake
+++ b/scripts/test_ports/vcpkg-fix-rpath/portfile.cmake
@ -1,5 +1,38 @@
 set(VCPKG_POLICY_EMPTY_PACKAGE enabled)

+# Test for empty string
+set(elf_dir "${CURRENT_PACKAGES_DIR}/lib")
+set(test_rpath "")
+set(expected "$ORIGIN")
+
+z_vcpkg_calculate_corrected_rpath(
+  ELF_FILE_DIR "${elf_dir}"
+  ORG_RPATH "${test_rpath}"
+  OUT_NEW_RPATH_VAR new_rpath
+)
+
+if(NOT "x${new_rpath}x" STREQUAL "x${expected}x")
+  message(FATAL_ERROR "--- Calculated rpath does not agree with expected rpath: '${new_rpath}' != '${expected}' ")
+else()
+  message(STATUS "--- Calculated rpath agrees with expected rpath: '${new_rpath}' ")
+endif()
+
+# Test for empty string in the tools directory
+set(elf_dir "${CURRENT_PACKAGES_DIR}/tools/hdf5")
+set(test_rpath "")
+set(expected "$ORIGIN:$ORIGIN/../../lib")
+
+z_vcpkg_calculate_corrected_rpath(
+  ELF_FILE_DIR "${elf_dir}"
+  ORG_RPATH "${test_rpath}"
+  OUT_NEW_RPATH_VAR new_rpath
+)
+
+if(NOT "x${new_rpath}x" STREQUAL "x${expected}x")
+  message(FATAL_ERROR "--- Calculated rpath does not agree with expected rpath: '${new_rpath}' != '${expected}' ")
+else()
+  message(STATUS "--- Calculated rpath agrees with expected rpath: '${new_rpath}' ")
+endif()

 # Simple replacement and outside path test
 set(elf_dir "${CURRENT_PACKAGES_DIR}/lib")
@ -86,4 +119,4 @@ if(NOT "x${new_rpath}x" STREQUAL "x${expected}x")
  message(FATAL_ERROR "--- Calculated rpath does not agree with expected rpath: '${new_rpath}' != '${expected}' ")
 else()
  message(STATUS "--- Calculated rpath agrees with expected rpath: '${new_rpath}' ")
-endif()
+endif()