Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make git revision detection optional #7356

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make git revision detection optional #7356

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 30 additions & 28 deletions cmake/Version.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,39 +119,41 @@ else(CLEMENTINE_VERSION_PRERELEASE)
endif(CLEMENTINE_VERSION_PRERELEASE)

# Add git revision
if(FORCE_GIT_REVISION)
set(GIT_REV ${FORCE_GIT_REVISION})
set(GIT_INFO_RESULT 0)
else(FORCE_GIT_REVISION)
find_program(GIT_EXECUTABLE git)
message(STATUS "Found git: ${GIT_EXECUTABLE}")

if(NOT GIT_EXECUTABLE-NOTFOUND)
execute_process(COMMAND ${GIT_EXECUTABLE} describe
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
RESULT_VARIABLE GIT_INFO_RESULT
OUTPUT_VARIABLE GIT_REV
OUTPUT_STRIP_TRAILING_WHITESPACE)
if(NOT ${GIT_INFO_RESULT} EQUAL 0)
message(SEND_ERROR "git describe failed with code ${GIT_INFO_RESULT}: ${GIT_REV}")
if(INCLUDE_GIT_REVISION)
if(FORCE_GIT_REVISION)
set(GIT_REV ${FORCE_GIT_REVISION})
set(GIT_INFO_RESULT 0)
else(FORCE_GIT_REVISION)
find_program(GIT_EXECUTABLE git)
message(STATUS "Found git: ${GIT_EXECUTABLE}")

if(NOT GIT_EXECUTABLE-NOTFOUND)
Copy link

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
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.

execute_process(COMMAND ${GIT_EXECUTABLE} describe
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
RESULT_VARIABLE GIT_INFO_RESULT
OUTPUT_VARIABLE GIT_REV
OUTPUT_STRIP_TRAILING_WHITESPACE)
if(NOT ${GIT_INFO_RESULT} EQUAL 0)
message(SEND_ERROR "git describe failed with code ${GIT_INFO_RESULT}: ${GIT_REV}")
endif()
endif()
endif()
endif()

string(REGEX REPLACE "^(.+)-([0-9]+)-(g[a-f0-9]+)$" "\\1;\\2;\\3"
GIT_PARTS ${GIT_REV})
string(REGEX REPLACE "^(.+)-([0-9]+)-(g[a-f0-9]+)$" "\\1;\\2;\\3"
GIT_PARTS ${GIT_REV})

if(NOT GIT_PARTS)
message(FATAL_ERROR "Failed to parse git revision string '${GIT_REV}'")
endif(NOT GIT_PARTS)
if(NOT GIT_PARTS)
message(FATAL_ERROR "Failed to parse git revision string '${GIT_REV}'")
endif(NOT GIT_PARTS)

list(LENGTH GIT_PARTS GIT_PARTS_LENGTH)
if(GIT_PARTS_LENGTH EQUAL 3)
list(GET GIT_PARTS 0 GIT_TAGNAME)
list(GET GIT_PARTS 1 GIT_COMMITCOUNT)
list(GET GIT_PARTS 2 GIT_SHA1)
set(HAS_GIT_REVISION ON)
endif(GIT_PARTS_LENGTH EQUAL 3)
list(LENGTH GIT_PARTS GIT_PARTS_LENGTH)
if(GIT_PARTS_LENGTH EQUAL 3)
list(GET GIT_PARTS 0 GIT_TAGNAME)
list(GET GIT_PARTS 1 GIT_COMMITCOUNT)
list(GET GIT_PARTS 2 GIT_SHA1)
set(HAS_GIT_REVISION ON)
endif(GIT_PARTS_LENGTH EQUAL 3)
endif()

if(INCLUDE_GIT_REVISION AND HAS_GIT_REVISION)
set(CLEMENTINE_VERSION_DISPLAY "${GIT_REV}")
Expand Down
Loading