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.

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 additional arguments after shorthand syntax #546

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 15 additions & 20 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,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
Expand All @@ -111,6 +111,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 combined with the other options from above:
```cmake
CPMAddPackage("gh:nlohmann/[email protected]"
OPTIONS "JSON_BuildTests OFF"
)
```

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.
Expand Down Expand Up @@ -403,12 +410,8 @@ CPMAddPackage("gh:jbeder/yaml-cpp#[email protected]")
### [nlohmann/json](https://github.com/nlohmann/json)

```cmake
CPMAddPackage(
NAME nlohmann_json
VERSION 3.9.1
GITHUB_REPOSITORY nlohmann/json
OPTIONS
"JSON_BuildTests OFF"
CPMAddPackage("gh:nlohmann/[email protected]"
OPTIONS "JSON_BuildTests OFF"
)
```

Expand Down Expand Up @@ -437,20 +440,15 @@ 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
OPTIONS "CXXOPTS_BUILD_EXAMPLES NO" "CXXOPTS_BUILD_TESTS NO" "CXXOPTS_ENABLE_INSTALL YES"
CPMAddPackage("gh:jarro2783/[email protected]"
OPTIONS "CXXOPTS_BUILD_EXAMPLES NO" "CXXOPTS_BUILD_TESTS NO" "CXXOPTS_ENABLE_INSTALL YES"
)
```

### [google/benchmark](https://github.com/google/benchmark)

```cmake
CPMAddPackage(
NAME benchmark
GITHUB_REPOSITORY google/benchmark
VERSION 1.5.2
CPMAddPackage("gh:google/[email protected]"
OPTIONS "BENCHMARK_ENABLE_TESTING Off"
)

Expand All @@ -463,11 +461,8 @@ endif()
### [Lua](https://www.lua.org)

```cmake
CPMAddPackage(
NAME lua
GIT_REPOSITORY https://github.com/lua/lua.git
VERSION 5.3.5
DOWNLOAD_ONLY YES
CPMAddPackage("gh:lua/[email protected]"
DOWNLOAD_ONLY YES
)

if (lua_ADDED)
Expand Down
30 changes: 22 additions & 8 deletions cmake/CPM.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -575,14 +575,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
Expand All @@ -605,6 +597,28 @@ 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 shorthand argument as first argument but with following arguments
elseif(
argnLength GREATER 1
AND NOT "${ARGV0}" IN_LIST oneValueArgs
AND NOT "${ARGV0}" IN_LIST multiValueArgs
)
list(POP_FRONT ARGN)
cpm_parse_add_package_single_arg("${ARGV0}" ARGV0)

# The shorthand syntax implies EXCLUDE_FROM_ALL and SYSTEM
set(ARGN "${ARGV0};${ARGN};EXCLUDE_FROM_ALL;YES;SYSTEM;YES;")
endif()

cmake_parse_arguments(CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}")

# Set default values for arguments
Expand Down
29 changes: 29 additions & 0 deletions test/integration/test_simple.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,38 @@ 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_shorthand_syntax = -> {
prj.create_lists_from_default_template package: <<~PACK
CPMAddPackage(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

# 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

}

create_with_commit_sha.()
update_to_version_1.()
update_with_option_off_and_build.()
update_with_option_off_and_build_with_shorthand_syntax.()
Copy link
Member

Choose a reason for hiding this comment

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

I feel like having this run as part of a larger test makes reasoning about the test itself difficult. Do you think you could move checking the shorthand extra arguments into its own isolated test case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure 👍

end
end
Loading