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

Merged
merged 6 commits into from
Jan 5, 2025
Merged

CMake AnyNewerVersion -> SameMajorVersion #1188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 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
9874367
Switch CMP0167 policy to NEW
aprokop Dec 11, 2024
f07a54e
First fix for the Windows build
aprokop Dec 11, 2024
702140c
Alternative approach
aprokop Jan 4, 2025
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
8 changes: 3 additions & 5 deletions .github/workflows/windows.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,8 @@ jobs:
run: |
mkdir build
cd build
cmake -D CMAKE_INSTALL_PREFIX=C:\kokkos-install \
-D Kokkos_ENABLE_THREADS=ON \
..
cmake --build . --target install -- -m
cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_MSVC_RUNTIME_LIBRARY="MultiThreadedDebugDLL" -DCMAKE_INSTALL_PREFIX=C:\kokkos-install -DKokkos_ENABLE_THREADS=ON ..
cmake --build . --config Release --target install -- -m
Comment on lines +35 to +36
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Release is the only option that worked for me here. Adding CMAKE_MSVC_RUNTIME_LIBRARY is necessary as otherwise the release and debug versions of the libraries will conflict when building ArborX in Debug.

- name: Configure ArborX
run: |
mkdir build
Expand All @@ -44,5 +42,5 @@ jobs:
- 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)
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.

endif()

# 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