From c4f7fc30145301cc1608df5ece78a2623c4ad8cb Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Fri, 6 Jan 2023 10:51:08 -0700 Subject: [PATCH] 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. --- test/core/DependencyUnitTests/CMakeLists.txt | 39 +++++++++++++++---- ...sts_EnableAllPackages_DumpDependencies.txt | 16 ++++---- tribits/CHANGELOG.md | 8 ++++ .../TribitsReadDepsFilesCreateDepsGraph.cmake | 2 +- .../ExpectedDependencies.txt | 6 +-- tribits/doc/guides/TribitsGuidesBody.rst | 30 +++++++++----- 6 files changed, 71 insertions(+), 30 deletions(-) diff --git a/test/core/DependencyUnitTests/CMakeLists.txt b/test/core/DependencyUnitTests/CMakeLists.txt index c1c46860f..63a25f0cc 100644 --- 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,9 +854,6 @@ 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" @@ -837,6 +861,7 @@ create_dependency_handling_test_case( "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 index a98ec6847..2544975a2 100644 --- 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 index c0ad93274..dfac622fc 100644 --- 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 index 9ec045e00..bd5f09e61 100644 --- 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 index 51dbaaa44..dccdd4e45 100644 --- 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 index 876566808..3b790406c 100644 --- a/tribits/doc/guides/TribitsGuidesBody.rst +++ b/tribits/doc/guides/TribitsGuidesBody.rst @@ -2269,6 +2269,11 @@ This defines all of the TPLs that ```` 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 ```` will not auto-enable a dependent upstream TPL +````. Likewise, disabling an upstream TPL ```` +will not auto-disable a dependent downstream TPL ````. + TriBITS External Package/TPL Core Variables ........................................... @@ -3676,19 +3681,24 @@ In more detail, these rules/behaviors are: .. __ENABLE_ALL_FORWARD_DEP_PACKAGES enables downstream packages/tests: -17) **_ENABLE_ALL_FORWARD_DEP_PACKAGES enables downstream packages/tests**: - Setting the user cache-variable +17) **_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: