Skip to content

Commit

Permalink
version.json patch command supports required field (#700)
Browse files Browse the repository at this point in the history
When a patch fails, rapids-cmake will log the failure under $CMAKE_BUILD_DIR/rapids-cmake/<the-dep>/log and continue. 
Now with this change rapids-cmake will collect all patches with the `required` field set to `true` that fail and throw an error.

Fixes [#699](#699)

Authors:
  - Robert Maynard (https://github.com/robertmaynard)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #700
  • Loading branch information
robertmaynard authored Oct 10, 2024
1 parent fe3362f commit 27b7b66
Show file tree
Hide file tree
Showing 13 changed files with 514 additions and 9 deletions.
6 changes: 6 additions & 0 deletions docs/cpm.rst
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ as needed.
is no longer needed in. If this patch is required for all
versions an empty string should be supplied.

``required``
An optional boolean value that represents if it is required that the patch
apply correctly.

The default value for this field is ``false``.

``proprietary_binary``

An optional dictionary of cpu architecture and operating system keys to url values that represents a download for a pre-built proprietary version of the library. This creates a new entry in the search
Expand Down
9 changes: 8 additions & 1 deletion rapids-cmake/cpm/detail/display_patch_status.cmake
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#=============================================================================
# Copyright (c) 2022, NVIDIA CORPORATION.
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -39,5 +39,12 @@ function(rapids_cpm_display_patch_status package_name)
message(STATUS "${line}")
endforeach()
endif()

set(err_file "${CMAKE_BINARY_DIR}/rapids-cmake/patches/${package_name}/err")
if(EXISTS "${err_file}")
file(READ "${err_file}" contents)
message(FATAL_ERROR "${contents}")
endif()

endif()
endfunction()
10 changes: 9 additions & 1 deletion rapids-cmake/cpm/detail/generate_patch_command.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ function(rapids_cpm_generate_patch_command package_name version patch_command)
# For each project cache the subset of the json
set(patch_files_to_run)
set(patch_issues_to_ref)
set(patch_required_to_apply)

# Gather number of patches
string(JSON patch_count LENGTH "${json_data}")
Expand All @@ -94,13 +95,20 @@ function(rapids_cpm_generate_patch_command package_name version patch_command)
endif()
list(APPEND patch_files_to_run "${file}")
list(APPEND patch_issues_to_ref "${issue}")

set(required FALSE)
rapids_cpm_json_get_value(${patch_data} required)
list(APPEND patch_required_to_apply "${required}")
endif()
unset(file)
unset(issue)
endif()
endforeach()
endif()

set(patch_script "${CMAKE_BINARY_DIR}/rapids-cmake/patches/${package_name}/patch.cmake")
set(log_file "${CMAKE_BINARY_DIR}/rapids-cmake/patches/${package_name}/log")
set(err_file "${CMAKE_BINARY_DIR}/rapids-cmake/patches/${package_name}/err")
if(patch_files_to_run)
string(TIMESTAMP current_year "%Y" UTC)
configure_file(${rapids-cmake-dir}/cpm/patches/command_template.cmake.in "${patch_script}"
Expand All @@ -109,6 +117,6 @@ function(rapids_cpm_generate_patch_command package_name version patch_command)
else()
# remove any old patch / log files that exist and are no longer needed due to a change in the
# package version / version.json
file(REMOVE "${patch_script}" "${log_file}")
file(REMOVE "${patch_script}" "${log_file}" "${err_file}")
endif()
endfunction()
26 changes: 19 additions & 7 deletions rapids-cmake/cpm/patches/command_template.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
#=============================================================================

set(msg_state)
function(rapids_cpm_run_git_patch file issue)
set(error_state)
function(rapids_cpm_run_git_patch file issue require)
set(git_command @GIT_EXECUTABLE@)
cmake_path(GET file FILENAME file_name)
cmake_path(GET file_name EXTENSION LAST_ONLY ext)
Expand Down Expand Up @@ -61,14 +62,20 @@ function(rapids_cpm_run_git_patch file issue)
)
endif()

# Setup where we log error message too
set(error_msg_var msg_state)
if(require)
set(error_msg_var error_state)
endif()

if(result EQUAL 0)
list(APPEND msg_state "rapids-cmake [@package_name@]: applied ${ext} ${file_name} to fix issue: '${issue}'")
list(APPEND msg_state "rapids-cmake [@package_name@]: applied ${ext} ${file_name} to fix issue: '${issue}'\n")
else()
list(APPEND msg_state "rapids-cmake [@package_name@]: failed to apply ${ext} ${file_name}")
list(APPEND msg_state "rapids-cmake [@package_name@]: git ${ext} output: ${repo_error_info}")
list(APPEND ${error_msg_var} "rapids-cmake [@package_name@]: failed to apply ${ext} ${file_name}\n")
list(APPEND ${error_msg_var} "rapids-cmake [@package_name@]: git ${ext} output: ${repo_error_info}\n")
endif()
list(APPEND msg_state "\n")
set(msg_state ${msg_state} PARENT_SCOPE)
set(error_state ${error_state} PARENT_SCOPE)
endfunction()

# We want to ensure that any patched files have a timestamp
Expand All @@ -83,10 +90,15 @@ execute_process(COMMAND ${CMAKE_COMMAND} -E sleep 1)

set(files "@patch_files_to_run@")
set(issues "@patch_issues_to_ref@")
set(required "@patch_required_to_apply@")
set(output_file "@log_file@")
foreach(file issue IN ZIP_LISTS files issues)
rapids_cpm_run_git_patch(${file} ${issue})
set(error_file "@err_file@")
foreach(file issue require IN ZIP_LISTS files issues required)
rapids_cpm_run_git_patch(${file} ${issue} ${require})
endforeach()
if(msg_state)
file(WRITE "${output_file}" ${msg_state})
endif()
if(error_state)
file(WRITE "${error_file}" ${error_state})
endif()
2 changes: 2 additions & 0 deletions testing/cpm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ add_cmake_config_test( cpm_find-existing-target-to-export-sets )
add_cmake_config_test( cpm_find-gtest-no-gmock )
add_cmake_config_test( cpm_find-options-escaped )
add_cmake_config_test( cpm_find-patch-command NO_CPM_CACHE)
add_cmake_config_test( cpm_find-patch-command-required NO_CPM_CACHE )
add_cmake_config_test( cpm_find-patch-command-required-fails NO_CPM_CACHE SHOULD_FAIL "rapids-cmake [fmt]: failed to apply patch")
add_cmake_config_test( cpm_find-restore-cpm-vars )
add_cmake_config_test( cpm_find-version-explicit-install.cmake )

Expand Down
45 changes: 45 additions & 0 deletions testing/cpm/cpm_find-patch-command-required-fails/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#=============================================================================
# Copyright (c) 2024, NVIDIA CORPORATION.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#=============================================================================
cmake_minimum_required(VERSION 3.26.4)
project(rapids-cpm_find-patch-command-project LANGUAGES CXX)

include("${rapids-cmake-dir}/cpm/detail/package_details.cmake")
rapids_cpm_package_details(fmt version repository tag shallow exclude)

set(deps_dir "${CMAKE_CURRENT_BINARY_DIR}/_fmt_dep")
if(NOT EXISTS "${deps_dir}")
file(MAKE_DIRECTORY "${deps_dir}")
find_package(Git)
execute_process(
COMMAND ${GIT_EXECUTABLE} clone --depth 1 --branch "${tag}" "${repository}"
WORKING_DIRECTORY "${deps_dir}")
endif()

set(fmt_dir "${deps_dir}/fmt")
list(APPEND CMAKE_PREFIX_PATH "${fmt_dir}")

include(${rapids-cmake-dir}/cpm/init.cmake)
rapids_cpm_init()

include(${rapids-cmake-dir}/cpm/package_override.cmake)
rapids_cpm_package_override(${CMAKE_CURRENT_SOURCE_DIR}/override.json)

include(${rapids-cmake-dir}/cpm/fmt.cmake)
rapids_cpm_fmt()

if(NOT "${fmt_ADDED}")
message(FATAL_ERROR "The found repo was used rather than downloading and patching a new version")
endif()
20 changes: 20 additions & 0 deletions testing/cpm/cpm_find-patch-command-required-fails/override.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"packages": {
"fmt": {
"patches": [
{
"file": "${current_json_dir}/patches/0001-move-git-sha1.patch",
"issue": "Move git sha1",
"fixed_in": "",
"required": false
},
{
"file": "${current_json_dir}/patches/0002-bad-git-change.patch",
"issue": "Fail to apply git patch",
"fixed_in": "",
"required": true
}
]
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
From deacd3fafd7fcfee954ae3044ae3ab60d36a9f3a Mon Sep 17 00:00:00 2001
From: Robert Maynard <[email protected]>
Date: Wed, 31 Jan 2024 15:00:47 -0500
Subject: [PATCH 1/2] Move GIT SHA1

---
git_file_1.txt | 1 +
1 file changed, 1 insertion(+)
create mode 100644 git_file_1.txt

diff --git a/git_file_1.txt b/git_file_1.txt
new file mode 100644
index 00000000..b242c360
--- /dev/null
+++ b/git_file_1.txt
@@ -0,0 +1 @@
+added file
--
2.43.0
Loading

0 comments on commit 27b7b66

Please sign in to comment.