Merge topic 'ep-redownload-on-SOURCE_DIR-change'

d3477eba06 ExternalProject: Rerun download on SOURCE_DIR change

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !6926

6 files changed, 108 insertions, 68 deletions

diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake
index 411a1a9..05af822 100644
--- a/Modules/ExternalProject.cmake
+++ b/Modules/ExternalProject.cmake
@@ -2473,10 +2473,13 @@ function(_ep_add_download_command name)
   set(depends)
   set(comment)
   set(work_dir)
+  set(extra_repo_info)
 
   if(cmd_set)
     set(work_dir ${download_dir})
+    set(method custom)
   elseif(cvs_repository)
+    set(method cvs)
     find_package(CVS QUIET)
     if(NOT CVS_EXECUTABLE)
       message(FATAL_ERROR "error: could not find cvs for checkout of ${name}")
@@ -2488,22 +2491,13 @@ function(_ep_add_download_command name)
     endif()
 
     get_property(cvs_tag TARGET ${name} PROPERTY _EP_CVS_TAG)
-
-    set(repository ${cvs_repository})
-    set(module ${cvs_module})
-    set(tag ${cvs_tag})
-    configure_file(
-      "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/ExternalProject/RepositoryInfo.txt.in"
-      "${stamp_dir}/${name}-cvsinfo.txt"
-      @ONLY
-      )
-
     get_filename_component(src_name "${source_dir}" NAME)
     get_filename_component(work_dir "${source_dir}" PATH)
     set(comment "Performing download step (CVS checkout) for '${name}'")
     set(cmd ${CVS_EXECUTABLE} -d ${cvs_repository} -q co ${cvs_tag} -d ${src_name} ${cvs_module})
-    list(APPEND depends ${stamp_dir}/${name}-cvsinfo.txt)
+
   elseif(svn_repository)
+    set(method svn)
     find_package(Subversion QUIET)
     if(NOT Subversion_SVN_EXECUTABLE)
       message(FATAL_ERROR "error: could not find svn for checkout of ${name}")
@@ -2514,15 +2508,6 @@ function(_ep_add_download_command name)
     get_property(svn_password TARGET ${name} PROPERTY _EP_SVN_PASSWORD)
     get_property(svn_trust_cert TARGET ${name} PROPERTY _EP_SVN_TRUST_CERT)
 
-    set(repository "${svn_repository} user=${svn_username} password=${svn_password}")
-    set(module)
-    set(tag ${svn_revision})
-    configure_file(
-      "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/ExternalProject/RepositoryInfo.txt.in"
-      "${stamp_dir}/${name}-svninfo.txt"
-      @ONLY
-      )
-
     get_filename_component(src_name "${source_dir}" NAME)
     get_filename_component(work_dir "${source_dir}" PATH)
     set(comment "Performing download step (SVN checkout) for '${name}'")
@@ -2538,8 +2523,9 @@ function(_ep_add_download_command name)
     endif()
     set(cmd ${Subversion_SVN_EXECUTABLE} co ${svn_repository} ${svn_revision}
       --non-interactive ${svn_trust_cert_args} ${svn_user_pw_args} ${src_name})
-    list(APPEND depends ${stamp_dir}/${name}-svninfo.txt)
+
   elseif(git_repository)
+    set(method git)
     # FetchContent gives us these directly, so don't try to recompute them
     if(NOT GIT_EXECUTABLE OR NOT GIT_VERSION_STRING)
       unset(CMAKE_MODULE_PATH) # Use CMake builtin find module
@@ -2584,21 +2570,21 @@ function(_ep_add_download_command name)
       list(PREPEND git_config advice.detachedHead=false)
     endif()
 
-    # For the download step, and the git clone operation, only the repository
-    # should be recorded in a configured RepositoryInfo file. If the repo
-    # changes, the clone script should be run again. But if only the tag
-    # changes, avoid running the clone script again. Let the 'always' running
-    # update step checkout the new tag.
+    # The command doesn't expose any details, so we need to record additional
+    # information in the RepositoryInfo.txt file. For the download step, only
+    # the things specifically affecting the clone operation should be recorded.
+    # If the repo changes, the clone script should be run again.
+    # But if only the tag changes, avoid running the clone script again.
+    # Let the 'always' running update step checkout the new tag.
     #
-    set(repository ${git_repository})
-    set(module)
-    set(tag ${git_remote_name})
-    configure_file(
-      "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/ExternalProject/RepositoryInfo.txt.in"
-      "${stamp_dir}/${name}-gitinfo.txt"
-      @ONLY
-      )
-
+    set(extra_repo_info
+"repository=${git_repository}
+remote=${git_remote_name}
+init_submodules=${git_init_submodules}
+recurse_submodules=${git_submodules_recurse}
+submodules=${git_submodules}
+CMP0097=${_EP_CMP0097}
+")
     get_filename_component(src_name "${source_dir}" NAME)
     get_filename_component(work_dir "${source_dir}" PATH)
 
@@ -2612,8 +2598,9 @@ function(_ep_add_download_command name)
       )
     set(comment "Performing download step (git clone) for '${name}'")
     set(cmd ${CMAKE_COMMAND} -P ${tmp_dir}/${name}-gitclone.cmake)
