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

CMake AnyNewerVersion -> SameMajorVersion #1188

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
+8 −8
Open

CMake AnyNewerVersion -> SameMajorVersion #1188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d790cf0
CMake AnyNewerVersion -> SameMajorVersion
aprokop Nov 12, 2024
e103ab0
Bump minimum required CMake version to 3.22
aprokop Dec 11, 2024
52956f6
Update CMake in docker images
aprokop Dec 11, 2024
316fff8
Specify Kokkos cmake build type for Windows workflow
aprokop Dec 11, 2024
ba2667d
Switch CMP0167 policy to NEW
aprokop Dec 11, 2024
ba11f00
Try explicitly building Kokkos with Debug
aprokop Dec 11, 2024
0dbd723
Try building ArborX explicityl in Debug mode in Windows CI
masterleinad Dec 11, 2024
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
6 changes: 3 additions & 3 deletions .github/workflows/windows.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ jobs:
cmake -D CMAKE_INSTALL_PREFIX=C:\kokkos-install \
-D Kokkos_ENABLE_THREADS=ON \
..
cmake --build . --target install -- -m
cmake --build . --config Debug --target install -- -m
- name: Configure ArborX
run: |
mkdir build
cd build
cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS="/EHsc /bigobj" -DKokkos_ROOT="C:\kokkos-install" ${{ steps.vcpkg.outputs.vcpkg-cmake-config }} -DARBORX_ENABLE_MPI=OFF -DARBORX_ENABLE_TESTS=ON -DARBORX_ENABLE_EXAMPLES=ON -DARBORX_ENABLE_BENCHMARKS=ON -DARBORX_ENABLE_HEADER_SELF_CONTAINMENT_TESTS=OFF ..
cmake -DCMAKE_CXX_FLAGS="/EHsc /bigobj" -DKokkos_ROOT="C:\kokkos-install" ${{ steps.vcpkg.outputs.vcpkg-cmake-config }} -DARBORX_ENABLE_MPI=OFF -DARBORX_ENABLE_TESTS=ON -DARBORX_ENABLE_EXAMPLES=ON -DARBORX_ENABLE_BENCHMARKS=ON -DARBORX_ENABLE_HEADER_SELF_CONTAINMENT_TESTS=OFF ..
- name: Build ArborX
shell: bash
run: |
cmake --build build --target install -- -m
cmake --build build --config Debug --target install -- -m
cd build
6 changes: 3 additions & 3 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.22)
project(ArborX CXX)

if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.30.0)
message(STATUS "Setting policy CMP0167 to use FindBoost module")
cmake_policy(SET CMP0167 OLD)
cmake_policy(SET CMP0167 NEW)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite get that change. Please explain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Admittedly, drive-by change. Instead of retaining old behavior, switch to the new one for finding boost modules. This allows to test both ways.

Copy link
Contributor

@dalg24 dalg24 Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think the right way to do this is unconditionally setting the policy to new.
That is

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,10 +1,8 @@
 cmake_minimum_required(VERSION 3.16)
 project(ArborX CXX)

-if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.30.0)
-  message(STATUS "Setting policy CMP0167 to use FindBoost module")
-  cmake_policy(SET CMP0167 OLD)
-endif()
+# FindBoost module removed in version 3.30
+cmake_policy(SET CMP0167 NEW)

 # use gnu standard install directories
 include(GNUInstallDirs)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you do that, you will get warnings for older versions of CMake

Policy "CMP0167" is not known to this version of CMake.


# use gnu standard install directories
Expand Down Expand Up @@ -137,7 +137,7 @@ configure_package_config_file(cmake/ArborXConfig.cmake.in
)
write_basic_package_version_file(${CMAKE_CURRENT_BINARY_DIR}/ArborXConfigVersion.cmake
VERSION ${ARBORX_VERSION_STRING}
COMPATIBILITY AnyNewerVersion
COMPATIBILITY SameMajorVersion
)
dalg24 marked this conversation as resolved.
Show resolved Hide resolved
install(FILES
${CMAKE_CURRENT_BINARY_DIR}/ArborXConfig.cmake
Expand Down
2 changes: 1 addition & 1 deletion docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ RUN KEYDUMP_URL=https://cloud1.cees.ornl.gov/download && \

# Install CMake
ENV CMAKE_DIR=/opt/cmake
RUN CMAKE_VERSION=3.16.9 && \
RUN CMAKE_VERSION=3.22.6 && \
CMAKE_KEY=2D2CEF1034921684 && \
CMAKE_URL=https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION} && \
CMAKE_SCRIPT=cmake-${CMAKE_VERSION}-Linux-x86_64.sh && \
Expand Down
2 changes: 1 addition & 1 deletion examples/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.22)
project(ArborXExamples CXX)
find_package(ArborX 2.0 REQUIRED)
enable_testing()
Expand Down
Loading