Make TPL-to-TPL dependencies optional (#557)

Disabling the upstream TPL HDF5 was disabling the TPL Netcdf and that broke several customer's Trilinos configure scripts. The issue is, that we can't treat TPL-to-TPL dependencies as required for cases like this. I also updated some documentation to make this more clear. In the future, we may need to expand TPL intra-dependencies to support optional and required dependencies because there are cases where you would like the behavior of required dependencies for some TPL relationships.
TriBITSPub · Jan 6, 2023 · c4f7fc3 · c4f7fc3
1 parent 3879de0
commit c4f7fc3
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 30 deletions.
diff --git a/test/core/DependencyUnitTests/CMakeLists.txt b/test/core/DependencyUnitTests/CMakeLists.txt
@@ -568,8 +568,8 @@ create_reduced_dependency_handling_test_case(
     -DTrilinos_DUMP_PACKAGE_DEPENDENCIES:BOOL=ON
     -DTrilinos_DUMP_FORWARD_PACKAGE_DEPENDENCIES:BOOL=ON
   PASS_REGULAR_EXPRESSION_ALL
-    "-- BLAS_FORWARD_LIB_DEFINED_DEPENDENCIES: LAPACK.R."
-    "-- LAPACK_LIB_DEFINED_DEPENDENCIES: BLAS.R."
+    "-- BLAS_FORWARD_LIB_DEFINED_DEPENDENCIES: LAPACK.O."
+    "-- LAPACK_LIB_DEFINED_DEPENDENCIES: BLAS.O."
     "Explicitly enabled external packages/TPLs on input [(]by user[)]:  0"
     "Explicitly disabled external packages/TPLs on input [(]by user or by default[)]:  0"
     "-- Setting TPL_ENABLE_LAPACK=ON because DependsOnLAPACK has a required dependence on LAPACK"
@@ -599,11 +599,38 @@ create_reduced_dependency_handling_test_case(
     "Final set of enabled external packages/TPLs:  BLAS LAPACK 2"
     "Processing enabled external package/TPL: BLAS [(]enabled explicitly, disable with -DTPL_ENABLE_BLAS=OFF[)]"
     "Processing enabled external package/TPL: LAPACK [(]enabled by DependsOnLAPACK, disable with -DTPL_ENABLE_LAPACK=OFF[)]"
-    "-- LAPACK_LIB_ENABLED_DEPENDENCIES: BLAS.R."
+    "-- LAPACK_LIB_ENABLED_DEPENDENCIES: BLAS.O."
     "-- DependsOnLAPACK_LIB_ENABLED_DEPENDENCIES: LAPACK.R."
   )
 
 
+create_reduced_dependency_handling_test_case(
+  IndirectTplDependency_ExplicitDisableBLAS
+  ARGS
+    -DTrilinos_EXTRA_REPOSITORIES=extraRepos/DependsOnLAPACK
+    -DTPL_ENABLE_BLAS=OFF
+    -DTrilinos_ENABLE_DependsOnLAPACK=ON
+    -DTrilinos_DUMP_PACKAGE_DEPENDENCIES:BOOL=ON
+    -DTrilinos_DUMP_FORWARD_PACKAGE_DEPENDENCIES:BOOL=ON
+  PASS_REGULAR_EXPRESSION_ALL
+    "-- BLAS_FORWARD_LIB_DEFINED_DEPENDENCIES: LAPACK.O."
+    "-- LAPACK_LIB_DEFINED_DEPENDENCIES: BLAS.O."
+    "Explicitly enabled external packages/TPLs on input [(]by user[)]:  0"
+    "Explicitly disabled external packages/TPLs on input [(]by user or by default[)]:  BLAS 1"
+    "-- Setting TPL_ENABLE_LAPACK=ON because DependsOnLAPACK has a required dependence on LAPACK"
+    "-- Setting DependsOnLAPACK_ENABLE_LAPACK=ON since Trilinos_ENABLE_DependsOnLAPACK=ON AND TPL_ENABLE_LAPACK=ON"
+    "Final set of enabled packages:  DependsOnLAPACK 1"
+    "Final set of enabled external packages/TPLs:  LAPACK 1"
+    "Final set of non-enabled external packages/TPLs:  .* BLAS .*"
+    "-- LAPACK: No enabled dependencies"
+    "-- DependsOnLAPACK_LIB_ENABLED_DEPENDENCIES: LAPACK.R."
+  )
+# NOTE: The above test ensures that the explicit disable of an upstream TPL
+# does not implicitly disable a downstream TPL (see TriBITSPub/TriBITS#557).
+# This was breaking use cases where people were enabling Netcdf but disabling
+# HDF5 (and that is a valid configuration when Netcdf is built without HDF5).
+
+
 #####################################################################
 #
 # Unit tests for dependency handling for full set of packages
@@ -827,16 +854,14 @@ create_dependency_handling_test_case(
     "Explicitly enabled top-level packages on input .by user.:  Zoltan 1"
     "Explicitly disabled top-level packages on input .by user or by default.:  Stokhos 1"
     "Explicitly disabled external packages/TPLs on input .by user or by default.:  MPI BLAS 2"
-    "-- Setting TPL_ENABLE_LAPACK=OFF because LAPACK has a required library dependence on disabled package BLAS"
-    "-- Setting TPL_ENABLE_SuperLUDist=OFF because SuperLUDist has a required library dependence on disabled package BLAS"
-    "-- Setting TPL_ENABLE_SuperLU=OFF because SuperLU has a required library dependence on disabled package BLAS"
     "-- Setting Trilinos_ENABLE_Teuchos=OFF because Teuchos has a required library dependence on disabled package BLAS"
     "-- Setting Trilinos_ENABLE_Epetra=OFF because Epetra has a required library dependence on disabled package BLAS"
     "-- Setting Trilinos_ENABLE_TrilinosFramework=ON"
     "Final set of enabled top-level packages:  TrilinosFramework Zoltan Shards 3"
     "Final set of enabled packages:  TrilinosFramework Zoltan Shards 3"
     "Final set of non-enabled top-level packages:  Teuchos RTOp Epetra Triutils Tpetra EpetraExt Stokhos Sacado Thyra Isorropia AztecOO Galeri Amesos Intrepid Ifpack ML Belos Stratimikos RBGen Phalanx Panzer 21"
     "Final set of non-enabled packages:  Teuchos RTOp Epetra Triutils Tpetra EpetraExt Stokhos Sacado ThyraCoreLibs ThyraGoodStuff ThyraCrazyStuff ThyraEpetra ThyraEpetraExt ThyraTpetra Thyra Isorropia AztecOO Galeri Amesos Intrepid Ifpack ML Belos Stratimikos RBGen Phalanx Panzer 27"
+    "Final set of enabled external packages/TPLs:  0"
   )
 # Above test makes sure that TRUE/FALSE also processed correctly for enable
 # vars.
@@ -1127,8 +1152,6 @@ create_dependency_handling_test_case(
   EnableZoltan_ZoltanEnableParMETIS_TplDisableParMETIS
   ARGS -DTrilinos_ENABLE_Zoltan=ON -DZoltan_ENABLE_ParMETIS=ON -DTPL_ENABLE_ParMETIS=OFF
   PASS_REGULAR_EXPRESSION_ALL
-    "-- Setting TPL_ENABLE_SuperLUDist=OFF because SuperLUDist has a required library dependence on disabled package ParMETIS"
-    "-- Setting TPL_ENABLE_SuperLU=OFF because SuperLU has a required library dependence on disabled package ParMETIS"
     "-- NOTE: Setting Zoltan_ENABLE_ParMETIS=OFF which was ON because Zoltan has an optional library dependence on disabled package ParMETIS"
     "Final set of enabled external packages/TPLs:  0"
   )

diff --git a/test/core/DependencyUnitTests/DepTests_EnableAllPackages_DumpDependencies.txt b/test/core/DependencyUnitTests/DepTests_EnableAllPackages_DumpDependencies.txt
@@ -18,20 +18,20 @@ Package dependencies information:
 -- MPI_FORWARD_LIB_DEFINED_DEPENDENCIES: Teuchos[O] Epetra[O] Zoltan[O] ML[O] Panzer[R]
 -- MPI_FORWARD_TEST_DEFINED_DEPENDENCIES: TrilinosFramework[O]
 
--- BLAS_FORWARD_LIB_DEFINED_DEPENDENCIES: LAPACK[R] SuperLUDist[R] SuperLU[R] Teuchos[R] Epetra[R] ML[R]
+-- BLAS_FORWARD_LIB_DEFINED_DEPENDENCIES: LAPACK[O] SuperLUDist[O] SuperLU[O] Teuchos[R] Epetra[R] ML[R]
 
--- LAPACK_LIB_DEFINED_DEPENDENCIES: BLAS[R]
+-- LAPACK_LIB_DEFINED_DEPENDENCIES: BLAS[O]
 -- LAPACK_FORWARD_LIB_DEFINED_DEPENDENCIES: Teuchos[R] Epetra[R] ML[R]
 
 -- Boost_FORWARD_LIB_DEFINED_DEPENDENCIES: Teuchos[O] Phalanx[R] Panzer[R]
 
 -- Scotch_FORWARD_LIB_DEFINED_DEPENDENCIES: Zoltan[O]
 
--- METIS_FORWARD_LIB_DEFINED_DEPENDENCIES: ParMETIS[R] ML[O]
+-- METIS_FORWARD_LIB_DEFINED_DEPENDENCIES: ParMETIS[O] ML[O]
 -- METIS_FORWARD_TEST_DEFINED_DEPENDENCIES: ML[O]
 
--- ParMETIS_LIB_DEFINED_DEPENDENCIES: METIS[R]
--- ParMETIS_FORWARD_LIB_DEFINED_DEPENDENCIES: SuperLUDist[R] SuperLU[R] Zoltan[O] Amesos[O] ML[O]
+-- ParMETIS_LIB_DEFINED_DEPENDENCIES: METIS[O]
+-- ParMETIS_FORWARD_LIB_DEFINED_DEPENDENCIES: SuperLUDist[O] SuperLU[O] Zoltan[O] Amesos[O] ML[O]
 -- ParMETIS_FORWARD_TEST_DEFINED_DEPENDENCIES: ML[O]
 
 -- CppUnit_FORWARD_TEST_DEFINED_DEPENDENCIES: Sacado[O]
@@ -44,10 +44,10 @@ Package dependencies information:
 
 -- y12m_FORWARD_LIB_DEFINED_DEPENDENCIES: AztecOO[O]
 
--- SuperLUDist_LIB_DEFINED_DEPENDENCIES: ParMETIS[R] BLAS[R]
+-- SuperLUDist_LIB_DEFINED_DEPENDENCIES: ParMETIS[O] BLAS[O]
 -- SuperLUDist_FORWARD_LIB_DEFINED_DEPENDENCIES: Amesos[O]
 
--- SuperLU_LIB_DEFINED_DEPENDENCIES: ParMETIS[R] BLAS[R]
+-- SuperLU_LIB_DEFINED_DEPENDENCIES: ParMETIS[O] BLAS[O]
 -- SuperLU_FORWARD_LIB_DEFINED_DEPENDENCIES: Amesos[O]
 
 -- UMFPACK_FORWARD_LIB_DEFINED_DEPENDENCIES: EpetraExt[O] Amesos[O]
@@ -173,7 +173,7 @@ Dumping direct enabled dependencies for each package ...
 
 -- BLAS: No enabled dependencies!
 
--- LAPACK_LIB_ENABLED_DEPENDENCIES: BLAS[R]
+-- LAPACK_LIB_ENABLED_DEPENDENCIES: BLAS[O]
 
 -- Boost: No enabled dependencies!
 

diff --git a/tribits/CHANGELOG.md b/tribits/CHANGELOG.md
@@ -2,6 +2,14 @@
 ChangeLog for TriBITS
 ----------------------------------------
 
+## 2023-01-06:
+
+* **Changed:** Changed all TPL dependencies back to 'Optional' so that
+  disabling an external package/TPL will **not** disable any downstream
+  external packages/TPLs that list a dependency on that external package/TPL.
+  This undoes the change on [2022-10-20](#2022-10-20) and restores backward
+  compatibility to the behavior before that change.
+
 ## 2022-12-20:
 
 * **Deprecated:** The macro `set_and_inc_dirs()` is deprecated and replaced by

diff --git a/tribits/core/package_arch/TribitsReadDepsFilesCreateDepsGraph.cmake b/tribits/core/package_arch/TribitsReadDepsFilesCreateDepsGraph.cmake
@@ -223,7 +223,7 @@ macro(tribits_read_external_package_deps_files_add_to_graph  tplName)
     tribits_trace_file_processing(TPL  INCLUDE  "${absTplDepsFile}")
     include(${absTplDepsFile})
     foreach(depPkg  IN LISTS  ${tplName}_LIB_DEFINED_DEPENDENCIES)
-      global_set(${tplName}_LIB_DEP_REQUIRED_${depPkg}  TRUE)
+      global_set(${tplName}_LIB_DEP_REQUIRED_${depPkg}  FALSE)
     endforeach()
     tribits_append_forward_dep_packages(${tplName}  LIB)
   endif()

diff --git a/tribits/doc/guides/ReducedMockTrilinosOutput/ExpectedDependencies.txt b/tribits/doc/guides/ReducedMockTrilinosOutput/ExpectedDependencies.txt
@@ -17,9 +17,9 @@ Package dependencies information:
 
 -- MPI_FORWARD_LIB_DEFINED_DEPENDENCIES: Teuchos[O] Epetra[O]
 
--- BLAS_FORWARD_LIB_DEFINED_DEPENDENCIES: LAPACK[R] Teuchos[R] Epetra[R]
+-- BLAS_FORWARD_LIB_DEFINED_DEPENDENCIES: LAPACK[O] Teuchos[R] Epetra[R]
 
--- LAPACK_LIB_DEFINED_DEPENDENCIES: BLAS[R]
+-- LAPACK_LIB_DEFINED_DEPENDENCIES: BLAS[O]
 -- LAPACK_FORWARD_LIB_DEFINED_DEPENDENCIES: Teuchos[R] Epetra[R]
 
 -- Boost_FORWARD_LIB_DEFINED_DEPENDENCIES: Teuchos[O]
@@ -68,7 +68,7 @@ Dumping direct enabled dependencies for each package ...
 
 -- BLAS: No enabled dependencies!
 
--- LAPACK_LIB_ENABLED_DEPENDENCIES: BLAS[R]
+-- LAPACK_LIB_ENABLED_DEPENDENCIES: BLAS[O]
 
 -- Boost: No enabled dependencies!
 

diff --git a/tribits/doc/guides/TribitsGuidesBody.rst b/tribits/doc/guides/TribitsGuidesBody.rst
@@ -2269,6 +2269,11 @@ This defines all of the TPLs that ``<tplName>`` could directly depends on but
 only dependencies for enabled upstream TPLs will be added to the IMPORTED
 targets.
 
+NOTE: TPL-to-TPL dependencies are optional.  Therefore, in the above example,
+enabling the TPL ``<tplName>`` will not auto-enable a dependent upstream TPL
+``<upstreamTpl_i>``.  Likewise, disabling an upstream TPL ``<upstreamTpl_i>``
+will not auto-disable a dependent downstream TPL ``<tplName>``.
+
 
 TriBITS External Package/TPL Core Variables
 ...........................................
@@ -3676,19 +3681,24 @@ In more detail, these rules/behaviors are:
 
 .. _<Project>_ENABLE_ALL_FORWARD_DEP_PACKAGES enables downstream packages/tests:
 
-17) **<Project>_ENABLE_ALL_FORWARD_DEP_PACKAGES enables downstream packages/tests**:
-    Setting the user cache-variable
+17) **<Project>_ENABLE_ALL_FORWARD_DEP_PACKAGES enables downstream
+    packages/tests**: Setting the user cache-variable
     ``${PROJECT_NAME}_ENABLE_ALL_FORWARD_PACKAGES=ON`` will result in the
-    `downstream`_ ``PT`` packages and tests to be enabled (and all ``PT``
-    and ``ST`` packages and tests when
+    `downstream`_ ``PT`` internal packages and tests to be enabled (and all
+    ``PT`` and ``ST`` packages and tests when
     ``${PROJECT_NAME}_SECONDARY_TESTED_CODE=ON``) for all explicitly enabled
-    packages.  For example, configuring with ``Trilinos_ENABLE_Epetra=ON``,
-    ``Trilinos_ENABLE_TESTS=ON``, and
+    internal packages.  For example, in the mock Trilinos project, configuring
+    with ``Trilinos_ENABLE_Epetra=ON``, ``Trilinos_ENABLE_TESTS=ON``, and
     ``Trilinos_ENABLE_ALL_FORWARD_PACKAGES=ON`` will result the package
-    enables (and test and example enables) for the packages ``Triutils``,
-    ``EpetraExt``, ``ThyraCoreLibs``, ``ThyraEpetra`` and ``Thyra``.  For an
-    example, see `Explicit enable of a package and downstream packages and
-    tests`_.
+    enables (and test and example enables) for the downstream packages
+    ``Triutils``, ``EpetraExt``, ``ThyraCoreLibs``, ``ThyraEpetra`` and
+    ``Thyra``.  For an example, see `Explicit enable of a package and
+    downstream packages and tests`_.  Note that when setting this option, the
+    enable of an external package/TPL will **not** result in the auto-enable
+    of downstream internal packages.  For example, setting
+    ``Trilinos_ENABLE_BLAS=ON`` will not result in the auto-enable of any
+    internal packages that depend on ``BLAS`` like ``Teuchos`` (in the mock
+    Trilinos project).
 
 .. _${PROJECT_NAME}_ENABLE_ALL_PACKAGES: