-
Notifications
You must be signed in to change notification settings - Fork 190
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
feature: allow URI to use shorthand syntax with additional options #617
base: master
Are you sure you want to change the base?
Changes from all commits
5913685
03c08f3
0edaafc
72e98e7
1cb8a5a
32900e9
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 | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -65,6 +65,7 @@ Afterwards, any targets defined in the dependency can be used directly. | |||||||||||||||
|
||||||||||||||||
```cmake | ||||||||||||||||
CPMAddPackage( | ||||||||||||||||
URI # shorthand including repo, name, version and tag (see shorthand syntax) | ||||||||||||||||
NAME # The unique name of the dependency (should be the exported target's name) | ||||||||||||||||
VERSION # The minimum version of the dependency (optional, defaults to 0) | ||||||||||||||||
PATCHES # Patch files to be applied sequentially using patch and PATCH_OPTIONS (optional) | ||||||||||||||||
|
@@ -74,6 +75,8 @@ CPMAddPackage( | |||||||||||||||
) | ||||||||||||||||
``` | ||||||||||||||||
|
||||||||||||||||
`URI` automatically sets `EXCLUDE_FROM_ALL YES` and `SYSTEM YES`. If this is not desired, `EXCLUDE_FROM_ALL NO` and `SYSTEM NO` have to be set. | ||||||||||||||||
|
||||||||||||||||
The origin may be specified by a `GIT_REPOSITORY`, but other sources, such as direct URLs, are [also supported](https://cmake.org/cmake/help/v3.11/module/ExternalProject.html#external-project-definition). | ||||||||||||||||
If `GIT_TAG` hasn't been explicitly specified it defaults to `v(VERSION)`, a common convention for git projects. | ||||||||||||||||
On the other hand, if `VERSION` hasn't been explicitly specified, CPM can automatically identify the version from the git tag in some common cases. | ||||||||||||||||
|
@@ -88,7 +91,7 @@ If an additional optional parameter `SYSTEM` is set to a truthy value, the SYSTE | |||||||||||||||
See the [add_subdirectory ](https://cmake.org/cmake/help/latest/command/add_subdirectory.html?highlight=add_subdirectory) | ||||||||||||||||
and [SYSTEM](https://cmake.org/cmake/help/latest/prop_tgt/SYSTEM.html#prop_tgt:SYSTEM) target property for details. | ||||||||||||||||
|
||||||||||||||||
A single-argument compact syntax is also supported: | ||||||||||||||||
A shorthand syntax is also supported: | ||||||||||||||||
|
||||||||||||||||
```cmake | ||||||||||||||||
# A git package from a given uri with a version | ||||||||||||||||
|
@@ -112,6 +115,13 @@ CPMAddPackage("https://example.com/my-package-1.2.3.zip#MD5=68e20f674a48be38d60e | |||||||||||||||
CPMAddPackage("https://example.com/[email protected]") | ||||||||||||||||
``` | ||||||||||||||||
|
||||||||||||||||
Additionally, the shorthand syntax can be used with the long version, using the `URI` specifier. | ||||||||||||||||
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. I think we can make it more clear that the new syntax allows for specifying additional arguments with the shorthand syntax:
Suggested change
|
||||||||||||||||
```cmake | ||||||||||||||||
CPMAddPackage(URI "gh:nlohmann/[email protected]" | ||||||||||||||||
OPTIONS "JSON_BuildTests OFF" | ||||||||||||||||
) | ||||||||||||||||
Comment on lines
+120
to
+122
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. Let's keep the formatting consistent with the other examples.
Suggested change
|
||||||||||||||||
``` | ||||||||||||||||
|
||||||||||||||||
After calling `CPMAddPackage`, the following variables are defined in the local scope, where `<dependency>` is the name of the dependency. | ||||||||||||||||
|
||||||||||||||||
- `<dependency>_SOURCE_DIR` is the path to the source of the dependency. | ||||||||||||||||
|
@@ -412,11 +422,8 @@ CPMAddPackage("gh:jbeder/yaml-cpp#[email protected]") | |||||||||||||||
|
||||||||||||||||
```cmake | ||||||||||||||||
CPMAddPackage( | ||||||||||||||||
NAME nlohmann_json | ||||||||||||||||
VERSION 3.9.1 | ||||||||||||||||
GITHUB_REPOSITORY nlohmann/json | ||||||||||||||||
OPTIONS | ||||||||||||||||
"JSON_BuildTests OFF" | ||||||||||||||||
URI "gh:nlohmann/[email protected]" | ||||||||||||||||
OPTIONS "JSON_BuildTests OFF" | ||||||||||||||||
) | ||||||||||||||||
``` | ||||||||||||||||
|
||||||||||||||||
|
@@ -446,8 +453,7 @@ For a working example of using CPM to download and configure the Boost C++ Libra | |||||||||||||||
```cmake | ||||||||||||||||
# the install option has to be explicitly set to allow installation | ||||||||||||||||
CPMAddPackage( | ||||||||||||||||
GITHUB_REPOSITORY jarro2783/cxxopts | ||||||||||||||||
VERSION 2.2.1 | ||||||||||||||||
URI "gh:jarro2783/[email protected]" | ||||||||||||||||
OPTIONS "CXXOPTS_BUILD_EXAMPLES NO" "CXXOPTS_BUILD_TESTS NO" "CXXOPTS_ENABLE_INSTALL YES" | ||||||||||||||||
) | ||||||||||||||||
``` | ||||||||||||||||
|
@@ -456,9 +462,7 @@ CPMAddPackage( | |||||||||||||||
|
||||||||||||||||
```cmake | ||||||||||||||||
CPMAddPackage( | ||||||||||||||||
NAME benchmark | ||||||||||||||||
GITHUB_REPOSITORY google/benchmark | ||||||||||||||||
VERSION 1.5.2 | ||||||||||||||||
URI "gh:google/[email protected]" | ||||||||||||||||
OPTIONS "BENCHMARK_ENABLE_TESTING Off" | ||||||||||||||||
) | ||||||||||||||||
|
||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -590,14 +590,6 @@ endfunction() | |
function(CPMAddPackage) | ||
cpm_set_policies() | ||
|
||
list(LENGTH ARGN argnLength) | ||
if(argnLength EQUAL 1) | ||
cpm_parse_add_package_single_arg("${ARGN}" ARGN) | ||
|
||
# The shorthand syntax implies EXCLUDE_FROM_ALL and SYSTEM | ||
set(ARGN "${ARGN};EXCLUDE_FROM_ALL;YES;SYSTEM;YES;") | ||
endif() | ||
|
||
set(oneValueArgs | ||
NAME | ||
FORCE | ||
|
@@ -620,10 +612,26 @@ function(CPMAddPackage) | |
|
||
set(multiValueArgs URL OPTIONS DOWNLOAD_COMMAND PATCHES) | ||
|
||
list(LENGTH ARGN argnLength) | ||
|
||
# Parse single shorthand argument | ||
if(argnLength EQUAL 1) | ||
cpm_parse_add_package_single_arg("${ARGN}" ARGN) | ||
|
||
# The shorthand syntax implies EXCLUDE_FROM_ALL and SYSTEM | ||
set(ARGN "${ARGN};EXCLUDE_FROM_ALL;YES;SYSTEM;YES;") | ||
|
||
# Parse URI shorthand argument | ||
elseif(argnLength GREATER 1 AND "${ARGV0}" STREQUAL "URI") | ||
list(REMOVE_AT ARGN 0 1) # remove "URI gh:<...>@version#tag" | ||
cpm_parse_add_package_single_arg("${ARGV1}" ARGV0) | ||
|
||
set(ARGN "${ARGV0};EXCLUDE_FROM_ALL;YES;SYSTEM;YES;${ARGN}") | ||
endif() | ||
Comment on lines
+625
to
+630
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. We should either make it clear in the docs that |
||
|
||
cmake_parse_arguments(CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}") | ||
|
||
# Set default values for arguments | ||
|
||
if(NOT DEFINED CPM_ARGS_VERSION) | ||
if(DEFINED CPM_ARGS_GIT_TAG) | ||
cpm_get_version_from_git_tag("${CPM_ARGS_GIT_TAG}" CPM_ARGS_VERSION) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,9 +83,69 @@ def test_update_single_package | |
# ...and notably no test for adder, which must be disabled from the option override from above | ||
assert_equal ['simple', 'using-adder'], exes | ||
} | ||
update_with_option_off_and_build_with_uri_shorthand_syntax = -> { | ||
prj.create_lists_from_default_template package: <<~PACK | ||
CPMAddPackage( | ||
URI gh:cpm-cmake/[email protected] | ||
OPTIONS "ADDER_BUILD_TESTS OFF" | ||
) | ||
PACK | ||
assert_success prj.configure | ||
assert_success prj.build | ||
|
||
exe_dir = File.join(prj.bin_dir, 'bin') | ||
assert File.directory? exe_dir | ||
|
||
exes = Dir[exe_dir + '/**/*'].filter { | ||
# on multi-configuration generators (like Visual Studio) the executables will be in bin/<Config> | ||
# also filter-out other artifacts like .pdb or .dsym | ||
!File.directory?(_1) && File.stat(_1).executable? | ||
}.map { | ||
# remove .exe extension if any (there will be one on Windows) | ||
File.basename(_1, '.exe') | ||
}.sort | ||
Comment on lines
+99
to
+106
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. Not very important, but this code is now duplicated three times in the test case. Could it be more readable if we turn this it into a method and call it three times? |
||
|
||
# we should end up with two executables | ||
# * simple - the simple example from adder | ||
# * using-adder - for this project | ||
# ...and notably no test for adder, which must be disabled from the option override from above | ||
assert_equal ['simple', 'using-adder'], exes | ||
} | ||
update_with_option_on_and_build_with_uri_shorthand_syntax_and_exclude_from_override = -> { | ||
prj.create_lists_from_default_template package: <<~PACK | ||
CPMAddPackage( | ||
URI gh:cpm-cmake/[email protected] | ||
OPTIONS "ADDER_BUILD_TESTS ON" | ||
EXCLUDE_FROM_ALL NO | ||
) | ||
PACK | ||
assert_success prj.configure | ||
assert_success prj.build | ||
|
||
exe_dir = File.join(prj.bin_dir, 'bin') | ||
assert File.directory? exe_dir | ||
|
||
exes = Dir[exe_dir + '/**/*'].filter { | ||
# on multi-configuration generators (like Visual Studio) the executables will be in bin/<Config> | ||
# also filter-out other artifacts like .pdb or .dsym | ||
!File.directory?(_1) && File.stat(_1).executable? | ||
}.map { | ||
# remove .exe extension if any (there will be one on Windows) | ||
File.basename(_1, '.exe') | ||
}.sort | ||
|
||
# we should end up with two executables | ||
# * simple - the simple example from adder | ||
# * using-adder - for this project | ||
# ...and notably no test for adder, which must be disabled from the option override from above | ||
assert_equal ['simple', 'test-adding', 'using-adder'], exes | ||
Comment on lines
+137
to
+141
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. The comment needs to be changed, as we are now expecting the tests to be built. |
||
} | ||
|
||
|
||
create_with_commit_sha.() | ||
update_to_version_1.() | ||
update_with_option_off_and_build.() | ||
update_with_option_off_and_build_with_uri_shorthand_syntax.() | ||
update_with_option_on_and_build_with_uri_shorthand_syntax_and_exclude_from_override.() | ||
end | ||
end |
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.
Thanks for adding the extra docs! Tbh I'm not sure if it won't be confusing that we show
URI
here, but alsoNAME
andVERSION
as only one or the other should be specified. Maybe we can leave it out here and keep this section as before to make it more obvious that the shorthand syntax is a way to replace the name and source info?