Differential D24641 Diff 69225 modules/ECMSourceVersionControl.cmake

Changeset View

Standalone View

View Options

modules/ECMSourceVersionControl.cmake

Context not available.
12 12​
13#============================================================================= 13​#=============================================================================
14# Copyright 2019 Harald Sitter <sitter@kde.org> 14​# Copyright 2019 Harald Sitter <sitter@kde.org>
15​# Copyright 2019 Thomas Fischer <fischer@unix-ag.uni-kl.de>
15# 16​#
16# Redistribution and use in source and binary forms, with or without 17​# Redistribution and use in source and binary forms, with or without
17# modification, are permitted provided that the following conditions 18​# modification, are permitted provided that the following conditions
Context not available.
36# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 37​# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
37# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 38​# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
38 39​
39if(EXISTS "${CMAKE_SOURCE_DIR}/.git" OR 40​if(EXISTS "${CMAKE_SOURCE_DIR}/.git")
40 EXISTS "${CMAKE_SOURCE_DIR}/.svn" OR 41​ # Git
41 EXISTS "${CMAKE_SOURCE_DIR}/.hg" OR ​
42 EXISTS "${CMAKE_SOURCE_DIR}/.bzr") ​
43 set(ECM_SOURCE_UNDER_VERSION_CONTROL TRUE) 42​ set(ECM_SOURCE_UNDER_VERSION_CONTROL TRUE)
43​ set(ECM_SOURCE_VERSION_CONTROL_WHICH "git")
44​elseif(EXISTS "${CMAKE_SOURCE_DIR}/.svn")
45​ # Subversion
46​ set(ECM_SOURCE_UNDER_VERSION_CONTROL TRUE)
47​ set(ECM_SOURCE_VERSION_CONTROL_WHICH "svn")
48​elseif(EXISTS "${CMAKE_SOURCE_DIR}/.hg")
49​ # Mercurial
50​ set(ECM_SOURCE_UNDER_VERSION_CONTROL TRUE)
51​ set(ECM_SOURCE_VERSION_CONTROL_WHICH "hg")
52​elseif(EXISTS "${CMAKE_SOURCE_DIR}/.bzr")
53​ # Bazaar
54​ set(ECM_SOURCE_UNDER_VERSION_CONTROL TRUE)
55​ set(ECM_SOURCE_VERSION_CONTROL_WHICH "bzr")
44else() 56​else()
57​ unset(ECM_SOURCE_VERSION_CONTROL_WHICH)
45 set(ECM_SOURCE_UNDER_VERSION_CONTROL FALSE) 58​ set(ECM_SOURCE_UNDER_VERSION_CONTROL FALSE)
46endif() 59​endif()
60​
61​set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_NOT_VCS FALSE)
sitterUnsubmitted
Done

Do we really need this? It seems to me we could just spam it for every call, in the grand scheme of things it makes no difference but is less logic one has to worry about when extending this module.

sitter: Do we really need this? It seems to me we could just spam it for every call, in the grand…
62​set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_MISSING_GIT_EXEC FALSE)
63​
64​function(ecm_source_version_control_probe_revison)
65​ if(ECM_SOURCE_VERSION_CONTROL_REVISION)
66​ return()
67​ endif()
68​ if(NOT ${ECM_SOURCE_UNDER_VERSION_CONTROL})
69​ if(NOT ${_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_NOT_VCS})
70​ message("Source directory not managed by a supported version control system (Git)")
sitterUnsubmitted
Done

I think you are leaking this variable into the parent scope. I am not super sure how to deal with this but I think I've seen _prefixes, or you use a function and explicitly forward into the PARENT_SCOPE (https://cmake.org/cmake/help/v3.0/command/set.html).

Alternatively with a multi-function approach I'd probably just make it ECM_SOURCE_VERSION_CONTROL_EXECUTABLE so it only needs finding on the first call.

sitter: I think you are leaking this variable into the parent scope. I am not super sure how to deal…
71​ set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_NOT_VCS TRUE PARENT_SCOPE)
72​ endif()
73​ elseif(${ECM_SOURCE_VERSION_CONTROL_WHICH} STREQUAL "git")
74​ # Git
75​ if(NOT _ECM_SOURCE_VERSION_GIT_EXECUTABLE)
sitterUnsubmitted
Done

I'd move the exec detection and missing reporting into its own helper which gets called by all "public" functions. Currently the logic is duped in both functions.

