-
Notifications
You must be signed in to change notification settings - Fork 201
Adding PATCHES keyword. #558
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
Changes from all commits
4395363
7793284
311cd3f
ceef053
5ed5dfe
523c14a
9347302
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 |
---|---|---|
|
@@ -31,6 +31,7 @@ parse: | |
EXCLUDE_FROM_ALL: 1 | ||
SYSTEM: 1 | ||
SOURCE_SUBDIR: 1 | ||
PATCHES: + | ||
OPTIONS: + | ||
cpmfindpackage: | ||
pargs: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -464,6 +464,69 @@ function(cpm_check_git_working_dir_is_clean repoPath gitTag isClean) | |
|
||
endfunction() | ||
|
||
# Add PATCH_COMMAND to CPM_ARGS_UNPARSED_ARGUMENTS. This method consumes a list of files in ARGN | ||
# then generates a `PATCH_COMMAND` appropriate for `ExternalProject_Add()`. This command is appended | ||
# to the parent scope's `CPM_ARGS_UNPARSED_ARGUMENTS`. | ||
function(cpm_add_patches) | ||
# Return if no patch files are supplied. | ||
if(NOT ARGN) | ||
return() | ||
endif() | ||
|
||
# Find the patch program. | ||
find_program(PATCH_EXECUTABLE patch) | ||
if(WIN32 AND NOT PATCH_EXECUTABLE) | ||
# The Windows git executable is distributed with patch.exe. Find the path to the executable, if | ||
# it exists, then search `../../usr/bin` for patch.exe. | ||
find_package(Git QUIET) | ||
if(GIT_EXECUTABLE) | ||
get_filename_component(extra_search_path ${GIT_EXECUTABLE} DIRECTORY) | ||
get_filename_component(extra_search_path ${extra_search_path} DIRECTORY) | ||
get_filename_component(extra_search_path ${extra_search_path} DIRECTORY) | ||
find_program(PATCH_EXECUTABLE patch HINTS "${extra_search_path}/usr/bin") | ||
endif() | ||
endif() | ||
if(NOT PATCH_EXECUTABLE) | ||
message(FATAL_ERROR "Couldn't find `patch` executable to use with PATCHES keyword.") | ||
endif() | ||
|
||
# Create a temporary | ||
set(temp_list ${CPM_ARGS_UNPARSED_ARGUMENTS}) | ||
|
||
# Ensure each file exists (or error out) and add it to the list. | ||
set(first_item True) | ||
foreach(PATCH_FILE ${ARGN}) | ||
# Make sure the patch file exists, if we can't find it, try again in the current directory. | ||
if(NOT EXISTS "${PATCH_FILE}") | ||
if(NOT EXISTS "${CMAKE_CURRENT_LIST_DIR}/${PATCH_FILE}") | ||
message(FATAL_ERROR "Couldn't find patch file: '${PATCH_FILE}'") | ||
endif() | ||
set(PATCH_FILE "${CMAKE_CURRENT_LIST_DIR}/${PATCH_FILE}") | ||
endif() | ||
|
||
# Convert to absolute path for use with patch file command. | ||
get_filename_component(PATCH_FILE "${PATCH_FILE}" ABSOLUTE) | ||
|
||
# The first patch entry must be preceded by "PATCH_COMMAND" while the following items are | ||
# preceded by "&&". | ||
if(first_item) | ||
set(first_item False) | ||
list(APPEND temp_list "PATCH_COMMAND") | ||
else() | ||
list(APPEND temp_list "&&") | ||
endif() | ||
# Add the patch command to the list | ||
list(APPEND temp_list "${PATCH_EXECUTABLE}" "-p1" "<" "${PATCH_FILE}") | ||
endforeach() | ||
|
||
# Move temp out into parent scope. | ||
set(CPM_ARGS_UNPARSED_ARGUMENTS | ||
${temp_list} | ||
PARENT_SCOPE | ||
) | ||
Comment on lines
+523
to
+526
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. Generally, if modifying the parent scope, I feel it would be a good idea to provide the variable name in the parent as an argument to the function to make it easier for us to read the called function. Do you think it would make sense here as well? 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'm not sure I understand the suggestion? Can you point me to an example? In this case, we put 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. @TheLartians 523c14a addresses the other comments, but I'm not sure what to do here... Suggestions? Thanks! 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 see your point that the function is basically just an outsourced section of a larger function, but in general I like to have clearly defined input and outputs in functions - in CMake's case using arguments so the caller can decide what goes in and comes out. In my experience ist helps a lot with readability. In any way it's just a style preference and not a blocker for this PR. |
||
|
||
endfunction() | ||
|
||
# method to overwrite internal FetchContent properties, to allow using CPM.cmake to overload | ||
# FetchContent calls. As these are internal cmake properties, this method should be used carefully | ||
# and may need modification in future CMake versions. Source: | ||
|
@@ -537,7 +600,7 @@ function(CPMAddPackage) | |
CUSTOM_CACHE_KEY | ||
) | ||
|
||
set(multiValueArgs URL OPTIONS DOWNLOAD_COMMAND) | ||
set(multiValueArgs URL OPTIONS DOWNLOAD_COMMAND PATCHES) | ||
|
||
cmake_parse_arguments(CPM_ARGS "" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}") | ||
|
||
|
@@ -628,6 +691,7 @@ function(CPMAddPackage) | |
SOURCE_DIR "${PACKAGE_SOURCE}" | ||
EXCLUDE_FROM_ALL "${CPM_ARGS_EXCLUDE_FROM_ALL}" | ||
SYSTEM "${CPM_ARGS_SYSTEM}" | ||
PATCHES "${CPM_ARGS_PATCHES}" | ||
OPTIONS "${CPM_ARGS_OPTIONS}" | ||
SOURCE_SUBDIR "${CPM_ARGS_SOURCE_SUBDIR}" | ||
DOWNLOAD_ONLY "${DOWNLOAD_ONLY}" | ||
|
@@ -683,6 +747,8 @@ function(CPMAddPackage) | |
set(CPM_FETCHCONTENT_BASE_DIR ${CMAKE_BINARY_DIR}/_deps) | ||
endif() | ||
|
||
cpm_add_patches(${CPM_ARGS_PATCHES}) | ||
|
||
if(DEFINED CPM_ARGS_DOWNLOAD_COMMAND) | ||
list(APPEND CPM_ARGS_UNPARSED_ARGUMENTS DOWNLOAD_COMMAND ${CPM_ARGS_DOWNLOAD_COMMAND}) | ||
elseif(DEFINED CPM_ARGS_SOURCE_DIR) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
cmake_minimum_required(VERSION 3.14 FATAL_ERROR) | ||
|
||
project(CPMExamplePatchHighway) | ||
|
||
# ---- Dependencies ---- | ||
|
||
include(../../cmake/CPM.cmake) | ||
|
||
# Google's highway Includes a SIMD sorting function that is faster than x86-simd-sort for larger | ||
# arrays. See: https://github.com/google/highway/blob/master/g3doc/quick_reference.md | ||
CPMAddPackage( | ||
NAME highway | ||
URL https://github.com/google/highway/archive/refs/tags/1.1.0.tar.gz | ||
URL_HASH SHA256=354a8b4539b588e70b98ec70844273e3f2741302c4c377bcc4e81b3d1866f7c9 | ||
PATCHES "highway.patch" # This adds SYSTEM to the includes. | ||
OPTIONS "HWY_ENABLE_EXAMPLES OFF" "HWY_ENABLE_INSTALL OFF" "HWY_ENABLE_TESTS OFF" | ||
) | ||
|
||
# ---- Executable ---- | ||
|
||
if(LINUX) | ||
# This would cause a float compare error inside the highway header code if the patch is NOT | ||
# applied. | ||
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wfloat-equal") | ||
endif() | ||
|
||
add_executable(CPMExamplePatchHighway "main.cpp") | ||
target_link_libraries(CPMExamplePatchHighway hwy hwy_contrib) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
Common subdirectories: a/.bcr and b/.bcr | ||
Common subdirectories: a/.github and b/.github | ||
diff -u a/CMakeLists.txt b/CMakeLists.txt | ||
--- a/CMakeLists.txt 2024-05-21 12:50:37.738318520 -0500 | ||
+++ b/CMakeLists.txt 2024-05-21 12:49:59.914226871 -0500 | ||
@@ -350,7 +350,7 @@ | ||
target_compile_options(hwy PRIVATE ${HWY_FLAGS}) | ||
set_property(TARGET hwy PROPERTY POSITION_INDEPENDENT_CODE ON) | ||
set_target_properties(hwy PROPERTIES VERSION ${LIBRARY_VERSION} SOVERSION ${LIBRARY_SOVERSION}) | ||
-target_include_directories(hwy PUBLIC | ||
+target_include_directories(hwy SYSTEM PUBLIC | ||
$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}> | ||
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>) | ||
target_compile_features(hwy PUBLIC cxx_std_11) | ||
@@ -370,7 +370,7 @@ | ||
target_compile_options(hwy_contrib PRIVATE ${HWY_FLAGS}) | ||
set_property(TARGET hwy_contrib PROPERTY POSITION_INDEPENDENT_CODE ON) | ||
set_target_properties(hwy_contrib PROPERTIES VERSION ${LIBRARY_VERSION} SOVERSION ${LIBRARY_SOVERSION}) | ||
-target_include_directories(hwy_contrib PUBLIC | ||
+target_include_directories(hwy_contrib SYSTEM PUBLIC | ||
$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}> | ||
$<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>) | ||
target_compile_features(hwy_contrib PUBLIC cxx_std_11) | ||
Common subdirectories: a/cmake and b/cmake | ||
Common subdirectories: a/debian and b/debian | ||
Common subdirectories: a/docs and b/docs | ||
Common subdirectories: a/g3doc and b/g3doc | ||
Common subdirectories: a/hwy and b/hwy |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
#include <hwy/contrib/sort/vqsort.h> // hwy::VQSort() for large data sets | ||
|
||
#include <cstdint> | ||
#include <random> | ||
#include <vector> | ||
|
||
// Use hwy::VQSort to sort larger vectors | ||
inline void sort_large(std::vector<double>& v) { | ||
hwy::VQSort(v.data(), v.size(), hwy::SortAscending{}); | ||
} | ||
|
||
int main(int, char**) { | ||
std::random_device random_device; | ||
std::default_random_engine random_engine(random_device()); | ||
std::uniform_real_distribution<double> uniform_dist(0.0, 100.0); | ||
|
||
const std::size_t sz = 100000; | ||
std::vector<double> v; | ||
v.reserve(sz); | ||
for (std::size_t i = 0; i < sz; ++i) { | ||
v.push_back(uniform_dist(random_engine)); | ||
} | ||
|
||
sort_large(v); | ||
|
||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
cmake_minimum_required(VERSION 3.14 FATAL_ERROR) | ||
|
||
project(using-patch-adder) | ||
|
||
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) | ||
|
||
include("%{cpm_path}") | ||
|
||
%{packages} | ||
|
||
add_executable(using-patch-adder using-patch-adder.cpp) | ||
|
||
target_link_libraries(using-patch-adder adder) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
diff --git a/code/adder/adder.hpp b/code/adder/adder.hpp | ||
index fdb9324..7c2fa00 100644 | ||
--- a/code/adder/adder.hpp | ||
+++ b/code/adder/adder.hpp | ||
@@ -1,6 +1,6 @@ | ||
#pragma once | ||
|
||
-namespace adder | ||
+namespace patched | ||
{ | ||
int add(int a, int b); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
diff --git a/code/adder/adder.cpp b/code/adder/adder.cpp | ||
index fc6e767..44b1197 100644 | ||
--- a/code/adder/adder.cpp | ||
+++ b/code/adder/adder.cpp | ||
@@ -1,6 +1,6 @@ | ||
#include "adder.hpp" | ||
|
||
-namespace adder | ||
+namespace patched | ||
{ | ||
int add(int a, int b) | ||
{ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
#include <adder/adder.hpp> | ||
#include <cstdio> | ||
|
||
int main() { | ||
int sum = patched::add(5, 3); | ||
std::printf("%d\n", sum); | ||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
require_relative './lib' | ||
|
||
# Tests using a multi-argumenet PATCHES command to fetch and modify a dependency | ||
|
||
class DownloadCommand < IntegrationTest | ||
|
||
def test_fetch_dependency_using_download_command | ||
prj = make_project from_template: 'using-patch-adder' | ||
|
||
prj.create_lists_from_default_template package: <<~PACK | ||
set(DOWNLOAD_DIR ${CMAKE_BINARY_DIR}/_deps/testpack-adder-src) | ||
CPMAddPackage( | ||
NAME testpack-adder | ||
DOWNLOAD_COMMAND git clone --depth 1 --branch v1.0.0 https://github.com/cpm-cmake/testpack-adder.git ${DOWNLOAD_DIR} | ||
OPTIONS "ADDER_BUILD_TESTS OFF" "ADDER_BUILD_EXAMPLES OFF" | ||
PATCHES | ||
patches/001-test_patches_command.patch | ||
patches/002-test_patches_command.patch | ||
) | ||
PACK | ||
|
||
# configure with unpopulated cache | ||
assert_success prj.configure | ||
assert_success prj.build | ||
end | ||
|
||
end |
Uh oh!
There was an error while loading. Please reload this page.