Fetch content #446
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,60 @@ | ||||||
| cmake_minimum_required(VERSION 3.14 FATAL_ERROR) | ||||||
|
|
||||||
| project( | ||||||
| CPM | ||||||
| VERSION 0.37.0 | ||||||
| LANGUAGES NONE | ||||||
| ) | ||||||
|
|
||||||
| option(CPM_INSTALL "Install CPM" OFF) | ||||||
|
There was a problem hiding this comment. Is the option necessary? Maybe it is to avoid install on ExternalProject, but if that's the case better use the There was a problem hiding this comment. I was thinking about people using CPM just to compile their project, forcing install would be unnecessary. But for a framework it could be interesting to allow the install of CPM There was a problem hiding this comment. That is not necessary. Cmake only installs if you pass There was a problem hiding this comment. I think it is the case, CMake will install all even upstreams There was a problem hiding this comment. If you really want to supress this option, maybe we could create a custom target only if we detect CPM is not the main repo and consumer could ask explicitly to install it or make it a dependency to one of the consumer's targets There was a problem hiding this comment. Indeed I confirm this is the case (need to fix my packages). Check the review later on about what is a sufficient way to circumvent this option. |
||||||
|
|
||||||
| include(CMakePackageConfigHelpers) | ||||||
| configure_package_config_file( | ||||||
| "${CMAKE_CURRENT_SOURCE_DIR}/cmake/CPMConfig.cmake.in" | ||||||
| "${CMAKE_CURRENT_BINARY_DIR}/CPMConfig.cmake" | ||||||
| INSTALL_DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/CPM" | ||||||
|
There was a problem hiding this comment. What if we would want to install multiple CPM versions side-by-side, would the previous version be overwritten? There was a problem hiding this comment. That is common practice. The installation via Including the verison in the |
||||||
| ) | ||||||
| write_basic_package_version_file( | ||||||
| "${CMAKE_CURRENT_BINARY_DIR}/CPMConfigVersion.cmake" COMPATIBILITY SameMajorVersion | ||||||
| ARCH_INDEPENDENT | ||||||
| ) | ||||||
|
|
||||||
| # For consumer using FetchContent | ||||||
| if(NOT ${CMAKE_PROJECT_NAME} STREQUAL ${PROJECT_NAME}) | ||||||
| # Trick to use the find_package | ||||||
| configure_file( | ||||||
| "${CMAKE_CURRENT_SOURCE_DIR}/cmake/CPM.cmake" "${CMAKE_CURRENT_BINARY_DIR}/CPM.cmake" COPYONLY | ||||||
| ) | ||||||
| set(CPM_DIR "${CMAKE_CURRENT_BINARY_DIR}") | ||||||
| # Check if the user download the repo with git and not in an official release | ||||||
| if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/.git" AND IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/.git") | ||||||
|
There was a problem hiding this comment. I've recently found out about this approach: Basically whenever Github creates the tar ball, it will change the content of
Let's go with this approach. We don't need to worry about the versions before this, because you can't use There was a problem hiding this comment. To test locally, use: $ git archive --format=tar HEAD --output=test.tar.gzThere was a problem hiding this comment. @TheLartians As an alternative to hard-coded version numbering, would you agree to such approach, i.e.:
I have never tried this in a project, but could be very elegant if it can be implemented. Things to confirm:
There was a problem hiding this comment. Relevant PR here. For now this approach works because we don't actually build anything. Basically all of this is now compressed to # Check if there is anything after the allowed format of `vX.Y.Z`, e.g. `vX.Y.Z-rc1`, `vX.Y.Z-9-gc449f6`, etc.
if (GIT_DESCRIBE MATCH "[v]?[0-9\\.]*.+")
set(CPM_RELEASE FALSE)
else ()
set(CPM_RELEASE TRUE)
endif ()There was a problem hiding this comment. This may, or may not be useful, i use this for versioning all my builds. Interested to add support for archival as this is new to knowledge to me but I parse the git-describe here. Note the use of There was a problem hiding this comment. Oh yeah, the regex is wrong because it is not maximally greedy. That link is overkill though because the |
||||||
| find_package(Git REQUIRED QUIET) | ||||||
| # 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 CPM_TAG | ||||||
| RESULT_VARIABLE GIT_DESCRIBE_ERROR_CODE | ||||||
| OUTPUT_STRIP_TRAILING_WHITESPACE | ||||||
| ) | ||||||
| if(NOT GIT_DESCRIBE_ERROR_CODE AND NOT CPM_TAG STREQUAL "") | ||||||
| set(CPM_RELEASE TRUE) | ||||||
| else() | ||||||
| set(CPM_RELEASE FALSE) | ||||||
| endif() | ||||||
| endif() | ||||||
flagarde marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| find_package(CPM CONFIG REQUIRED NO_POLICY_SCOPE) | ||||||
|
There was a problem hiding this comment. How does this work? I've never thought you could There was a problem hiding this comment. The CPMConfig is generated but not installed automatically. I think generally maybe it could not work but in our case we just need to include CPM.cmake so it works. It is working because I have provided CPM_DIR to say where to find the generated Config file I did some test on my computer. It seems to work There was a problem hiding this comment. It seems you can use an Configfile generated inside the same project and CMake propose : There was a problem hiding this comment. Ok, I see how that works, but this will mark release tarballs as A more manageable approach is to set
There was a problem hiding this comment. To my understanding release tarballs will not have a .git folder inside so the CPM_RELEASE will be unset in this case. I see you point. What you propose would work but this requires CPM maintainers to agree on this behaviour. I tried to stay conservative on the workflows and CI I think for now this would work : if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/.git" AND IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/.git")
find_package(Git REQUIRED QUIET)
# 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 CPM_TAG
RESULT_VARIABLE GIT_DESCRIBE_ERROR_CODE
OUTPUT_STRIP_TRAILING_WHITESPACE
)
if(NOT GIT_DESCRIBE_ERROR_CODE AND NOT CPM_TAG STREQUAL "")
set(CPM_RELEASE TRUE)
else()
set(CPM_RELEASE FALSE)
endif()
else()
set(CPM_RELEASE TRUE)
endif()There was a problem hiding this comment. Yes, there is also a possibility of the opposite (downloading a tarball for a specific commit), but let's ignore this for now. I think we should ping the maintainers about the long-term approach, either here or a separate issue. There was a problem hiding this comment. I didn't know you could do this :) but it seems very a niche scenario, that it is not possible for now with get_cpm anyway. There was a problem hiding this comment. We are over-complicating it here. This works just fine
Suggested change
There was a problem hiding this comment. Ok, I found a small bug with how we are looking for the .git I can change this and your changes. I tested using :
cmake_minimum_required(VERSION 3.24)
project(TestCPMFetchContent)
include(FetchContent)
FetchContent_Declare(CPM
GIT_REPOSITORY https://github.com/flagarde/CPM.cmake
GIT_TAG FetchContent
OVERRIDE_FIND_PACKAGE)
FetchContent_MakeAvailable(CPM)
find_package(CPM)
# the install option has to be explicitly set to allow installation
CPMAddPackage(
GITHUB_REPOSITORY jarro2783/cxxopts
VERSION 2.2.1
OPTIONS "CXXOPTS_BUILD_EXAMPLES NO" "CXXOPTS_BUILD_TESTS NO" "CXXOPTS_ENABLE_INSTALL YES"
)In this case not loop but a crash : CMake Error at CMakeLists.txt:15 (CPMAddPackage):
Unknown CMake command "CPMAddPackage".
-- Configuring incomplete, errors occurred!without OVERRIDE_FIND_PACKAGE : -- The C compiler identification is GNU 12.2.1
-- The CXX compiler identification is GNU 12.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
fatal : ni ceci ni aucun de ses répertoires parents (jusqu'au point de montage /) n'est un dépôt git
Arrêt à la limite du système de fichiers (GIT_DISCOVERY_ACROSS_FILESYSTEM n'est pas défini).
CMake Warning at CMakeLists.txt:12 (find_package):
By not providing "FindCPM.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "CPM", but
CMake did not find one.
Could not find a package configuration file provided by "CPM" with any of
the following names:
CPMConfig.cmake
cpm-config.cmake
Add the installation prefix of "CPM" to CMAKE_PREFIX_PATH or set "CPM_DIR"
to a directory containing one of the above files. If "CPM" provides a
separate development package or SDK, be sure it has been installed.
-- CPM: Adding package [email protected] (v2.2.1)
-- cxxopts version 2.2.0
-- Configuring done
-- Generating done
There was a problem hiding this comment. Now it seems to work :) Please have a try too.. just in case There was a problem hiding this comment. All good now. For an initial integration I'm 👍 on this. |
||||||
| endif() | ||||||
|
|
||||||
| if(CPM_INSTALL) | ||||||
flagarde marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| # Without it : Unable to determine default CMAKE_INSTALL_LIBDIR directory because no target | ||||||
| # architecture is known. Please enable at least one language before including GNUInstallDirs. | ||||||
| enable_language(C) | ||||||
| include(GNUInstallDirs) | ||||||
| install( | ||||||
| FILES "${CMAKE_CURRENT_BINARY_DIR}/CPMConfig.cmake" | ||||||
| "${CMAKE_CURRENT_BINARY_DIR}/CPMConfigVersion.cmake" | ||||||
| "${CMAKE_CURRENT_SOURCE_DIR}/cmake/CPM.cmake" | ||||||
| DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/CPM" | ||||||
| ) | ||||||
| endif() | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| @PACKAGE_INIT@ | ||
|
|
||
| include("${CMAKE_CURRENT_LIST_DIR}/CPM.cmake" NO_POLICY_SCOPE) | ||
|
|
||
| check_required_components(CPM) |
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'm not a fan of hardcoding the version in Git as this will require maintainers to be careful when creating new releases that the version and according tag are identical. Additionally the check below will force consumers to use make proper git clones including tags to ensure they are not in a development version.
How about instead using a mechanism like in the publish workflow to publish a downloadable zip archive of the relevant files with the current version on new tags?
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 would disagree on this. It makes the intent of the versioning more clear. For example, when navigating the git commits, it is easier to know which version the current commit is intended for from the
CMakelists.txtfile. The usual workflow for a release is:Number 2. can be automated in
publishaction to always occur after a release is published. For the case of bug fix increments, those should be branched from the relevant version and merged back into the development branch. For the major version it should be done manually to indicate intent of api change.