Trilinos: Best Practice for Deprecating Cmake options

[csiefer2]

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 with Tpetra_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.

[bartlettroscoe]

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:

• https://docs.trilinos.org/files/TrilinosBuildReference.html#adjusting-cmake-deprecation-warnings

I actually spent a bit of time discussing this with Kyle Edwards from Kitware in issue:

• Design and implement strategy for deprecating TriBITS code TriBITSPub/TriBITS#429

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)?

[csiefer2]

I'm OK with a Trilinos-wide cmake option deprecated macro, like proposed above. @ccober6 Opinion?

[sebrowne]

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?

[bartlettroscoe]

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?

[github-actions]

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.