-    list(APPEND depends ${stamp_dir}/${name}-gitinfo.txt)
+
   elseif(hg_repository)
+    set(method hg)
     find_package(Hg QUIET)
     if(NOT HG_EXECUTABLE)
       message(FATAL_ERROR "error: could not find hg for clone of ${name}")
@@ -2624,21 +2611,14 @@ function(_ep_add_download_command name)
       set(hg_tag "tip")
     endif()
 
-    # For the download step, and the hg clone operation, only the repository
-    # should be recorded in a configured RepositoryInfo file. If the repo
-    # changes, the clone script should be run again. But if only the tag
-    # changes, avoid running the clone script again. Let the 'always' running
-    # update step checkout the new tag.
+    # The command doesn't expose any details, so we need to record additional
+    # information in the RepositoryInfo.txt file. For the download step, only
+    # the things specifically affecting the clone operation should be recorded.
+    # If the repo changes, the clone script should be run again.
+    # But if only the tag changes, avoid running the clone script again.
+    # Let the 'always' running update step checkout the new tag.
     #
-    set(repository ${hg_repository})
-    set(module)
-    set(tag)
-    configure_file(
-      "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/ExternalProject/RepositoryInfo.txt.in"
-      "${stamp_dir}/${name}-hginfo.txt"
-      @ONLY
-      )
-
+    set(extra_repo_info "repository=${hg_repository}")
     get_filename_component(src_name "${source_dir}" NAME)
     get_filename_component(work_dir "${source_dir}" PATH)
 
@@ -2652,8 +2632,9 @@ function(_ep_add_download_command name)
       )
     set(comment "Performing download step (hg clone) for '${name}'")
     set(cmd ${CMAKE_COMMAND} -P ${tmp_dir}/${name}-hgclone.cmake)
-    list(APPEND depends ${stamp_dir}/${name}-hginfo.txt)
+
   elseif(url)
+    set(method url)
     get_filename_component(work_dir "${source_dir}" PATH)
     get_property(hash TARGET ${name} PROPERTY _EP_URL_HASH)
     _ep_get_hash_regex(_ep_hash_regex)
@@ -2671,15 +2652,10 @@ function(_ep_add_download_command name)
     if(md5 AND NOT hash)
       set(hash "MD5=${md5}")
     endif()
-    set(repository "external project URL")
-    set(module "${url}")
-    set(tag "${hash}")
-    configure_file(
-      "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/ExternalProject/RepositoryInfo.txt.in"
-      "${stamp_dir}/${name}-urlinfo.txt"
-      @ONLY
-      )
-    list(APPEND depends ${stamp_dir}/${name}-urlinfo.txt)
+    set(extra_repo_info
+"url(s)=${url}
+hash=${hash}
+")
 
     list(LENGTH url url_list_length)
     if(NOT "${url_list_length}" STREQUAL "1")
@@ -2700,6 +2676,7 @@ function(_ep_add_download_command name)
         COMMAND ${CMAKE_COMMAND} -E copy_directory ${abs_dir} ${source_dir})
     else()
       get_property(no_extract TARGET "${name}" PROPERTY _EP_DOWNLOAD_NO_EXTRACT)
+      string(APPEND extra_repo_info "no_extract=${no_extract}\n")
       if("${url}" MATCHES "^[a-z]+://")
         # TODO: Should download and extraction be different steps?
         if("x${fname}" STREQUAL "x")
@@ -2754,6 +2731,11 @@ function(_ep_add_download_command name)
         endif ()
         set(comment "Performing download step (${steps}) for '${name}'")
         file(WRITE "${stamp_dir}/verify-${name}.cmake" "") # already verified by 'download_script'
+
+        # Rather than adding everything to the RepositoryInfo.txt file, it is
+        # more robust to just depend on the download script. That way, we will
+        # re-download if any aspect of the download changes.
+        list(APPEND depends "${download_script}")
       else()
         set(file "${url}")
         if (no_extract)
@@ -2782,6 +2764,7 @@ function(_ep_add_download_command name)
       endif ()
     endif()
   else()
