Skip to content

Make git revision detection optional#7356

Open
bassmadrigal wants to merge 1 commit intoclementine-player:masterfrom
bassmadrigal:master
Open

Make git revision detection optional#7356
bassmadrigal wants to merge 1 commit intoclementine-player:masterfrom
bassmadrigal:master

Conversation

@bassmadrigal
Copy link
Copy Markdown

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

find_program(GIT_EXECUTABLE git)
message(STATUS "Found git: ${GIT_EXECUTABLE}")

if(NOT GIT_EXECUTABLE-NOTFOUND)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
if(NOT GIT_EXECUTABLE-NOTFOUND)
if(GIT_EXECUTABLE)

See #7342's explanation for more details.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tag 1.4.1 fails to build from source tarball

2 participants