From c57865abafd264d876ab4647d0d5ead33767eb4c Mon Sep 17 00:00:00 2001
From: "Roscoe A. Bartlett" <rabartl@sandia.gov>
Date: Thu, 23 Feb 2023 07:42:19 -0700
Subject: [PATCH] Ifpack2: Remove tribtits_add_option_and_define() calls for
 vars TriBITS automatically sets (#10355, #11583)

The updated funtion tribits_add_option_and_define(<optionName> <defineName>
...) will error out if a local non-cache var of the same name <defineName>
already exists.  As explained in the TriBITS Users Guide, for all dependencies
listed in the package's Dependencies.cmake file, TriBITS automatically defines
and sets the vars:

* <Package>_ENABLE_<UpstreamDepPkg>: Cache var and/or non-cache local project-level var

* HAVE_<PACKAGE>_<UPSTREAMDEPPKG>: Non-cache local project-level var

The function tribits_add_option_and_define(<optionName> <defineName> ...)
creates an INTERNAL cache var with the name <defineName>.  (This was done
because some Trilinos packages communicated info through these <defineName>
vars for some reason.)

With new behavior in CMake 3.21+ with policy CMP0126, setting the INTERNAL
cache var <defineName> no longer resets the local var of the same name.
Therefore, the function tribits_add_option_and_define(<optionName>
<defineName> ...) has no impact and does not change anything that impacts
behavior.  Also, since TriBITS automatically defines documented cache vars
with the name <Package>_ENABLE_<UpstreamDepPkg>, the statement
set(<optionName> <value> CACHE BOOL ...) does not have any effect because the
cache var is already set.  Therefore, these calls to:

tribits_add_option_and_define(<Package>_ENABLE_<UpstreamDepPkg>
  HAVE_<PACKAGE>_<UPSTREAMDEPPKG> ...)

have no impact and just confused behavior.  (But were actually masking a nasty
defect described in #11583 after updating the CMake minimum version to 3.23.)

This commit also adds a formal optional dependency on MPI to replace the
option Ifpack2_ENABLE_MPI and the macro define var HAVE_IFPACK2_MPI that are
defined automatically by TriBITS.
---
 packages/ifpack2/CMakeLists.txt           | 19 -------------------
 packages/ifpack2/cmake/Dependencies.cmake |  2 +-
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/packages/ifpack2/CMakeLists.txt b/packages/ifpack2/CMakeLists.txt
index a540f8206586..b445107eea01 100644
--- a/packages/ifpack2/CMakeLists.txt
+++ b/packages/ifpack2/CMakeLists.txt
@@ -70,25 +70,6 @@ TRIBITS_ADD_ETI_SUPPORT()
 #This replaces an option that was reusing an existing name.
 SET(HAVE_${PACKAGE_NAME_UC}_QD "${PACKAGE_NAME}_ENABLE_QD")
 
-#Adding a define for TPL METIS
-IF (TPL_ENABLE_METIS)
-  SET (IFPACK2_ENABLE_METIS_DEFAULT ON)
-ELSE ()
-  SET (IFPACK2_ENABLE_METIS_DEFAULT OFF)
-ENDIF ()
-TRIBITS_ADD_OPTION_AND_DEFINE(
-  ${PACKAGE_NAME}_ENABLE_METIS
-  HAVE_IFPACK2_METIS
-  "Enable METIS for Ifpack2."
-  ${IFPACK2_ENABLE_METIS_DEFAULT} )
-
-#Adding an option and define when MPI is enabled.
-TRIBITS_ADD_OPTION_AND_DEFINE(
-  ${PACKAGE_NAME}_ENABLE_MPI
-  HAVE_IFPACK2_MPI
-  "Enable MPI build of Ifpack2."
-  ${TPL_ENABLE_MPI} )
-
 ASSERT_DEFINED(${PACKAGE_NAME}_ENABLE_Lemon)
 ASSERT_DEFINED(${PACKAGE_NAME}_ENABLE_Amesos2)
 ASSERT_DEFINED(${PACKAGE_NAME}_ENABLE_Cholmod)
diff --git a/packages/ifpack2/cmake/Dependencies.cmake b/packages/ifpack2/cmake/Dependencies.cmake
index fc4a5e176e40..dc5b40abf678 100644
--- a/packages/ifpack2/cmake/Dependencies.cmake
+++ b/packages/ifpack2/cmake/Dependencies.cmake
@@ -3,6 +3,6 @@ SET(LIB_OPTIONAL_DEP_PACKAGES Xpetra Zoltan2Core ThyraTpetraAdapters Amesos2 Shy
 SET(TEST_REQUIRED_DEP_PACKAGES Belos Galeri)
 SET(TEST_OPTIONAL_DEP_PACKAGES Amesos2 ShyLU_NodeHTS ML AztecOO Epetra)
 SET(LIB_REQUIRED_DEP_TPLS)
-SET(LIB_OPTIONAL_DEP_TPLS HYPRE Cholmod Lemon METIS)
+SET(LIB_OPTIONAL_DEP_TPLS HYPRE Cholmod Lemon METIS MPI)
 SET(TEST_REQUIRED_DEP_TPLS)
 SET(TEST_OPTIONAL_DEP_TPLS QD HYPRE METIS)