Make git revision detection optional #7356
Conversation
Potentially fixes clementine-player#7355
| find_program(GIT_EXECUTABLE git) | ||
| message(STATUS "Found git: ${GIT_EXECUTABLE}") | ||
|
|
||
| if(NOT GIT_EXECUTABLE-NOTFOUND) |
There was a problem hiding this comment.
This still suffers from the problem I was attempting to fix in #7342. I.e. it shouldn't check for a "NOTFOUND" variable. That variable doesn't exist so this is NOT FALSE i.e. a tautology (always true, making the condition redundant).
It should check for the existence of GIT_EXECUTABLE instead:
| if(NOT GIT_EXECUTABLE-NOTFOUND) | |
| if(GIT_EXECUTABLE) |
See #7342's explanation for more details.
There was a problem hiding this comment.
This may well still cause a problem with certain builds, but it wasn't something that would ever be checked with the release tarballs I'm focusing on. With release tarballs, no git information will be included in the tarball, so INCLUDE_GIT_REVISION should be changed to OFF in those tarballs, and, if my changes are included, would skip over any of the git checking, never running into the check you mentioned.
Since release tarballs generated by GitHub don't include the .git/ folder, they will always fail with the current detection methods.
I haven't looked into the git detection at all since I am the maintainer of this package for SlackBuilds.org and they don't support git cloning when building packages. So your change might still be needed, but it was not looked at in this PR. That line only changed due to indentation when adding the if(INCLUDE_GIT_REVISION) statement before everything else.
With the tagged 1.4.1 source tarball downloaded from github, cmake fails due to forcing detection of a git revision, which doesn't exist from the github generated tarball.
This fix allows one to toggle that using the INCLUDE_GIT_REVISION variable at the top of cmake/Version.cmake, which if set to OFF will skip over git revision detection, allowing the user to complete cmake.
(Currently that INCLUDE_GIT_REVISION variable would need to be changed for a tagged release or have the user change it via sed or similar.)
Potentially fixes #7355