sitter: I'd move the exec detection and missing reporting into its own helper which gets called by all…
76​ find_program(_ECM_SOURCE_VERSION_GIT_EXECUTABLE
77​ NAMES git.bat git # for Windows, 'git.bat' must be found before 'git'
78​ )
79​ endif()
80​ if(_ECM_SOURCE_VERSION_GIT_EXECUTABLE)
81​ execute_process(
82​ WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}"
83​ COMMAND "${_ECM_SOURCE_VERSION_GIT_EXECUTABLE}" rev-parse --short HEAD
84​ OUTPUT_VARIABLE ECM_SOURCE_VERSION_CONTROL_REVISION
85​ OUTPUT_STRIP_TRAILING_WHITESPACE
86​ )
87​ set(ECM_SOURCE_VERSION_CONTROL_REVISION "${ECM_SOURCE_VERSION_CONTROL_REVISION}" PARENT_SCOPE)
sitterUnsubmitted
Done

I think it's more idiomatic if you accept the variable name as a function argument instead of hardcoding one.

For example find_program, but really most helpers work like that off the top of my head.

sitter: I think it's more idiomatic if you accept the variable name as a function argument instead of…
88​ else()
89​ if(NOT ${_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_MISSING_GIT_EXEC})
90​ message(WARNING "No Git executable found despite .git directory located in source directory")
91​ set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_MISSING_GIT_EXEC TRUE PARENT_SCOPE)
92​ endif()
93​ endif()
94​ else()
95​ message(WARNING "Cannot retrieve revision for version control system '${ECM_SOURCE_VERSION_CONTROL_WHICH}'")
96​ endif()
97​endfunction()
98​
99​function(ecm_source_version_control_probe_branch)
100​ if(ECM_SOURCE_VERSION_CONTROL_BRANCH)
101​ return()
102​ endif()
103​ if(NOT ${ECM_SOURCE_UNDER_VERSION_CONTROL})
104​ if(NOT ${_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_NOT_VCS})
105​ message("Source directory not managed by a supported version control system (Git)")
106​ set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_NOT_VCS TRUE PARENT_SCOPE)
107​ endif()
108​ elseif(${ECM_SOURCE_VERSION_CONTROL_WHICH} STREQUAL "git")
109​ # Git
110​ if(NOT _ECM_SOURCE_VERSION_GIT_EXECUTABLE)
111​ find_program(_ECM_SOURCE_VERSION_GIT_EXECUTABLE
112​ NAMES git.bat git # for Windows, 'git.bat' must be found before 'git'
113​ )
114​ endif()
115​ if(_ECM_SOURCE_VERSION_GIT_EXECUTABLE)
116​ execute_process(
117​ WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}"
118​ COMMAND "${_ECM_SOURCE_VERSION_GIT_EXECUTABLE}" rev-parse --abbrev-ref HEAD
119​ OUTPUT_VARIABLE ECM_SOURCE_VERSION_CONTROL_BRANCH
120​ OUTPUT_STRIP_TRAILING_WHITESPACE
121​ )
122​ set(ECM_SOURCE_VERSION_CONTROL_BRANCH "${ECM_SOURCE_VERSION_CONTROL_BRANCH}" PARENT_SCOPE)
123​ else()
124​ if(NOT ${_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_MISSING_GIT_EXEC})
125​ message(WARNING "No Git executable found despite .git directory located in source directory")
126​ set(_ECM_SOURCE_VERSION_CONTROL_ALREADY_WARNED_MISSING_GIT_EXEC TRUE PARENT_SCOPE)
127​ endif()
128​ endif()
129​ else()
130​ message(WARNING "Cannot retrieve branch for version control system '${ECM_SOURCE_VERSION_CONTROL_WHICH}'")
131​ endif()
132​endfunction()
133​
134​function(ecm_source_version_control_status)
135​ if(${ECM_SOURCE_UNDER_VERSION_CONTROL} AND "${ECM_SOURCE_VERSION_CONTROL_WHICH}" STREQUAL "git")
136​ message(STATUS "Source directory '${CMAKE_SOURCE_DIR}' is under version control by Git.")
137​ ecm_source_version_control_probe_revison()
138​ ecm_source_version_control_probe_branch()
139​ if(ECM_SOURCE_VERSION_CONTROL_BRANCH AND ECM_SOURCE_VERSION_CONTROL_BRANCH)
140​ message(STATUS "The current Git checkout is branch '${ECM_SOURCE_VERSION_CONTROL_BRANCH}' at commit ${ECM_SOURCE_VERSION_CONTROL_REVISION}.")
141​ endif()
142​ endif()
143​endfunction()
Context not available.