Allow to use FetchContent to get CPM #437
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. IMHO: The There was a problem hiding this comment. Yes should be safer indeed |
||
|
|
||
| if(GIT_EXECUTABLE) | ||
|
There was a problem hiding this comment. NOTE: this is not needed and will not reached with |
||
| # 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() | ||
|
There was a problem hiding this comment. Why use May you show us a usage example? There was a problem hiding this comment. If I understand this correctly, it is to get the CPM version from the git tags currently used. I would rather prefer we make the version on the cmake be the main version source. This would require the maintainers to agree to such change of workflow. It might be possible to link/create a github action to read the cmake version and push tags automatically 🤔 There was a problem hiding this comment. Yes it is to extract the version from the tag we are asking. I agree it would be better to include the right version to the project but the drawback is that the user could ask to use the main branch without any tags and would receive the version that is refering to the latest tags/version but still be upstream if the have pulled changes before releasing a new version... Using git to acces the tag, if the consumer ask to point to a commit or a branch (main for example), than he will receive the warning that he is using a unversionned cpm (because HEAD is not tagged so CURRENT_CPM_VERSION will be set to '1.0.0-development-version' which will trigger a warning on CPM.cmake). It's the simplest way I have found to do this, and as we are using Fetchcontent we are sure Git is present. There was a problem hiding this comment. I can't quite follow this. The user can get a specific version from the tags, and for debugging/testing they'll get the There was a problem hiding this comment. Imagine your master branch have some commit not part of a tag, and not yet ready for an updated version you will be between for example the latest version let's say 0.39.0 and the next wonderful version 0.40.0. Which version to put ? 0.39.0 is impossible because you are not anymore in 0.39.0 but not yet on 0.40.0 There was a problem hiding this comment. it could be possible to have an empty version for not released commit I agree but this required more change on the workflow There was a problem hiding this comment. The general approach is to append the short hash or branch to the last known version (as But, since this would not be intended for production I think it is ok to have some ambiguity in the version. It would be possible to get the latest version everytime as well by setting it to github latest release tar ball. If we still want to get the version being requested without git, I would investigate if FetchContent exposes any variable related to how it is being called and take the value from there. |
||
|
|
||
| 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
QUIEToption disables informational messages, including those indicating that the package cannot be found if it is notREQUIRED.The
REQUIREDoption 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_cpmand instead use more native commands cmake commands.