From e71c2807ba173f80df5b1fceb0d84a0bda4e5e88 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Fri, 29 May 2020 21:40:15 +1000 Subject: ExternalProject: Remote checkout needs to include the remote name Commit 0aea435aa1 (ExternalProject: Provide choice of git update strategies, 2020-02-12) added the git update strategies, but the CHECKOUT strategy was not handling remote refs correctly. The local ref would be checked out instead and no warning or error would have been emitted. The test that should have caught this was also malformed and did not actually move the local master branch as intended. --- Modules/ExternalProject-gitupdate.cmake.in | 117 +++++++++++---------- .../ExternalProjectUpdateTest.cmake | 13 ++- 2 files changed, 74 insertions(+), 56 deletions(-) diff --git a/Modules/ExternalProject-gitupdate.cmake.in b/Modules/ExternalProject-gitupdate.cmake.in index e993c3c..eff39c1 100644 --- a/Modules/ExternalProject-gitupdate.cmake.in +++ b/Modules/ExternalProject-gitupdate.cmake.in @@ -59,7 +59,7 @@ if(error_code OR is_remote_ref OR NOT ("${tag_sha}" STREQUAL "${head_sha}")) message(FATAL_ERROR "Failed to fetch repository '@git_repository@'") endif() - if(is_remote_ref AND NOT "@git_update_strategy@" STREQUAL "CHECKOUT") + if(is_remote_ref) # Check if stash is needed execute_process( COMMAND "@git_EXECUTABLE@" status --porcelain @@ -72,8 +72,8 @@ if(error_code OR is_remote_ref OR NOT ("${tag_sha}" STREQUAL "${head_sha}")) endif() string(LENGTH "${repo_status}" need_stash) - # If not in clean state, stash changes in order to be able to be able to - # perform git pull --rebase + # If not in clean state, stash changes in order to be able to perform a + # rebase or checkout without losing those changes permanently if(need_stash) execute_process( COMMAND "@git_EXECUTABLE@" stash save @git_stash_save_options@ @@ -85,66 +85,77 @@ if(error_code OR is_remote_ref OR NOT ("${tag_sha}" STREQUAL "${head_sha}")) endif() endif() - # Pull changes from the remote branch - execute_process( - COMMAND "@git_EXECUTABLE@" rebase "${git_remote}/${git_tag}" - WORKING_DIRECTORY "@work_dir@" - RESULT_VARIABLE error_code - OUTPUT_VARIABLE rebase_output - ERROR_VARIABLE rebase_output - ) - if(error_code) - # Rebase failed, undo the rebase attempt before continuing + if("@git_update_strategy@" STREQUAL "CHECKOUT") execute_process( - COMMAND "@git_EXECUTABLE@" rebase --abort - WORKING_DIRECTORY "@work_dir@" - ) - - if(NOT "@git_update_strategy@" STREQUAL "REBASE_CHECKOUT") - # Not allowed to do a checkout as a fallback, so cannot proceed - if(need_stash) - execute_process( - COMMAND "@git_EXECUTABLE@" stash pop --index --quiet - WORKING_DIRECTORY "@work_dir@" - ) - endif() - message(FATAL_ERROR "\nFailed to rebase in: '@work_dir@'." - "\nOutput from the attempted rebase follows:" - "\n${rebase_output}" - "\n\nYou will have to resolve the conflicts manually") - endif() - - # Fall back to checkout. We create an annotated tag so that the user - # can manually inspect the situation and revert if required. - # We can't log the failed rebase output because MSVC sees it and - # intervenes, causing the build to fail even though it completes. - # Write it to a file instead. - string(TIMESTAMP tag_timestamp "%Y%m%dT%H%M%S" UTC) - set(tag_name _cmake_ExternalProject_moved_from_here_${tag_timestamp}Z) - set(error_log_file ${CMAKE_CURRENT_LIST_DIR}/rebase_error_${tag_timestamp}Z.log) - file(WRITE ${error_log_file} "${rebase_output}") - message(WARNING "Rebase failed, output has been saved to ${error_log_file}" - "\nFalling back to checkout, previous commit tagged as ${tag_name}") - execute_process( - COMMAND "@git_EXECUTABLE@" tag -a - -m "ExternalProject attempting to move from here to ${git_remote}/${git_tag}" - ${tag_name} + COMMAND "@git_EXECUTABLE@" checkout "${git_remote}/${git_tag}" WORKING_DIRECTORY "@work_dir@" RESULT_VARIABLE error_code - ) + ) if(error_code) - message(FATAL_ERROR "Failed to add marker tag") + message(FATAL_ERROR "Failed to checkout tag: '${git_remote}/${git_tag}'") endif() - + else() + # Pull changes from the remote branch execute_process( - COMMAND "@git_EXECUTABLE@" checkout ${git_remote}/${git_tag} + COMMAND "@git_EXECUTABLE@" rebase "${git_remote}/${git_tag}" WORKING_DIRECTORY "@work_dir@" RESULT_VARIABLE error_code - ) + OUTPUT_VARIABLE rebase_output + ERROR_VARIABLE rebase_output + ) if(error_code) - message(FATAL_ERROR "Failed to checkout : '${git_remote}/${git_tag}'") - endif() + # Rebase failed, undo the rebase attempt before continuing + execute_process( + COMMAND "@git_EXECUTABLE@" rebase --abort + WORKING_DIRECTORY "@work_dir@" + ) + + if(NOT "@git_update_strategy@" STREQUAL "REBASE_CHECKOUT") + # Not allowed to do a checkout as a fallback, so cannot proceed + if(need_stash) + execute_process( + COMMAND "@git_EXECUTABLE@" stash pop --index --quiet + WORKING_DIRECTORY "@work_dir@" + ) + endif() + message(FATAL_ERROR "\nFailed to rebase in: '@work_dir@'." + "\nOutput from the attempted rebase follows:" + "\n${rebase_output}" + "\n\nYou will have to resolve the conflicts manually") + endif() + + # Fall back to checkout. We create an annotated tag so that the user + # can manually inspect the situation and revert if required. + # We can't log the failed rebase output because MSVC sees it and + # intervenes, causing the build to fail even though it completes. + # Write it to a file instead. + string(TIMESTAMP tag_timestamp "%Y%m%dT%H%M%S" UTC) + set(tag_name _cmake_ExternalProject_moved_from_here_${tag_timestamp}Z) + set(error_log_file ${CMAKE_CURRENT_LIST_DIR}/rebase_error_${tag_timestamp}Z.log) + file(WRITE ${error_log_file} "${rebase_output}") + message(WARNING "Rebase failed, output has been saved to ${error_log_file}" + "\nFalling back to checkout, previous commit tagged as ${tag_name}") + execute_process( + COMMAND "@git_EXECUTABLE@" tag -a + -m "ExternalProject attempting to move from here to ${git_remote}/${git_tag}" + ${tag_name} + WORKING_DIRECTORY "@work_dir@" + RESULT_VARIABLE error_code + ) + if(error_code) + message(FATAL_ERROR "Failed to add marker tag") + endif() + + execute_process( + COMMAND "@git_EXECUTABLE@" checkout "${git_remote}/${git_tag}" + WORKING_DIRECTORY "@work_dir@" + RESULT_VARIABLE error_code + ) + if(error_code) + message(FATAL_ERROR "Failed to checkout : '${git_remote}/${git_tag}'") + endif() + endif() endif() if(need_stash) diff --git a/Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake b/Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake index ba0c598..ddc513f 100644 --- a/Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake +++ b/Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake @@ -199,8 +199,14 @@ if(do_git_tests) foreach(strategy IN ITEMS CHECKOUT REBASE_CHECKOUT) # Move local master back, then apply a change that will cause a conflict - # during rebase. We want to test the fallback to checkout. - check_a_tag(master 5842b503ba4113976d9bb28d57b5aee1ad2736b7 1 REBASE) + # during rebase + execute_process(COMMAND ${GIT_EXECUTABLE} checkout master + WORKING_DIRECTORY ${ExternalProjectUpdate_BINARY_DIR}/CMakeExternals/Source/TutorialStep1-GIT + RESULT_VARIABLE error_code + ) + if(error_code) + message(FATAL_ERROR "Could not reset local master back to tag1.") + endif() execute_process(COMMAND ${GIT_EXECUTABLE} reset --hard tag1 WORKING_DIRECTORY ${ExternalProjectUpdate_BINARY_DIR}/CMakeExternals/Source/TutorialStep1-GIT RESULT_VARIABLE error_code @@ -208,6 +214,7 @@ if(do_git_tests) if(error_code) message(FATAL_ERROR "Could not reset local master back to tag1.") endif() + set(cmlFile ${ExternalProjectUpdate_BINARY_DIR}/CMakeExternals/Source/TutorialStep1-GIT/CMakeLists.txt) file(READ ${cmlFile} contents) string(REPLACE "find TutorialConfig.h" "find TutorialConfig.h (conflict here)" @@ -222,7 +229,7 @@ if(do_git_tests) message(FATAL_ERROR "Could not commit conflicting change.") endif() # This should discard our commit but leave behind an annotated tag - check_a_tag(master 5842b503ba4113976d9bb28d57b5aee1ad2736b7 1 ${strategy}) + check_a_tag(origin/master 5842b503ba4113976d9bb28d57b5aee1ad2736b7 1 ${strategy}) endforeach() endif() -- cgit v0.12 From 1236590507c38ae85847257e41776d0a9935024b Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Fri, 29 May 2020 21:14:47 +1000 Subject: FetchContent: Pass through CMAKE_EP_GIT_REMOTE_UPDATE_STRATEGY if set This was missed when the initial support was added in commit 0aea435aa1 (ExternalProject: Provide choice of git update strategies, 2020-02-12) --- Modules/FetchContent.cmake | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/Modules/FetchContent.cmake b/Modules/FetchContent.cmake index 69f2513..e05ca96 100644 --- a/Modules/FetchContent.cmake +++ b/Modules/FetchContent.cmake @@ -930,16 +930,16 @@ ExternalProject_Add_Step(${contentName}-populate copyfile endif() if(CMAKE_GENERATOR) - set(generatorOpts "-G${CMAKE_GENERATOR}") + set(subCMakeOpts "-G${CMAKE_GENERATOR}") if(CMAKE_GENERATOR_PLATFORM) - list(APPEND generatorOpts "-A${CMAKE_GENERATOR_PLATFORM}") + list(APPEND subCMakeOpts "-A${CMAKE_GENERATOR_PLATFORM}") endif() if(CMAKE_GENERATOR_TOOLSET) - list(APPEND generatorOpts "-T${CMAKE_GENERATOR_TOOLSET}") + list(APPEND subCMakeOpts "-T${CMAKE_GENERATOR_TOOLSET}") endif() if(CMAKE_MAKE_PROGRAM) - list(APPEND generatorOpts "-DCMAKE_MAKE_PROGRAM:FILEPATH=${CMAKE_MAKE_PROGRAM}") + list(APPEND subCMakeOpts "-DCMAKE_MAKE_PROGRAM:FILEPATH=${CMAKE_MAKE_PROGRAM}") endif() else() @@ -947,7 +947,12 @@ ExternalProject_Add_Step(${contentName}-populate copyfile # generator is set (and hence CMAKE_MAKE_PROGRAM could not be # trusted even if provided). We will have to rely on being # able to find the default generator and build tool. - unset(generatorOpts) + unset(subCMakeOpts) + endif() + + if(DEFINED CMAKE_EP_GIT_REMOTE_UPDATE_STRATEGY) + list(APPEND subCMakeOpts + "-DCMAKE_EP_GIT_REMOTE_UPDATE_STRATEGY=${CMAKE_EP_GIT_REMOTE_UPDATE_STRATEGY}") endif() # Create and build a separate CMake project to carry out the population. @@ -958,7 +963,7 @@ ExternalProject_Add_Step(${contentName}-populate copyfile configure_file("${CMAKE_CURRENT_FUNCTION_LIST_DIR}/FetchContent/CMakeLists.cmake.in" "${ARG_SUBBUILD_DIR}/CMakeLists.txt") execute_process( - COMMAND ${CMAKE_COMMAND} ${generatorOpts} . + COMMAND ${CMAKE_COMMAND} ${subCMakeOpts} . RESULT_VARIABLE result ${outputOptions} WORKING_DIRECTORY "${ARG_SUBBUILD_DIR}" -- cgit v0.12 From 8aa4d51ec5695777168dcea02113482bf02e307f Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Fri, 29 May 2020 19:06:59 +1000 Subject: ExternalProject: Add missing release note for git update strategy This release note was mistakenly omitted from commit 0aea435aa1 (ExternalProject: Provide choice of git update strategies, 2020-02-12). --- Help/release/dev/fc-ep-git-update-strategy.rst | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 Help/release/dev/fc-ep-git-update-strategy.rst diff --git a/Help/release/dev/fc-ep-git-update-strategy.rst b/Help/release/dev/fc-ep-git-update-strategy.rst new file mode 100644 index 0000000..b48fdcf --- /dev/null +++ b/Help/release/dev/fc-ep-git-update-strategy.rst @@ -0,0 +1,9 @@ +fc-ep-git-update-strategy +------------------------- + +* The :command:`ExternalProject_Add` command gained a new + ``GIT_REMOTE_UPDATE_STRATEGY`` keyword. This can be used to specify how + failed rebase operations during a git update should be handled. + The ``CMAKE_EP_GIT_REMOTE_UPDATE_STRATEGY`` variable was also added as a + global default and is honored by both the :module:`ExternalProject` and + :module:`FetchContent` modules. -- cgit v0.12