+    set(method source_dir)
     _ep_is_dir_empty("${source_dir}" empty)
     if(${empty})
       message(SEND_ERROR
@@ -2799,6 +2782,17 @@ function(_ep_add_download_command name)
     endif()
   endif()
 
+  # We use configure_file() to write the repo_info_file so that the file's
+  # timestamp is not updated if we don't change the contents
+
+  set(repo_info_file ${stamp_dir}/${name}-${method}info.txt)
+  list(APPEND depends ${repo_info_file})
+  configure_file(
+    "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/ExternalProject/RepositoryInfo.txt.in"
+    "${repo_info_file}"
+    @ONLY
+  )
+
   get_property(log TARGET ${name} PROPERTY _EP_LOG_DOWNLOAD)
   if(log)
     set(log LOG 1)
diff --git a/Modules/ExternalProject/RepositoryInfo.txt.in b/Modules/ExternalProject/RepositoryInfo.txt.in
index df8e322..b81850f 100644
--- a/Modules/ExternalProject/RepositoryInfo.txt.in
+++ b/Modules/ExternalProject/RepositoryInfo.txt.in
@@ -1,3 +1,9 @@
-repository='@repository@'
-module='@module@'
-tag='@tag@'
+# This is a generated file and its contents are an internal implementation detail.
+# The download step will be re-executed if anything in this file changes.
+# No other meaning or use of this file is supported.
+
+method=@method@
+command=@cmd@
+source_dir=@source_dir@
+work_dir=@work_dir@
+@extra_repo_info@
diff --git a/Tests/RunCMake/ExternalProject/MultiCommand.cmake b/Tests/RunCMake/ExternalProject/MultiCommand.cmake
index 0849658..3e8bd94 100644
--- a/Tests/RunCMake/ExternalProject/MultiCommand.cmake
+++ b/Tests/RunCMake/ExternalProject/MultiCommand.cmake
@@ -1,5 +1,12 @@
 include(ExternalProject)
 
+# Force all steps to be re-run by removing timestamps from any previous run.
+# This has to happen before we call ExternalProject_Add() because that command
+# writes some files to the stamp directory for recording repository details.
+set(STAMP_DIR ${CMAKE_BINARY_DIR}/multiCommand-prefix/src/multiCommand-stamp)
+file(REMOVE_RECURSE "${STAMP_DIR}")
+file(MAKE_DIRECTORY "${STAMP_DIR}")
+
 # Verify COMMAND keyword is recognized after various *_COMMAND options
 ExternalProject_Add(multiCommand
   DOWNLOAD_COMMAND  "${CMAKE_COMMAND}" -E echo "download 1"
@@ -17,8 +24,3 @@ ExternalProject_Add(multiCommand
   INSTALL_COMMAND   "${CMAKE_COMMAND}" -E echo "install 1"
           COMMAND   "${CMAKE_COMMAND}" -E echo "install 2"
 )
-
-# Force all steps to be re-run by removing timestamps from any previous run
-ExternalProject_Get_Property(multiCommand STAMP_DIR)
-file(REMOVE_RECURSE "${STAMP_DIR}")
-file(MAKE_DIRECTORY "${STAMP_DIR}")
diff --git a/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake b/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
index 48f8b23..fde384f 100644
--- a/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
+++ b/Tests/RunCMake/ExternalProject/RunCMakeTest.cmake
@@ -61,6 +61,23 @@ if(NOT XCODE_VERSION OR XCODE_VERSION VERSION_LESS 12)
 endif()
 run_steps_CMP0114(NEW)
 
+function(__ep_test_source_dir_change)
+  set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/SourceDirChange-build)
+  set(RunCMake_TEST_NO_CLEAN 1)
+  file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}")
+  file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}")
+  run_cmake(SourceDirChange)
+  run_cmake_command(SourceDirChange-build1 ${CMAKE_COMMAND} --build .)
+  # Because some file systems have timestamps with only one second resolution,
+  # we have to ensure we don't re-run the configure stage too quickly after the
+  # first build. Otherwise, the modified RepositoryInfo.txt files the next
+  # configure writes might still have the same timestamp as the previous one.
+  execute_process(COMMAND ${CMAKE_COMMAND} -E sleep 1.125)
+  run_cmake_command(SourceDirChange-change ${CMAKE_COMMAND} -DSOURCE_DIR_CHANGE=YES .)
+  run_cmake_command(SourceDirChange-build2 ${CMAKE_COMMAND} --build .)
+endfunction()
+__ep_test_source_dir_change()
+
 # Run both cmake and build steps. We always do a clean before the
 # build to ensure that the download step re-runs each time.
 function(__ep_test_with_build testName)
diff --git a/Tests/RunCMake/ExternalProject/SourceDirChange-build2-stdout.txt b/Tests/RunCMake/ExternalProject/SourceDirChange-build2-stdout.txt
new file mode 100644
index 0000000..22dac9d
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/SourceDirChange-build2-stdout.txt
@@ -0,0 +1 @@
+Download command executed
diff --git a/Tests/RunCMake/ExternalProject/SourceDirChange.cmake b/Tests/RunCMake/ExternalProject/SourceDirChange.cmake
new file mode 100644
index 0000000..62213cd
--- /dev/null
+++ b/Tests/RunCMake/ExternalProject/SourceDirChange.cmake
@@ -0,0 +1,20 @@
+include(ExternalProject)
+
+file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/first")
+file(MAKE_DIRECTORY "${CMAKE_BINARY_DIR}/second")
+
+if("${SOURCE_DIR_CHANGE}" STREQUAL "")
+  set(source_dir first)
+else()
+  set(source_dir second)
+endif()
+
+ExternalProject_Add(source_dir_change
+  SOURCE_DIR        "${CMAKE_BINARY_DIR}/${source_dir}"
+  DOWNLOAD_COMMAND  "${CMAKE_COMMAND}" -E echo "Download command executed"
+  UPDATE_COMMAND    ""
+  CONFIGURE_COMMAND ""
+  BUILD_COMMAND     ""
+  TEST_COMMAND      ""
+  INSTALL_COMMAND   ""
+)