ExternalProject: Avoid reconfigure when updates are disconnected

When UPDATE_DISCONNECTED is true, create separate update_disconnected
and patch_disconnected targets which have ALWAYS set to false.
Make the configure step depend on patch_disconnected in this case too.
This ensures the configure, build, install and test steps are not
executed unnecessarily when updates are disconnected.

Make the update and patch commands depend on the details of those
steps. This ensures they are re-executed when any of those details
change, even if updates are disconnected.

Allow updates to occur even if UPDATE_DISCONNECTED is true, but don't
contact the remote in that case. If asked to update to a ref that isn't known
locally, that is now detected and causes a fatal error when updates are
disconnected. Previously, the build would have silently and erroneously
continued to use the old ref.

Fixes: #16419, #19703, #21146

3 files changed, 49 insertions, 23 deletions

diff --git a/Modules/ExternalProject/PatchInfo.txt.in b/Modules/ExternalProject/PatchInfo.txt.in
new file mode 100644
index 0000000..112953c
--- /dev/null
+++ b/Modules/ExternalProject/PatchInfo.txt.in
@@ -0,0 +1,6 @@
+# This is a generated file and its contents are an internal implementation detail.
+# The update step will be re-executed if anything in this file changes.
+# No other meaning or use of this file is supported.
+
+command=@cmd@
+work_dir=@work_dir@
diff --git a/Modules/ExternalProject/UpdateInfo.txt.in b/Modules/ExternalProject/UpdateInfo.txt.in
new file mode 100644
index 0000000..67ee434
--- /dev/null
+++ b/Modules/ExternalProject/UpdateInfo.txt.in
@@ -0,0 +1,7 @@
+# This is a generated file and its contents are an internal implementation detail.
+# The patch step will be re-executed if anything in this file changes.
+# No other meaning or use of this file is supported.
+
+command (connected)=@cmd@
+command (disconnected)=@cmd_disconnected@
+work_dir=@work_dir@
diff --git a/Modules/ExternalProject/gitupdate.cmake.in b/Modules/ExternalProject/gitupdate.cmake.in
index 50f0167..eb3cda7 100644
--- a/Modules/ExternalProject/gitupdate.cmake.in
+++ b/Modules/ExternalProject/gitupdate.cmake.in
@@ -3,6 +3,15 @@
 
 cmake_minimum_required(VERSION 3.5)
 
+function(do_fetch)
+  message(VERBOSE "Fetching latest from the remote @git_remote_name@")
+  execute_process(
+    COMMAND "@git_EXECUTABLE@" --git-dir=.git fetch --tags --force "@git_remote_name@"
+    WORKING_DIRECTORY "@work_dir@"
+    COMMAND_ERROR_IS_FATAL LAST
+  )
+endfunction()
+
 function(get_hash_for_ref ref out_var err_var)
   execute_process(
     COMMAND "@git_EXECUTABLE@" --git-dir=.git rev-parse "${ref}^0"
@@ -33,17 +42,16 @@ execute_process(
 )
 if(show_ref_output MATCHES "^[a-z0-9]+[ \\t]+refs/remotes/")
   # Given a full remote/branch-name and we know about it already. Since
-  # branches can move around, we always have to fetch.
-  set(fetch_required YES)
+  # branches can move around, we should always fetch, if permitted.
+  if(can_fetch)
+    do_fetch()
+  endif()
   set(checkout_name "@git_tag@")
 
 elseif(show_ref_output MATCHES "^[a-z0-9]+[ \\t]+refs/tags/")
   # Given a tag name that we already know about. We don't know if the tag we
-  # have matches the remote though (tags can move), so we should fetch.
-  set(fetch_required YES)
-  set(checkout_name "@git_tag@")
-
-  # Special case to preserve backward compatibility: if we are already at the
+  # have matches the remote though (tags can move), so we should fetch. As a
+  # special case to preserve backward compatibility, if we are already at the
   # same commit as the tag we hold locally, don't do a fetch and assume the tag
   # hasn't moved on the remote.
   # FIXME: We should provide an option to always fetch for this case
@@ -53,12 +61,20 @@ elseif(show_ref_output MATCHES "^[a-z0-9]+[ \\t]+refs/tags/")
     return()
   endif()
 
+  if(can_fetch)
+    do_fetch()
+  endif()
+  set(checkout_name "@git_tag@")
+
 elseif(show_ref_output MATCHES "^[a-z0-9]+[ \\t]+refs/heads/")
   # Given a branch name without any remote and we already have a branch by that
   # name. We might already have that branch checked out or it might be a
-  # different branch. It isn't safe to use a bare branch name without the
-  # remote, so do a fetch and replace the ref with one that includes the remote.
-  set(fetch_required YES)
+  # different branch. It isn't fully safe to use a bare branch name without the
+  # remote, so do a fetch (if allowed) and replace the ref with one that
+  # includes the remote.
+  if(can_fetch)
+    do_fetch()
+  endif()
   set(checkout_name "@git_remote_name@/@git_tag@")
 
 else()
@@ -70,20 +86,26 @@ else()
 
   elseif(tag_sha STREQUAL "")
     # We don't know about this ref yet, so we have no choice but to fetch.
+    if(NOT can_fetch)
+      message(FATAL_ERROR
+        "Requested git ref \"@git_tag@\" is not present locally, and not "
+        "allowed to contact remote due to UPDATE_DISCONNECTED setting."
+      )
+    endif()
+
     # We deliberately swallow any error message at the default log level
     # because it can be confusing for users to see a failed git command.
     # That failure is being handled here, so it isn't an error.
-    set(fetch_required YES)
-    set(checkout_name "@git_tag@")
     if(NOT error_msg STREQUAL "")
       message(VERBOSE "${error_msg}")
     endif()
+    do_fetch()
+    set(checkout_name "@git_tag@")
 
   else()
     # We have the commit, so we know we were asked to find a commit hash
     # (otherwise it would have been handled further above), but we don't
-    # have that commit checked out yet
-    set(fetch_required NO)
+    # have that commit checked out yet. We don't need to fetch from the remote.
     set(checkout_name "@git_tag@")
     if(NOT error_msg STREQUAL "")
       message(WARNING "${error_msg}")
@@ -92,15 +114,6 @@ else()
   endif()
 endif()
 
-if(fetch_required)
-  message(VERBOSE "Fetching latest from the remote @git_remote_name@")
-  execute_process(
-    COMMAND "@git_EXECUTABLE@" --git-dir=.git fetch --tags --force "@git_remote_name@"
-    WORKING_DIRECTORY "@work_dir@"
-    COMMAND_ERROR_IS_FATAL ANY
-  )
-endif()
-
 set(git_update_strategy "@git_update_strategy@")
 if(git_update_strategy STREQUAL "")
   # Backward compatibility requires REBASE as the default behavior