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

Support CPM_<dependency name>_SOURCE local package override from ENV #406

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Support CPM_<dependency name>_SOURCE local package override from ENV #406

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
edca4b6
Allow CPM_DOWNLOAD_LOCATION from ENV or Command-line
CraigHutchinson Sep 15, 2022
1a6b27f
Allow Local package override from ENV variable
CraigHutchinson Sep 15, 2022
ec7c226
Removed nesting on `CPM_DOWNLOAD_LOCATION`
CraigHutchinson Sep 21, 2022
3ebaabc
Apply `fix-cmake-format` build target
CraigHutchinson Sep 28, 2022
d680cb7
Change to use `CPM_PATH` as user-defined CPM source location
CraigHutchinson Oct 4, 2022
f753ce8
Revert add of `style` to test build
CraigHutchinson Oct 10, 2022
8f58d23
Removed trailing space
CraigHutchinson Oct 10, 2022
68a0f74
Simplified local-package with TODO for allowing as extension-point
CraigHutchinson Oct 13, 2022
d1344f9
Merge branch 'master' into LocalPackageOverride_EnvSupport
CraigHutchinson Mar 8, 2023
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
13 changes: 10 additions & 3 deletions cmake/CPM.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,10 +611,17 @@ function(CPMAddPackage)
return()
endif()

# Check for ENV overide for local package-source when not already defined
# NOTE: Uses a recursive CPMAddPackage call where `CPM_${CPM_ARGS_NAME}_SOURCE` == "" which causes first if-case to stop recusrion
if( CPM_${CPM_ARGS_NAME}_SOURCE )
file(TO_CMAKE_PATH "${CPM_${CPM_ARGS_NAME}_SOURCE}" PACKAGE_SOURCE)
CraigHutchinson marked this conversation as resolved.
Show resolved Hide resolved
elseif( DEFINED ENV{CPM_${CPM_ARGS_NAME}_SOURCE})
file(TO_CMAKE_PATH "$ENV{CPM_${CPM_ARGS_NAME}_SOURCE}" PACKAGE_SOURCE)
endif()

# Check for manual overrides
if(NOT CPM_ARGS_FORCE AND NOT "${CPM_${CPM_ARGS_NAME}_SOURCE}" STREQUAL "")
set(PACKAGE_SOURCE ${CPM_${CPM_ARGS_NAME}_SOURCE})
set(CPM_${CPM_ARGS_NAME}_SOURCE "")
if(NOT CPM_ARGS_FORCE AND NOT "${PACKAGE_SOURCE}" STREQUAL "")
set(CPM_${CPM_ARGS_NAME}_SOURCE "") # Assign empty to prevent recursion
CPMAddPackage(
NAME "${CPM_ARGS_NAME}"
SOURCE_DIR "${PACKAGE_SOURCE}"
Expand Down
16 changes: 10 additions & 6 deletions cmake/get_cpm.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
set(CPM_DOWNLOAD_VERSION 1.0.0-development-version)

if(CPM_SOURCE_CACHE)
set(CPM_DOWNLOAD_LOCATION "${CPM_SOURCE_CACHE}/cpm/CPM_${CPM_DOWNLOAD_VERSION}.cmake")
elseif(DEFINED ENV{CPM_SOURCE_CACHE})
set(CPM_DOWNLOAD_LOCATION "$ENV{CPM_SOURCE_CACHE}/cpm/CPM_${CPM_DOWNLOAD_VERSION}.cmake")
else()
set(CPM_DOWNLOAD_LOCATION "${CMAKE_BINARY_DIR}/cmake/CPM_${CPM_DOWNLOAD_VERSION}.cmake")
if ( NOT CPM_DOWNLOAD_LOCATION)
Copy link
Member

Choose a reason for hiding this comment

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

An unwanted side effect of keeping the value of CPM_DOWNLOAD_LOCATION is that users would no longer receive a warning when a upstream package uses a newer CPM version than the outer project. This is currently handled inside the newer CPM.cmake script that would be included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I can just about see the issue you refer to here but maybe not, patient with me for a moment....

I may not be right, but for my current use-case I am using CPM_DOWNLOAD_LOCATION to override a projects CPM version at the top of the build. However until all the packages on which it depends are also updated to contain this check the CPM version will not be set across the entire build. However when/if this change is integrated into all packages then they would all end up using the same CPM script file.

In my view this being able to set the entire build to use a single CPM script feels preferential for developing than to supporting and testing having mixing of CPM versions across packages? It also allows testing if a dependency would have issues upgrading to a new CPM for instance with ease etc.

Copy link
Member

Choose a reason for hiding this comment

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

I may not be right, but for my current use-case I am using CPM_DOWNLOAD_LOCATION to override a projects CPM version at the top of the build. However until all the packages on which it depends are also updated to contain this check the CPM version will not be set across the entire build. However when/if this change is integrated into all packages then they would all end up using the same CPM script file.

That is true.

In my view this being able to set the entire build to use a single CPM script feels preferential for developing than to supporting and testing having mixing of CPM versions across packages? It also allows testing if a dependency would have issues upgrading to a new CPM for instance with ease etc.

In the current version, we also don't mix CPM functions. Currently, if a more recent CPM version than used downstream is included, a warning is emitted notifying the user that a newer version is available.

While not essential, we should be aware that this functionality would not persist with the suggested change.

if(DEFINED ENV{CPM_DOWNLOAD_LOCATION})
file(TO_CMAKE_PATH "$ENV{CPM_DOWNLOAD_LOCATION}" CPM_DOWNLOAD_LOCATION)
elseif(CPM_SOURCE_CACHE)
set(CPM_DOWNLOAD_LOCATION "${CPM_SOURCE_CACHE}/cpm/CPM_${CPM_DOWNLOAD_VERSION}.cmake")
elseif(DEFINED ENV{CPM_SOURCE_CACHE})
set(CPM_DOWNLOAD_LOCATION "$ENV{CPM_SOURCE_CACHE}/cpm/CPM_${CPM_DOWNLOAD_VERSION}.cmake")
else()
set(CPM_DOWNLOAD_LOCATION "${CMAKE_BINARY_DIR}/cmake/CPM_${CPM_DOWNLOAD_VERSION}.cmake")
endif()
endif()
TheLartians marked this conversation as resolved.
Show resolved Hide resolved

# Expand relative path. This is important if the provided path contains a tilde (~)
Expand Down