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

Don't use cmake_path before CMake 3.20 #266

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

cwpearson
Copy link
Contributor

cmake_path in cmake/utils.cmake was added in CMake 3.20

@cwpearson cwpearson requested a review from dalg24 July 27, 2024 11:34
@dalg24
Copy link
Member

dalg24 commented Jul 29, 2024

I would prefer if we did

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4529918..198080c 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -77,7 +77,11 @@ endif()
 # Libraries
 if(KokkosTools_ENABLE_PAPI)
   find_package(PAPI REQUIRED) # TODO: papi-connector requires v6.0 or newer
-  cmake_path(GET PAPI_INCLUDE_DIR PARENT_PATH PAPI_ROOT)
+  if (CMAKE_VERSION VERSION_GREATER_EQUAL "3.20")
+    cmake_path(GET PAPI_INCLUDE_DIR PARENT_PATH PAPI_ROOT)
+  else()
+    get_filename_component(PARENT_DIR(PARENT_DIR ${PAPI_INCLUDE_DIR} PAPI_ROOT)
+  endif()
   message(STATUS "Found PAPI ${PAPI_VERSION_STRING} at ${PAPI_ROOT}")
   set(KokkosTools_HAS_PAPI ON)
 else()

(not tested)
rather than forcing our hand to bump the minimum. Printing information on the screen for an optional package should not be the thing dictating when we do so and what the minimum should be.

@cwpearson
Copy link
Contributor Author

I will test this with cmake 3.19 and 3.20 and see what happens

@cwpearson
Copy link
Contributor Author

Tested with PAPI 6 and cmake 3.19 / 3.20

@cwpearson cwpearson changed the title CMake minimum version 3.16 -> 3.20 Don't use cmake_path before CMake 3.20 Jul 31, 2024
@dalg24 dalg24 merged commit 7604355 into kokkos:develop Jul 31, 2024
7 checks passed
@dalg24 dalg24 added bug Build Building Kokkos tools labels Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Build Building Kokkos tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants