-
Notifications
You must be signed in to change notification settings - Fork 201
Allow to use FetchContent to get CPM #437
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
cmake_minimum_required(VERSION 3.14 FATAL_ERROR) | ||
|
||
find_package(Git REQUIRED QUIET) | ||
|
||
project(CPM LANGUAGES NONE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO: The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes should be safer indeed |
||
|
||
if(GIT_EXECUTABLE) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NOTE: this is not needed and will not reached with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. true 👍🏻 |
||
# Generate a git-describe version string from Git repository tags | ||
execute_process( | ||
COMMAND ${GIT_EXECUTABLE} tag --points-at HEAD | ||
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} | ||
OUTPUT_VARIABLE CURRENT_CPM_VERSION | ||
RESULT_VARIABLE GIT_DESCRIBE_ERROR_CODE | ||
OUTPUT_STRIP_TRAILING_WHITESPACE | ||
) | ||
if(NOT GIT_DESCRIBE_ERROR_CODE AND NOT CURRENT_CPM_VERSION STREQUAL "") | ||
string(SUBSTRING ${CURRENT_CPM_VERSION} 1 -1 CURRENT_CPM_VERSION) | ||
else() | ||
set(CURRENT_CPM_VERSION 1.0.0-development-version) | ||
endif() | ||
endif() | ||
|
||
include("${CMAKE_CURRENT_SOURCE_DIR}/cmake/CPM.cmake") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove REQUIRED please!
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain why ? I agree that FetchContent needs it so it should be safe but it should not hurt just for safety / be clear to the user that it is needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you can FetchContent from tarball release on systems without git (maybe a lightweight docker)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from CMake doc:
The
QUIET
option disables informational messages, including those indicating that the package cannot be found if it is notREQUIRED
.The
REQUIRED
option stops processing with an error message if the package cannot be found.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is the needed behaviour, Git is needed to extract the version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree but without Git the purpose of CPM is very limited. In this case people could still use get_cpm. My idea was to provide an alternative to get_cpm. If we want to get ou of Git this requires more changes (Maybe in next PR :) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is still useful as is because the user does not have to include an external file of
get_cpm
and instead use more native commands cmake commands.