Open csiefer2 opened 10 months ago
CC: @csiefer2, @ccober6, @sebrowne, @trilinos/framework
I am not sure how this should be done for Trilinos but I call tell you how we handle deprecated cache variables in TriBITS. We created the tribits_deprecated() function that used like this:
set (<cacheVarName> <defaultValue> CACHE <cacheType> “<doc>”)
if (<cacheVarName>) # Or whatever the condition should be
tribits_derpedated(<cacheVarName>
“The variable <cacheVarName> is deprecated blah blah blah …”)
endif()
Then, the user (or developer) can control how to handle the usage of a deprecated variables by setting the cache var:
-D TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE=<deprecationLevel>
For the values of <deprecationLevel>
, see:
I actually spent a bit of time discussing this with Kyle Edwards from Kitware in issue:
which we implemented over a year ago.
But that is for deprecating TriBITS code, not necessarily for project-specific Trilinos CMake code.
Likely the best thing is likely to copy and paste the the file TribitsDeprecatedHelpers.cmake into Trilinos and rename it to TribitsDeprecatedCMakeHelpers.cmake
with the renamed function trilinos_deprecated()
and the renamed cache variable Trilinos_HANDLE_DEPRECATED_CMAKE_OPTIONS
and use it like:
tribits_add_option_and_define(<cacheVarName> …)
if (<cacheVarName>) # Whatever the condition should be
trilinos_derpedated(<cacheVarName>
“The variable <cacheVarName> is deprecated blah blah blah …”)
endif()
Or each package could have its own deprecation function that keys off of a global value (but that seems like overkill)?
I'm OK with a Trilinos-wide cmake option deprecated macro, like proposed above. @ccober6 Opinion?
I'm in favor of this approach.
Maybe add a trilinos_deprecated_if_enabled()
that can be used in most cases, since most things we'd want to catch will be when somebody turns on an option that we want to remove?
I'm in favor of this approach.
👍
Maybe add a
trilinos_deprecated_if_enabled()
that can be used in most cases, since most things we'd want to catch will be when somebody turns on an option that we want to remove?
That is reasonable. You would just have trilinos_deprecated_if_enabled()
call trilinos_deprecated()
like:
function(trilinos_deprecated_if_enabled cachVarName msg)
if (${cacheVarName})
tribits_deprecated("${msg}")
endif()
endfunction()
So you would have two new CMake functions: trilinos_deprecated()
and trilinos_deprecated_if_enabled()
.
Sound good to everyone?
While Trilinos packages have a standard way of deprecating code, we don't have a standard way of deprecating cmake configuration options. The Tpetra driver for this is
Tpetra_ASSUME_CUDA_AWARE_MPI
, which we replaced withTpetra_ASSUME_GPU_AWARE_MPI
a while ago, but left the old option in so it wouldn't break the applications. We'd like to remove the old guy entirely, but need a way to warn apps to change how they configure Trilinos before we break their scripts.@bartlettroscoe has suggested a potential way to do this, which he'll share below.