D24641: Collect more information from version control systems

[Harald Sitter]

sitter added a comment.


  Hm, how about separate functions though? With a single stat any given build still needs N process forks even when they only want 1 value.
  
  In D24641#548394 <https://phabricator.kde.org/D24641#548394>, @thomasfischer wrote:
  
  > To clarify `ECM_SOURCE_VERSION_CONTROL_COMMIT_COUNT`: it counts the number of commits in direct line of succession from the repository's initialization to the current commit. It does not include commits in other branches. Basically the number of commits listed in a plain `git log`. The commit count gives a quick indication of the progress of a repository (or branch) without requiring to look up the repo's commit messages or hashes.
  
  
  I understand. This is super heavy though. I have just run it on all clones I have on this PC and for some repos that takes upwards of half a second to complete... on an SSD, so I also ran a quick check on our CI servers, which use HDDs and there it takes at least half a second with some repos (e.g. kdevelop) going up to 26 seconds (uncached)! I wouldn't mind terribly if the various things got split into their own functions (ecm_source_version_control_revison, ecm_source_version_control_branch, ecm_source_version_control_commit_count... or some names like that) but even then I have to question the use case behind the commit count information. So, what is the use case? What do you do with this information? It occurs to me that if you know the hash you could look up the commit count of that hash should you need it, but I struggle to imagine a scenario where that number is relevant.

INLINE COMMENTS

> ECMSourceVersionControl.cmake:70
> +        message(STATUS "Source directory '${CMAKE_SOURCE_DIR}' is under version control by Git.")
> +        find_program(GIT_EXECUTABLE
> +            NAMES git.bat git # for Windows, 'git.bat' must be found before 'git'

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.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D24641

To: thomasfischer, sitter, kossebau
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20191024/39552fb7/attachment-0001.html>