From 27ed27e8865dbc6fb667813df4e4ad445caab34a Mon Sep 17 00:00:00 2001 From: Bohdan Buinich Date: Tue, 3 Jun 2025 14:06:58 +0300 Subject: [PATCH 1/2] fix: Patch commands fail trying to re-apply the same patch twice Fixes #618 --- README.md | 5 +- cmake/CPM.cmake | 193 ++++++++++++++++++----- test/unit/package-lock_prettify.cmake | 18 +++ test/unit/package_patch.cmake | 219 ++++++++++++++++++++++++++ 4 files changed, 393 insertions(+), 42 deletions(-) create mode 100644 test/unit/package_patch.cmake diff --git a/README.md b/README.md index 4f1e7099..db2e6965 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ Afterwards, any targets defined in the dependency can be used directly. CPMAddPackage( NAME # The unique name of the dependency (should be the exported target's name) VERSION # The minimum version of the dependency (optional, defaults to 0) - PATCHES # Patch files to be applied sequentially using patch and PATCH_OPTIONS (optional) + PATCHES # Patch files to be applied sequentially using patch (optional) OPTIONS # Configuration options passed to the dependency (optional) DOWNLOAD_ONLY # If set to YES, the project is downloaded, but not configured (optional, default NO) [...] # Origin parameters forwarded to FetchContent_Declare, see below @@ -86,8 +86,7 @@ If `GIT_TAG` hasn't been explicitly specified it defaults to `v(VERSION)`, a com On the other hand, if `VERSION` hasn't been explicitly specified, CPM can automatically identify the version from the git tag in some common cases. `GIT_TAG` can also be set to a specific commit or a branch name such as `master`, however this isn't recommended, as such packages will only be updated when the cache is cleared. -`PATCHES` takes a list of patch files to apply sequentially. For a basic example, see [Highway](examples/highway/CMakeLists.txt). -We recommend that if you use `PATCHES`, you also set `CPM_SOURCE_CACHE`. See [issue 577](https://github.com/cpm-cmake/CPM.cmake/issues/577). +`PATCHES` takes a list of patch files to apply sequentially. Patch files are applied from the dependency source directory and skipped on later configure runs after the same patch set has been applied successfully. For a basic example, see [Highway](examples/highway/CMakeLists.txt). If an additional optional parameter `EXCLUDE_FROM_ALL` is set to a truthy value, then any targets defined inside the dependency won't be built by default. See the [CMake docs](https://cmake.org/cmake/help/latest/prop_tgt/EXCLUDE_FROM_ALL.html) for details. diff --git a/cmake/CPM.cmake b/cmake/CPM.cmake index 64e33749..32c865d0 100644 --- a/cmake/CPM.cmake +++ b/cmake/CPM.cmake @@ -539,70 +539,176 @@ function(cpm_check_git_working_dir_is_clean repoPath gitTag isClean) endfunction() -# Add PATCH_COMMAND to CPM_ARGS_UNPARSED_ARGUMENTS. This method consumes a list of files in ARGN -# then generates a `PATCH_COMMAND` appropriate for `ExternalProject_Add()`. This command is appended -# to the parent scope's `CPM_ARGS_UNPARSED_ARGUMENTS`. +function(cpm_escape_for_generated_cmake input output_variable) + set(value "${input}") + string(REPLACE "\\" "/" value "${value}") + string(REPLACE "\"" "\\\"" value "${value}") + string(REPLACE "$" "\\$" value "${value}") + string(REPLACE ";" "\\;" value "${value}") + set(${output_variable} + "${value}" + PARENT_SCOPE + ) +endfunction() + +# Generate a script that applies patches from the dependency source directory. +function(cpm_write_apply_patches_script script_path patch_executable patch_set_hash) + cpm_escape_for_generated_cmake("${patch_executable}" escaped_patch_executable) + + set(script "# Auto-generated patch application script\n") + string(APPEND script "set(patch_executable \"${escaped_patch_executable}\")\n") + string(APPEND script "set(patch_arguments -p1)\n") + string(APPEND script "set(patch_stamp_file \".cpm_patches/${patch_set_hash}.stamp\")\n") + string(APPEND script "set(patch_files)\n") + + foreach(patch_file IN LISTS ARGN) + cpm_escape_for_generated_cmake("${patch_file}" escaped_patch_file) + string(APPEND script "list(APPEND patch_files \"${escaped_patch_file}\")\n") + endforeach() + + string( + APPEND + script + [=[ + +get_filename_component(patch_working_directory "." ABSOLUTE) + +if(EXISTS "${patch_stamp_file}") + message(STATUS "Patch set already applied: ${patch_stamp_file}") + return() +endif() + +foreach(patch_file IN LISTS patch_files) + message(STATUS "Checking patch: ${patch_file}") + + execute_process( + COMMAND "${patch_executable}" --dry-run ${patch_arguments} + WORKING_DIRECTORY "${patch_working_directory}" + INPUT_FILE "${patch_file}" + RESULT_VARIABLE patch_check_result + OUTPUT_VARIABLE patch_check_output + ERROR_VARIABLE patch_check_error + ) + + if(patch_check_result EQUAL 0) + message(STATUS "Applying patch: ${patch_file}") + execute_process( + COMMAND "${patch_executable}" ${patch_arguments} + WORKING_DIRECTORY "${patch_working_directory}" + INPUT_FILE "${patch_file}" + RESULT_VARIABLE patch_apply_result + OUTPUT_VARIABLE patch_apply_output + ERROR_VARIABLE patch_apply_error + ) + if(patch_apply_result EQUAL 0) + message(STATUS "Applied patch: ${patch_file}") + else() + message( + FATAL_ERROR + "Patch failed: ${patch_file}\n" + "Working directory: ${patch_working_directory}\n" + "Patch executable: ${patch_executable}\n" + "Patch arguments: ${patch_arguments}\n" + "${patch_apply_output}\n${patch_apply_error}" + ) + endif() + else() + execute_process( + COMMAND "${patch_executable}" --dry-run ${patch_arguments} --reverse + WORKING_DIRECTORY "${patch_working_directory}" + INPUT_FILE "${patch_file}" + RESULT_VARIABLE reverse_check_result + OUTPUT_VARIABLE reverse_check_output + ERROR_VARIABLE reverse_check_error + ) + if(reverse_check_result EQUAL 0) + message(STATUS "Patch already applied: ${patch_file}") + else() + message( + FATAL_ERROR + "Patch cannot be applied and is not already applied: ${patch_file}\n" + "Working directory: ${patch_working_directory}\n" + "Patch executable: ${patch_executable}\n" + "Patch arguments: ${patch_arguments}\n" + "${patch_check_output}\n${patch_check_error}\n" + "${reverse_check_output}\n${reverse_check_error}" + ) + endif() + endif() +endforeach() + +get_filename_component(patch_stamp_directory "${patch_stamp_file}" DIRECTORY) +file(MAKE_DIRECTORY "${patch_stamp_directory}") +file(WRITE "${patch_stamp_file}" "applied\n") +]=] + ) + + file(WRITE "${script_path}" "${script}") +endfunction() + +# Add a generated PATCH_COMMAND for the provided patch files. function(cpm_add_patches) - # Return if no patch files are supplied. if(NOT ARGN) return() endif() - # Find the patch program. find_program(PATCH_EXECUTABLE patch) + if(CMAKE_HOST_WIN32 AND NOT PATCH_EXECUTABLE) - # The Windows git executable is distributed with patch.exe. Find the path to the executable, if - # it exists, then search `../usr/bin` and `../../usr/bin` for patch.exe. + # Git for Windows ships patch.exe in adjacent Unix tool directories. find_package(Git QUIET) if(GIT_EXECUTABLE) - get_filename_component(extra_search_path ${GIT_EXECUTABLE} DIRECTORY) - get_filename_component(extra_search_path_1up ${extra_search_path} DIRECTORY) - get_filename_component(extra_search_path_2up ${extra_search_path_1up} DIRECTORY) + get_filename_component(_git_bin_dir "${GIT_EXECUTABLE}" DIRECTORY) + get_filename_component(_git_parent_dir "${_git_bin_dir}" DIRECTORY) + get_filename_component(_git_root_dir "${_git_parent_dir}" DIRECTORY) + find_program( - PATCH_EXECUTABLE patch HINTS "${extra_search_path_1up}/usr/bin" - "${extra_search_path_2up}/usr/bin" + PATCH_EXECUTABLE patch HINTS "${_git_parent_dir}/usr/bin" "${_git_root_dir}/usr/bin" ) endif() endif() + if(NOT PATCH_EXECUTABLE) message(FATAL_ERROR "Couldn't find `patch` executable to use with PATCHES keyword.") endif() - # Create a temporary - set(temp_list ${CPM_ARGS_UNPARSED_ARGUMENTS}) + set(resolved_patch_files) + set(patch_arguments -p1) + set(patch_set_fingerprint "${PATCH_EXECUTABLE}" "${patch_arguments}" "WORKING_DIRECTORY=.") - # Ensure each file exists (or error out) and add it to the list. - set(first_item True) - foreach(PATCH_FILE ${ARGN}) - # Make sure the patch file exists, if we can't find it, try again in the current directory. - if(NOT EXISTS "${PATCH_FILE}") - if(NOT EXISTS "${CMAKE_CURRENT_LIST_DIR}/${PATCH_FILE}") - message(FATAL_ERROR "Couldn't find patch file: '${PATCH_FILE}'") + foreach(patch_file IN LISTS ARGN) + # Resolve relative patch files from the current package list file. + if(NOT EXISTS "${patch_file}") + set(package_relative_patch_file "${CMAKE_CURRENT_LIST_DIR}/${patch_file}") + if(NOT EXISTS "${package_relative_patch_file}") + message(FATAL_ERROR "Couldn't find patch file: '${patch_file}'") endif() - set(PATCH_FILE "${CMAKE_CURRENT_LIST_DIR}/${PATCH_FILE}") + set(patch_file "${package_relative_patch_file}") endif() - # Convert to absolute path for use with patch file command. - get_filename_component(PATCH_FILE "${PATCH_FILE}" ABSOLUTE) - - # The first patch entry must be preceded by "PATCH_COMMAND" while the following items are - # preceded by "&&". - if(first_item) - set(first_item False) - list(APPEND temp_list "PATCH_COMMAND") - else() - list(APPEND temp_list "&&") - endif() - # Add the patch command to the list - list(APPEND temp_list "${PATCH_EXECUTABLE}" "-p1" "<" "${PATCH_FILE}") + get_filename_component(patch_file "${patch_file}" ABSOLUTE) + file(TO_CMAKE_PATH "${patch_file}" patch_file) + file(SHA1 "${patch_file}" patch_file_hash) + list(APPEND resolved_patch_files "${patch_file}") + list(APPEND patch_set_fingerprint "${patch_file}" "${patch_file_hash}") endforeach() - # Move temp out into parent scope. + # Include patch contents in the stamp key. + string(REPLACE ";" "\n" patch_set_fingerprint_text "${patch_set_fingerprint}") + string(SHA1 patch_set_hash "${patch_set_fingerprint_text}") + set(patch_script_directory "${CMAKE_BINARY_DIR}/CPM_scripts") + set(patch_script "${patch_script_directory}/cpm_apply_patches_${patch_set_hash}.cmake") + + file(MAKE_DIRECTORY "${patch_script_directory}") + cpm_write_apply_patches_script( + "${patch_script}" "${PATCH_EXECUTABLE}" "${patch_set_hash}" ${resolved_patch_files} + ) + + list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS PATCH_COMMAND "${CMAKE_COMMAND}" -P "${patch_script}") set(CPM_ARGS_UNPARSED_ARGUMENTS - ${temp_list} + "${CPM_ARGS_UNPARSED_ARGUMENTS}" PARENT_SCOPE ) - endfunction() # method to overwrite internal FetchContent properties, to allow using CPM.cmake to overload @@ -1316,7 +1422,7 @@ function(cpm_prettify_package_arguments OUT_VAR IS_IN_COMMENT) EXCLUDE_FROM_ALL SOURCE_SUBDIR ) - set(multiValueArgs URL OPTIONS DOWNLOAD_COMMAND) + set(multiValueArgs URL OPTIONS DOWNLOAD_COMMAND PATCHES) cmake_parse_arguments(CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) foreach(oneArgName ${oneValueArgs}) @@ -1342,6 +1448,15 @@ function(cpm_prettify_package_arguments OUT_VAR IS_IN_COMMENT) if(${IS_IN_COMMENT}) string(APPEND PRETTY_OUT_VAR "#") endif() + if(multiArgName STREQUAL "PATCHES") + if(IS_ABSOLUTE "${singleOption}") + string(REPLACE ${CMAKE_SOURCE_DIR} "\${CMAKE_SOURCE_DIR}" singleOption + "${singleOption}" + ) + elseif(NOT singleOption MATCHES "^\\$\\{") + set(singleOption "\${CMAKE_SOURCE_DIR}/${singleOption}") + endif() + endif() string(APPEND PRETTY_OUT_VAR " \"${singleOption}\"\n") endforeach() endif() diff --git a/test/unit/package-lock_prettify.cmake b/test/unit/package-lock_prettify.cmake index e77110d0..db3481fc 100644 --- a/test/unit/package-lock_prettify.cmake +++ b/test/unit/package-lock_prettify.cmake @@ -1,5 +1,6 @@ cmake_minimum_required(VERSION 3.14 FATAL_ERROR) +set(CPM_DONT_CREATE_PACKAGE_LOCK TRUE) include(${CPM_PATH}/CPM.cmake) include(${CPM_PATH}/testing.cmake) @@ -40,3 +41,20 @@ set(EXPECTED_COMMENTED_LOCALDIR "# local directory " ) assert_equal(${PRETTY_ARGN} ${EXPECTED_COMMENTED_LOCALDIR}) + +# cmake-format: off +cpm_prettify_package_arguments(PRETTY_ARGN false + NAME Dependency + PATCHES + patches/fix.patch + ${CMAKE_SOURCE_DIR}/patches/absolute.patch +) +# cmake-format: on +set(EXPECTED_PATCHES + " NAME Dependency + PATCHES + \"\${CMAKE_SOURCE_DIR}/patches/fix.patch\" + \"\${CMAKE_SOURCE_DIR}/patches/absolute.patch\" +" +) +assert_equal(${PRETTY_ARGN} ${EXPECTED_PATCHES}) diff --git a/test/unit/package_patch.cmake b/test/unit/package_patch.cmake new file mode 100644 index 00000000..e10e856c --- /dev/null +++ b/test/unit/package_patch.cmake @@ -0,0 +1,219 @@ +cmake_minimum_required(VERSION 3.14 FATAL_ERROR) + +set(CPM_DONT_CREATE_PACKAGE_LOCK TRUE) +include(${CPM_PATH}/testing.cmake) +include(${CPM_PATH}/CPM.cmake) + +function(write_patch patch_file old_text new_text) + file( + WRITE "${patch_file}" + "diff --git a/input.txt b/input.txt +--- a/input.txt ++++ b/input.txt +@@ -1 +1 @@ +-${old_text} ++${new_text} +" + ) +endfunction() + +function(get_patch_script output_variable) + list(FIND CPM_ARGS_UNPARSED_ARGUMENTS "PATCH_COMMAND" patch_command_index) + assert_not_equal("${patch_command_index}" "-1") + + math(EXPR patch_script_index "${patch_command_index} + 3") + list(GET CPM_ARGS_UNPARSED_ARGUMENTS ${patch_script_index} patch_script) + assert_exists("${patch_script}") + + set(${output_variable} + "${patch_script}" + PARENT_SCOPE + ) +endfunction() + +function(run_patch_script patch_script working_directory) + execute_process( + COMMAND "${CMAKE_COMMAND}" -P "${patch_script}" + WORKING_DIRECTORY "${working_directory}" + RESULT_VARIABLE script_result + OUTPUT_VARIABLE script_output + ERROR_VARIABLE script_error + ) + + if(NOT script_result EQUAL 0) + message(FATAL_ERROR "Patch script failed:\n${script_output}\n${script_error}") + endif() +endfunction() + +function(run_patch_script_expect_failure patch_script working_directory) + execute_process( + COMMAND "${CMAKE_COMMAND}" -P "${patch_script}" + WORKING_DIRECTORY "${working_directory}" + RESULT_VARIABLE script_result + OUTPUT_VARIABLE script_output + ERROR_VARIABLE script_error + ) + + assert_not_equal("${script_result}" "0") + string(FIND "${script_error}" "Working directory:" working_directory_index) + string(FIND "${script_error}" "Patch executable:" patch_executable_index) + assert_not_equal("${working_directory_index}" "-1") + assert_not_equal("${patch_executable_index}" "-1") +endfunction() + +function(assert_file_content file expected_content) + file(READ "${file}" actual_content) + assert_equal("${actual_content}" "${expected_content}") +endfunction() + +# ---------------------------------------------------------------------------------------- +# Test Case 1: No patch files +# ---------------------------------------------------------------------------------------- +function(run_test_no_patches) + unset(CPM_ARGS_UNPARSED_ARGUMENTS) + cpm_add_patches() + + assert_not_defined(CPM_ARGS_UNPARSED_ARGUMENTS) +endfunction() + +# ---------------------------------------------------------------------------------------- +# Test Case 2: Single patch file applies once and is skipped on the second run +# ---------------------------------------------------------------------------------------- +function(run_test_single_patch) + set(test_dir "${CMAKE_CURRENT_BINARY_DIR}/single_patch") + set(patch_file "${test_dir}/change.patch") + + file(REMOVE_RECURSE "${test_dir}") + file(MAKE_DIRECTORY "${test_dir}") + file(WRITE "${test_dir}/input.txt" "old\n") + write_patch("${patch_file}" "old" "new") + + unset(CPM_ARGS_UNPARSED_ARGUMENTS) + cpm_add_patches("${patch_file}") + get_patch_script(patch_script) + + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") + + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") + + file(REMOVE_RECURSE "${test_dir}/.cpm_patches") + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") +endfunction() + +# ---------------------------------------------------------------------------------------- +# Test Case 3: Multiple dependent patch files apply sequentially and are skipped as a set +# ---------------------------------------------------------------------------------------- +function(run_test_multiple_patches) + set(test_dir "${CMAKE_CURRENT_BINARY_DIR}/multiple_patches") + set(patch_1 "${test_dir}/change_1.patch") + set(patch_2 "${test_dir}/change_2.patch") + + file(REMOVE_RECURSE "${test_dir}") + file(MAKE_DIRECTORY "${test_dir}") + file(WRITE "${test_dir}/input.txt" "one\n") + write_patch("${patch_1}" "one" "two") + write_patch("${patch_2}" "two" "three") + + unset(CPM_ARGS_UNPARSED_ARGUMENTS) + cpm_add_patches("${patch_1}" "${patch_2}") + get_patch_script(patch_script) + + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "three\n") + + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "three\n") +endfunction() + +# ---------------------------------------------------------------------------------------- +# Test Case 4: Patch file paths may contain spaces +# ---------------------------------------------------------------------------------------- +function(run_test_patch_path_with_spaces) + set(test_dir "${CMAKE_CURRENT_BINARY_DIR}/patch path with spaces") + set(patch_dir "${CMAKE_CURRENT_BINARY_DIR}/patch files with spaces") + set(patch_file "${patch_dir}/change with spaces.patch") + + file(REMOVE_RECURSE "${test_dir}" "${patch_dir}") + file(MAKE_DIRECTORY "${test_dir}" "${patch_dir}") + file(WRITE "${test_dir}/input.txt" "old\n") + write_patch("${patch_file}" "old" "new") + + unset(CPM_ARGS_UNPARSED_ARGUMENTS) + cpm_add_patches("${patch_file}") + get_patch_script(patch_script) + + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") +endfunction() + +# ---------------------------------------------------------------------------------------- +# Test Case 5: Edited patch files produce a new patch set +# ---------------------------------------------------------------------------------------- +function(run_test_patch_file_content_change) + set(test_dir "${CMAKE_CURRENT_BINARY_DIR}/changed_patch_file") + set(patch_file "${test_dir}/change.patch") + + file(REMOVE_RECURSE "${test_dir}") + file(MAKE_DIRECTORY "${test_dir}") + file(WRITE "${test_dir}/input.txt" "old\n") + write_patch("${patch_file}" "old" "new") + + unset(CPM_ARGS_UNPARSED_ARGUMENTS) + cpm_add_patches("${patch_file}") + get_patch_script(initial_patch_script) + + run_patch_script("${initial_patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") + + write_patch("${patch_file}" "new" "newer") + + unset(CPM_ARGS_UNPARSED_ARGUMENTS) + cpm_add_patches("${patch_file}") + get_patch_script(updated_patch_script) + + assert_not_equal("${initial_patch_script}" "${updated_patch_script}") + + run_patch_script("${updated_patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "newer\n") +endfunction() + +# ---------------------------------------------------------------------------------------- +# Test Case 6: Failed patch files do not write a stamp +# ---------------------------------------------------------------------------------------- +function(run_test_failed_patch_does_not_write_stamp) + set(test_dir "${CMAKE_CURRENT_BINARY_DIR}/failed_patch") + set(patch_file "${test_dir}/change.patch") + + file(REMOVE_RECURSE "${test_dir}") + file(MAKE_DIRECTORY "${test_dir}") + file(WRITE "${test_dir}/input.txt" "current\n") + write_patch("${patch_file}" "missing" "new") + + unset(CPM_ARGS_UNPARSED_ARGUMENTS) + cpm_add_patches("${patch_file}") + get_patch_script(patch_script) + + run_patch_script_expect_failure("${patch_script}" "${test_dir}") + assert_not_exists("${test_dir}/.cpm_patches") +endfunction() + +function(cleanup_patch_test_files) + file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/single_patch") + file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/multiple_patches") + file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/patch path with spaces") + file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/patch files with spaces") + file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/changed_patch_file") + file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/failed_patch") + file(REMOVE_RECURSE "${CMAKE_BINARY_DIR}/CPM_scripts") +endfunction() + +run_test_no_patches() +run_test_single_patch() +run_test_multiple_patches() +run_test_patch_path_with_spaces() +run_test_patch_file_content_change() +run_test_failed_patch_does_not_write_stamp() +cleanup_patch_test_files() From 2ef2b1e297042bff04df1487feb3bd91f9b4a267 Mon Sep 17 00:00:00 2001 From: Bohdan Buinich Date: Thu, 7 May 2026 11:13:30 +0300 Subject: [PATCH 2/2] fix macos compatibility --- cmake/CPM.cmake | 146 +++++++++++++++++++++++----------- test/unit/package_patch.cmake | 75 +++++++++++++++++ 2 files changed, 175 insertions(+), 46 deletions(-) diff --git a/cmake/CPM.cmake b/cmake/CPM.cmake index 32c865d0..0378334f 100644 --- a/cmake/CPM.cmake +++ b/cmake/CPM.cmake @@ -551,13 +551,32 @@ function(cpm_escape_for_generated_cmake input output_variable) ) endfunction() +function(cpm_patch_supports_dry_run patch_executable output_variable) + execute_process( + COMMAND "${patch_executable}" --help + RESULT_VARIABLE _result + OUTPUT_VARIABLE _help + ERROR_VARIABLE _help_err + ) + string(FIND "${_help}${_help_err}" "--dry-run" _index) + if(_index GREATER_EQUAL 0) + set(${output_variable} YES PARENT_SCOPE) + else() + set(${output_variable} NO PARENT_SCOPE) + endif() +endfunction() + # Generate a script that applies patches from the dependency source directory. -function(cpm_write_apply_patches_script script_path patch_executable patch_set_hash) +function(cpm_write_apply_patches_script script_path patch_executable patch_supports_dry_run + patch_set_hash +) cpm_escape_for_generated_cmake("${patch_executable}" escaped_patch_executable) set(script "# Auto-generated patch application script\n") string(APPEND script "set(patch_executable \"${escaped_patch_executable}\")\n") + string(APPEND script "set(patch_supports_dry_run ${patch_supports_dry_run})\n") string(APPEND script "set(patch_arguments -p1)\n") + string(APPEND script "set(patch_forward_arguments -N -p1 -r -)\n") string(APPEND script "set(patch_stamp_file \".cpm_patches/${patch_set_hash}.stamp\")\n") string(APPEND script "set(patch_files)\n") @@ -579,60 +598,92 @@ if(EXISTS "${patch_stamp_file}") endif() foreach(patch_file IN LISTS patch_files) - message(STATUS "Checking patch: ${patch_file}") - - execute_process( - COMMAND "${patch_executable}" --dry-run ${patch_arguments} - WORKING_DIRECTORY "${patch_working_directory}" - INPUT_FILE "${patch_file}" - RESULT_VARIABLE patch_check_result - OUTPUT_VARIABLE patch_check_output - ERROR_VARIABLE patch_check_error - ) - - if(patch_check_result EQUAL 0) - message(STATUS "Applying patch: ${patch_file}") + if(patch_supports_dry_run) + message(STATUS "Checking patch: ${patch_file}") execute_process( - COMMAND "${patch_executable}" ${patch_arguments} + COMMAND "${patch_executable}" --dry-run ${patch_arguments} WORKING_DIRECTORY "${patch_working_directory}" INPUT_FILE "${patch_file}" - RESULT_VARIABLE patch_apply_result - OUTPUT_VARIABLE patch_apply_output - ERROR_VARIABLE patch_apply_error + RESULT_VARIABLE patch_check_result + OUTPUT_VARIABLE patch_check_output + ERROR_VARIABLE patch_check_error ) - if(patch_apply_result EQUAL 0) - message(STATUS "Applied patch: ${patch_file}") + + if(patch_check_result EQUAL 0) + message(STATUS "Applying patch: ${patch_file}") + execute_process( + COMMAND "${patch_executable}" ${patch_arguments} + WORKING_DIRECTORY "${patch_working_directory}" + INPUT_FILE "${patch_file}" + RESULT_VARIABLE patch_apply_result + OUTPUT_VARIABLE patch_apply_output + ERROR_VARIABLE patch_apply_error + ) + if(patch_apply_result EQUAL 0) + message(STATUS "Applied patch: ${patch_file}") + else() + message( + FATAL_ERROR + "Patch failed: ${patch_file}\n" + "Working directory: ${patch_working_directory}\n" + "Patch executable: ${patch_executable}\n" + "Patch arguments: ${patch_arguments}\n" + "${patch_apply_output}\n${patch_apply_error}" + ) + endif() else() - message( - FATAL_ERROR - "Patch failed: ${patch_file}\n" - "Working directory: ${patch_working_directory}\n" - "Patch executable: ${patch_executable}\n" - "Patch arguments: ${patch_arguments}\n" - "${patch_apply_output}\n${patch_apply_error}" + execute_process( + COMMAND "${patch_executable}" --dry-run ${patch_arguments} --reverse + WORKING_DIRECTORY "${patch_working_directory}" + INPUT_FILE "${patch_file}" + RESULT_VARIABLE reverse_check_result + OUTPUT_VARIABLE reverse_check_output + ERROR_VARIABLE reverse_check_error ) + if(reverse_check_result EQUAL 0) + message(STATUS "Patch already applied: ${patch_file}") + else() + message( + FATAL_ERROR + "Patch cannot be applied and is not already applied: ${patch_file}\n" + "Working directory: ${patch_working_directory}\n" + "Patch executable: ${patch_executable}\n" + "Patch arguments: ${patch_arguments}\n" + "${patch_check_output}\n${patch_check_error}\n" + "${reverse_check_output}\n${reverse_check_error}" + ) + endif() endif() else() + message(STATUS "Applying patch: ${patch_file}") execute_process( - COMMAND "${patch_executable}" --dry-run ${patch_arguments} --reverse + COMMAND "${patch_executable}" ${patch_forward_arguments} WORKING_DIRECTORY "${patch_working_directory}" INPUT_FILE "${patch_file}" - RESULT_VARIABLE reverse_check_result - OUTPUT_VARIABLE reverse_check_output - ERROR_VARIABLE reverse_check_error + RESULT_VARIABLE patch_apply_result + OUTPUT_VARIABLE patch_apply_output + ERROR_VARIABLE patch_apply_error ) - if(reverse_check_result EQUAL 0) - message(STATUS "Patch already applied: ${patch_file}") + + if(patch_apply_result EQUAL 0) + message(STATUS "Applied patch: ${patch_file}") else() - message( - FATAL_ERROR - "Patch cannot be applied and is not already applied: ${patch_file}\n" - "Working directory: ${patch_working_directory}\n" - "Patch executable: ${patch_executable}\n" - "Patch arguments: ${patch_arguments}\n" - "${patch_check_output}\n${patch_check_error}\n" - "${reverse_check_output}\n${reverse_check_error}" - ) + set(patch_apply_log "${patch_apply_output}\n${patch_apply_error}") + # -N makes patch exit non-zero when hunks are skipped. Detect already-applied patches by + # matching known output strings: GNU patch emits "Reversed (or previously applied)", + # BSD/macOS patch emits "Skipping patch". + if(patch_apply_log MATCHES "Reversed|previously applied|Skipping patch") + message(STATUS "Patch already applied: ${patch_file}") + else() + message( + FATAL_ERROR + "Patch failed: ${patch_file}\n" + "Working directory: ${patch_working_directory}\n" + "Patch executable: ${patch_executable}\n" + "Patch arguments: ${patch_forward_arguments}\n" + "${patch_apply_log}" + ) + endif() endif() endif() endforeach() @@ -673,8 +724,11 @@ function(cpm_add_patches) endif() set(resolved_patch_files) - set(patch_arguments -p1) - set(patch_set_fingerprint "${PATCH_EXECUTABLE}" "${patch_arguments}" "WORKING_DIRECTORY=.") + cpm_patch_supports_dry_run("${PATCH_EXECUTABLE}" patch_supports_dry_run) + + # Fingerprint: executable path + dry-run capability + working dir marker + each file's path and + # content hash. Any change to the patch set produces a new hash and therefore a new stamp file. + set(patch_set_fingerprint "${PATCH_EXECUTABLE}" "${patch_supports_dry_run}" "WORKING_DIRECTORY=.") foreach(patch_file IN LISTS ARGN) # Resolve relative patch files from the current package list file. @@ -693,7 +747,6 @@ function(cpm_add_patches) list(APPEND patch_set_fingerprint "${patch_file}" "${patch_file_hash}") endforeach() - # Include patch contents in the stamp key. string(REPLACE ";" "\n" patch_set_fingerprint_text "${patch_set_fingerprint}") string(SHA1 patch_set_hash "${patch_set_fingerprint_text}") set(patch_script_directory "${CMAKE_BINARY_DIR}/CPM_scripts") @@ -701,7 +754,8 @@ function(cpm_add_patches) file(MAKE_DIRECTORY "${patch_script_directory}") cpm_write_apply_patches_script( - "${patch_script}" "${PATCH_EXECUTABLE}" "${patch_set_hash}" ${resolved_patch_files} + "${patch_script}" "${PATCH_EXECUTABLE}" "${patch_supports_dry_run}" "${patch_set_hash}" + ${resolved_patch_files} ) list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS PATCH_COMMAND "${CMAKE_COMMAND}" -P "${patch_script}") diff --git a/test/unit/package_patch.cmake b/test/unit/package_patch.cmake index e10e856c..9cd6d942 100644 --- a/test/unit/package_patch.cmake +++ b/test/unit/package_patch.cmake @@ -101,6 +101,7 @@ function(run_test_single_patch) file(REMOVE_RECURSE "${test_dir}/.cpm_patches") run_patch_script("${patch_script}" "${test_dir}") assert_file_content("${test_dir}/input.txt" "new\n") + assert_not_exists("${test_dir}/input.txt.rej") endfunction() # ---------------------------------------------------------------------------------------- @@ -198,6 +199,68 @@ function(run_test_failed_patch_does_not_write_stamp) run_patch_script_expect_failure("${patch_script}" "${test_dir}") assert_not_exists("${test_dir}/.cpm_patches") + assert_not_exists("${test_dir}/input.txt.rej") +endfunction() + +# ---------------------------------------------------------------------------------------- +# Test Case 7: Patch scripts work without dry-run support (forced NO) +# ---------------------------------------------------------------------------------------- +function(run_test_patch_without_dry_run_support) + set(test_dir "${CMAKE_CURRENT_BINARY_DIR}/patch_without_dry_run") + set(patch_file "${test_dir}/change.patch") + set(patch_script "${CMAKE_BINARY_DIR}/CPM_scripts/cpm_apply_patches_no_dry_run_test.cmake") + + find_program(PATCH_EXECUTABLE patch REQUIRED) + + file(REMOVE_RECURSE "${test_dir}") + file(MAKE_DIRECTORY "${test_dir}") + file(WRITE "${test_dir}/input.txt" "old\n") + write_patch("${patch_file}" "old" "new") + + file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/CPM_scripts") + cpm_write_apply_patches_script( + "${patch_script}" "${PATCH_EXECUTABLE}" NO "no_dry_run_test" "${patch_file}" + ) + + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") + + file(REMOVE_RECURSE "${test_dir}/.cpm_patches") + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") + assert_not_exists("${test_dir}/input.txt.rej") +endfunction() + +# ---------------------------------------------------------------------------------------- +# Test Case 8: Patch scripts work with dry-run support (forced YES) +# Only runs on systems where --dry-run is supported (skipped on BSD/macOS patch). +# ---------------------------------------------------------------------------------------- +function(run_test_patch_with_dry_run_support) + set(test_dir "${CMAKE_CURRENT_BINARY_DIR}/patch_with_dry_run") + set(patch_file "${test_dir}/change.patch") + set(patch_script "${CMAKE_BINARY_DIR}/CPM_scripts/cpm_apply_patches_dry_run_test.cmake") + + find_program(PATCH_EXECUTABLE patch REQUIRED) + + file(REMOVE_RECURSE "${test_dir}") + file(MAKE_DIRECTORY "${test_dir}") + file(WRITE "${test_dir}/input.txt" "old\n") + write_patch("${patch_file}" "old" "new") + + file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/CPM_scripts") + cpm_write_apply_patches_script( + "${patch_script}" "${PATCH_EXECUTABLE}" YES "dry_run_test" "${patch_file}" + ) + + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") + + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") + + file(REMOVE_RECURSE "${test_dir}/.cpm_patches") + run_patch_script("${patch_script}" "${test_dir}") + assert_file_content("${test_dir}/input.txt" "new\n") endfunction() function(cleanup_patch_test_files) @@ -207,6 +270,8 @@ function(cleanup_patch_test_files) file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/patch files with spaces") file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/changed_patch_file") file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/failed_patch") + file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/patch_without_dry_run") + file(REMOVE_RECURSE "${CMAKE_CURRENT_BINARY_DIR}/patch_with_dry_run") file(REMOVE_RECURSE "${CMAKE_BINARY_DIR}/CPM_scripts") endfunction() @@ -216,4 +281,14 @@ run_test_multiple_patches() run_test_patch_path_with_spaces() run_test_patch_file_content_change() run_test_failed_patch_does_not_write_stamp() +run_test_patch_without_dry_run_support() + +find_program(_cpm_patch_exe patch QUIET) +if(_cpm_patch_exe) + cpm_patch_supports_dry_run("${_cpm_patch_exe}" _cpm_patch_dry_run) + if(_cpm_patch_dry_run) + run_test_patch_with_dry_run_support() + endif() +endif() + cleanup_patch_test_files()