arborx / ArborX

Performance-portable geometric search library
BSD 3-Clause "New" or "Revised" License
184 stars 39 forks source link

CMake `AnyNewerVersion` -> `SameMajorVersion` #1188

Open aprokop opened 6 days ago

aprokop commented 6 days ago

We are not backwards compatible for version 2.0. Allowing users' 1.x code to build against 2.0 is almost 100% would lead to build failures. Would rather prevent that during configuration.

Switching to SameMajorVersion would likely be temporary, for a few 2.x releases, after which we would switch back.

An alternative to switching to SameMajorVersion would be to update CMake requirements to at least 3.19 (release 11/18/2020, so about a year older than CMake we are using right now) and do

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -136,7 +136,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 AnyNewerVersion <2.0
 )
 install(FILES
   ${CMAKE_CURRENT_BINARY_DIR}/ArborXConfig.cmake

This has a benefit of not having to revisit this again later.

Note: AnyNewerVersion was introduced in #984.

Rombur commented 6 days ago

This means that users cannot support both 1.x and 2.x versions in their own code. That's going to be a pain.

aprokop commented 6 days ago

How about we conditionally us a version range when cmake is recent enough and we defer the actual update of our cmake minimum requirement?

For the current CMake, we switch to SameMajorVersion?

This means that users cannot support both 1.x and 2.x versions in their own code. That's going to be a pain.

~Fair point. Do you suggest sticking with AnyNewerVersion? Do you envision users wanting to do that? In relation to that, we should probably also provide ARBORX_VERSION macro to ease code writing depending on ArborX version.~ Actually, I'm not sure exactly what you mean by this. They could do find_package(ArborX REQUIRED) without specifying version. Depending on what version they find, they could enable/disable their code.

Rombur commented 6 days ago

They could do find_package(ArborX REQUIRED) without specifying version

I want to be able to require a minimum version because older versions did not have ray tracing.

aprokop commented 6 days ago

I want to be able to require a minimum version because older versions did not have ray tracing.

Why not require ArborX 2.0? Though, that would likely only work for something small like Adamantine. Not sure about deal.II.

Rombur commented 6 days ago

Though, that would likely only work for something small like Adamantine. Not sure about deal.II.

Yes, I could live with SameMajorVersion in adamantine but it's a lot harder in deal.ii with all the dependencies. Someone is going to be stuck using an older version of Trilinos which means they cannot update Arborx, and so they cannot update deal.ii