Proposed CMake improvements

[garybuhrmaster]

Proposed CMake improvements

---

The current mythtv/CMakeLists.txt for CMake builds requires a branch name.  However, the calculation of the branch name in cmake/SourceVersion.cmake explicitly unsets the branch name in some cases, and when one is in detached head mode (for example, doing a git bisect) the branch name is not properly calculated (second part of the proposal addresses that)

The branch name should not be required.

Proposed patch:

diff --git a/mythtv/CMakeLists.txt b/mythtv/CMakeLists.txt
index 9d5c0a22db..d0f0cd4e97 100644
--- a/mythtv/CMakeLists.txt
+++ b/mythtv/CMakeLists.txt
@@ -13,8 +13,7 @@ cmake_minimum_required(VERSION 3.20)
 #
 # Validate parameters
 #
-foreach(_param IN ITEMS SUPER_SOURCE_DIR SUPER_VERSION MYTHTV_SOURCE_VERSION
-                        MYTHTV_SOURCE_BRANCH)
+foreach(_param IN ITEMS SUPER_SOURCE_DIR SUPER_VERSION MYTHTV_SOURCE_VERSION)
   if(${_param} STREQUAL "")
     message(FATAL_ERROR "${_param} is a required parameter")
   endif()

---

The current cmake/SourceVersion.cmake file deviates from the existing configure script in calculating the branch name using different git commands.  While those changes make sense in theory, this results in the branch name being empty in detached head mode (for example during a git bisect).

Attempt to find branch name consistent with past configure

Proposed patch:

diff --git a/cmake/SourceVersion.cmake b/cmake/SourceVersion.cmake
index 51a2472c6f..1fd481444a 100644
--- a/cmake/SourceVersion.cmake
+++ b/cmake/SourceVersion.cmake
@@ -118,6 +118,20 @@ function(mythtv_find_source_version version version_major branch)
       OUTPUT_VARIABLE branchName
       OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET)

+    # While this command appears to work for all cases,
+    # it does depend on the specific git output format,
+    # so use it only if the simplier command does not.
+    if(NOT branchName)
+      execute_process(
+        COMMAND ${GIT_EXECUTABLE} branch --no-color --omit-empty --sort=refname --contains HEAD --merged HEAD
+        OUTPUT_VARIABLE branchNames
+        OUTPUT_STRIP_TRAILING_WHITESPACE ERROR_QUIET)
+      string(REGEX MATCHALL "[^\n\r]+" branchNamesList ${branchNames})
+      list(GET branchNamesList 0 branchName)
+      string(REGEX REPLACE "^[\\*\\+] " "" branchName ${branchName})
+      string(STRIP ${branchName} branchName)
+    endif()
+
     if(versionString)
       message(STATUS "  version:${versionString} branch:${branchName}")
     endif()

[linuxdude42]

The MYTHTV_SOURCE_BRANCH value passed into mythtv/CMakeLists.txt is required. Its used when creating the version.h file from version.h.in. I tried to recreate the logic used by the old build system version.sh shell script in the SourceVersion.cmake file. Maybe I missed something? Does the old build system have the same problem when doing a bisect?

[garybuhrmaster]

The MYTHTV_SOURCE_BRANCH value passed into mythtv/CMakeLists.txt is required. Its used when creating the version.h file from version.h.in. I tried to recreate the logic used by the old build system version.sh shell script in the SourceVersion.cmake file. Maybe I missed something? Does the old build system have the same problem when doing a bisect?

My recollection is yes (but most importantly, it would always build, and although I think it might have generated "unknown" in at least some cases). However, no matter, if you are going to enforce a branch name you need to fix the generation when a branch name cannot be obtained because the source is not git-ish (so at least default to some form of "unknown"). The second part of the proposed patch handles more cases, but there are still going to be cases of unable to determine the branch (because it is not a git-ish source).