From 315a200f0c45415e8ab0da058aab9bbfa39c3c05 Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Wed, 20 Jan 2021 10:15:45 +1100 Subject: FindGit: Cache the GIT_EXECUTABLE version for the current run The git version should not change while CMake is running. When using FetchContent with many dependencies, the repeated calls to get the git version every time ExternalProject is used can be measurable on some platforms. This commit queries that version only once and then caches it in a global property for the rest of that run. The git version can still safely change between runs because it is not cached, only the GIT_EXECUTABLE location is cached. Relates: #21703 --- Modules/ExternalProject.cmake | 22 ++++++++++------ Modules/FetchContent.cmake | 16 ++++++++++++ Modules/FetchContent/CMakeLists.cmake.in | 2 ++ Modules/FindGit.cmake | 44 +++++++++++++++++++++++++++----- 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/Modules/ExternalProject.cmake b/Modules/ExternalProject.cmake index 3307bc6..1e50c6d 100644 --- a/Modules/ExternalProject.cmake +++ b/Modules/ExternalProject.cmake @@ -2630,10 +2630,13 @@ function(_ep_add_download_command name) --non-interactive ${svn_trust_cert_args} ${svn_user_pw_args} ${src_name}) list(APPEND depends ${stamp_dir}/${name}-svninfo.txt) elseif(git_repository) - unset(CMAKE_MODULE_PATH) # Use CMake builtin find module - find_package(Git QUIET) - if(NOT GIT_EXECUTABLE) - message(FATAL_ERROR "error: could not find git for clone of ${name}") + # 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 + find_package(Git QUIET) + if(NOT GIT_EXECUTABLE) + message(FATAL_ERROR "error: could not find git for clone of ${name}") + endif() endif() _ep_get_git_submodules_recurse(git_submodules_recurse) @@ -2951,10 +2954,13 @@ function(_ep_add_update_command name) --non-interactive ${svn_trust_cert_args} ${svn_user_pw_args}) set(always 1) elseif(git_repository) - unset(CMAKE_MODULE_PATH) # Use CMake builtin find module - find_package(Git QUIET) - if(NOT GIT_EXECUTABLE) - message(FATAL_ERROR "error: could not find git for fetch of ${name}") + # 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 + find_package(Git QUIET) + if(NOT GIT_EXECUTABLE) + message(FATAL_ERROR "error: could not find git for fetch of ${name}") + endif() endif() set(work_dir ${source_dir}) set(comment "Performing update step for '${name}'") diff --git a/Modules/FetchContent.cmake b/Modules/FetchContent.cmake index ba2acfc..8adef47 100644 --- a/Modules/FetchContent.cmake +++ b/Modules/FetchContent.cmake @@ -964,6 +964,22 @@ ExternalProject_Add_Step(${contentName}-populate copyfile "-DCMAKE_EP_GIT_REMOTE_UPDATE_STRATEGY=${CMAKE_EP_GIT_REMOTE_UPDATE_STRATEGY}") endif() + # Avoid using if(... IN_LIST ...) so we don't have to alter policy settings + set(__FETCHCONTENT_CACHED_INFO "") + list(FIND ARG_UNPARSED_ARGUMENTS GIT_REPOSITORY indexResult) + if(indexResult GREATER_EQUAL 0) + find_package(Git QUIET) + set(__FETCHCONTENT_CACHED_INFO +"# Pass through things we've already detected in the main project to avoid +# paying the cost of redetecting them again in ExternalProject_Add() +set(GIT_EXECUTABLE [==[${GIT_EXECUTABLE}]==]) +set(GIT_VERSION_STRING [==[${GIT_VERSION_STRING}]==]) +set_property(GLOBAL PROPERTY _CMAKE_FindGit_GIT_EXECUTABLE_VERSION + [==[${GIT_EXECUTABLE};${GIT_VERSION_STRING}]==] +) +") + endif() + # Create and build a separate CMake project to carry out the population. # If we've already previously done these steps, they will not cause # anything to be updated, so extra rebuilds of the project won't occur. diff --git a/Modules/FetchContent/CMakeLists.cmake.in b/Modules/FetchContent/CMakeLists.cmake.in index 45e4df0..5ebb12f 100644 --- a/Modules/FetchContent/CMakeLists.cmake.in +++ b/Modules/FetchContent/CMakeLists.cmake.in @@ -9,6 +9,8 @@ cmake_minimum_required(VERSION ${CMAKE_VERSION}) project(${contentName}-populate NONE) +@__FETCHCONTENT_CACHED_INFO@ + include(ExternalProject) ExternalProject_Add(${contentName}-populate ${ARG_EXTRA} diff --git a/Modules/FindGit.cmake b/Modules/FindGit.cmake index f8346b6..83da707 100644 --- a/Modules/FindGit.cmake +++ b/Modules/FindGit.cmake @@ -77,14 +77,44 @@ unset(git_names) unset(_git_sourcetree_path) if(GIT_EXECUTABLE) - execute_process(COMMAND ${GIT_EXECUTABLE} --version - OUTPUT_VARIABLE git_version - ERROR_QUIET - OUTPUT_STRIP_TRAILING_WHITESPACE) - if (git_version MATCHES "^git version [0-9]") - string(REPLACE "git version " "" GIT_VERSION_STRING "${git_version}") + # Avoid querying the version if we've already done that this run. For + # projects that use things like ExternalProject or FetchContent heavily, + # this saving can be measurable on some platforms. + set(__doGitVersionCheck YES) + if(DEFINED GIT_VERSION_STRING) + # This is an internal property, projects must not try to use it. + # We don't want this stored in the cache because it might still change + # between CMake runs, but it shouldn't change during a run. + get_property(__gitVersionProp GLOBAL + PROPERTY _CMAKE_FindGit_GIT_EXECUTABLE_VERSION + ) + if(__gitVersionProp) + list(GET __gitVersionProp 0 __gitExe) + list(GET __gitVersionProp 1 __gitVersion) + if("${__gitExe}" STREQUAL "${GIT_EXECUTABLE}" AND + "${__gitVersion}" STREQUAL "${GIT_VERSION_STRING}") + set(__doGitVersionCheck NO) + endif() + endif() + unset(__gitVersionProp) + unset(__gitExe) + unset(__gitVersion) + endif() + + if(__doGitVersionCheck) + execute_process(COMMAND ${GIT_EXECUTABLE} --version + OUTPUT_VARIABLE git_version + ERROR_QUIET + OUTPUT_STRIP_TRAILING_WHITESPACE) + if (git_version MATCHES "^git version [0-9]") + string(REPLACE "git version " "" GIT_VERSION_STRING "${git_version}") + set_property(GLOBAL PROPERTY _CMAKE_FindGit_GIT_EXECUTABLE_VERSION + "${GIT_EXECUTABLE};${GIT_VERSION_STRING}" + ) + endif() + unset(git_version) endif() - unset(git_version) + unset(__doGitVersionCheck) get_property(_findgit_role GLOBAL PROPERTY CMAKE_ROLE) if(_findgit_role STREQUAL "PROJECT" AND NOT TARGET Git::Git) -- cgit v0.12 From 76fdeb6d13c4cfdaac8e933a9ef94c8e9def54ef Mon Sep 17 00:00:00 2001 From: Craig Scott Date: Sat, 16 Jan 2021 15:50:13 +1100 Subject: Tests: FindGit already provides the git version, re-use it --- Tests/ExternalProject/CMakeLists.txt | 10 ++-------- Tests/ExternalProjectUpdate/CMakeLists.txt | 12 +++--------- Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake | 10 ++-------- 3 files changed, 7 insertions(+), 25 deletions(-) diff --git a/Tests/ExternalProject/CMakeLists.txt b/Tests/ExternalProject/CMakeLists.txt index a1130dc..8fd44b4 100644 --- a/Tests/ExternalProject/CMakeLists.txt +++ b/Tests/ExternalProject/CMakeLists.txt @@ -298,15 +298,9 @@ set(do_git_tests 0) if(GIT_EXECUTABLE) set(do_git_tests 1) - execute_process( - COMMAND "${GIT_EXECUTABLE}" --version - OUTPUT_VARIABLE ov - OUTPUT_STRIP_TRAILING_WHITESPACE - ) - string(REGEX REPLACE "^git version (.+)$" "\\1" git_version "${ov}") - message(STATUS "git_version='${git_version}'") + message(STATUS "GIT_VERSION_STRING='${GIT_VERSION_STRING}'") - if(git_version VERSION_LESS 1.6.5) + if("${GIT_VERSION_STRING}" VERSION_LESS 1.6.5) message(STATUS "No ExternalProject git tests with git client less than version 1.6.5") set(do_git_tests 0) endif() diff --git a/Tests/ExternalProjectUpdate/CMakeLists.txt b/Tests/ExternalProjectUpdate/CMakeLists.txt index fc10c67..b31a38b 100644 --- a/Tests/ExternalProjectUpdate/CMakeLists.txt +++ b/Tests/ExternalProjectUpdate/CMakeLists.txt @@ -51,15 +51,9 @@ set(do_git_tests 0) if(GIT_EXECUTABLE) set(do_git_tests 1) - execute_process( - COMMAND "${GIT_EXECUTABLE}" --version - OUTPUT_VARIABLE ov - OUTPUT_STRIP_TRAILING_WHITESPACE - ) - string(REGEX REPLACE "^git version (.+)$" "\\1" git_version "${ov}") - message(STATUS "git_version='${git_version}'") - - if(git_version VERSION_LESS 1.6.5) + message(STATUS "GIT_VERSION_STRING='${GIT_VERSION_STRING}'") + + if("${GIT_VERSION_STRING}" VERSION_LESS 1.6.5) message(STATUS "No ExternalProject git tests with git client less than version 1.6.5") set(do_git_tests 0) endif() diff --git a/Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake b/Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake index ddc513f..4b1d074 100644 --- a/Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake +++ b/Tests/ExternalProjectUpdate/ExternalProjectUpdateTest.cmake @@ -167,15 +167,9 @@ set(do_git_tests 0) if(GIT_EXECUTABLE) set(do_git_tests 1) - execute_process( - COMMAND "${GIT_EXECUTABLE}" --version - OUTPUT_VARIABLE ov - OUTPUT_STRIP_TRAILING_WHITESPACE - ) - string(REGEX REPLACE "^git version (.+)$" "\\1" git_version "${ov}") - message(STATUS "git_version='${git_version}'") + message(STATUS "GIT_VERSION_STRING='${GIT_VERSION_STRING}'") - if(git_version VERSION_LESS 1.6.5) + if("${GIT_VERSION_STRING}" VERSION_LESS 1.6.5) message(STATUS "No ExternalProject git tests with git client less than version 1.6.5") set(do_git_tests 0) endif() -- cgit v0.12