Skip to content

Commit

Permalink
Make TPL-to-TPL dependencies optional (#557)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bartlettroscoe committed Jan 6, 2023
1 parent 3879de0 commit c4f7fc3
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 30 deletions.
39 changes: 31 additions & 8 deletions test/core/DependencyUnitTests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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!

Expand Down
8 changes: 8 additions & 0 deletions tribits/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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!

Expand Down
30 changes: 20 additions & 10 deletions tribits/doc/guides/TribitsGuidesBody.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
...........................................
Expand Down Expand Up @@ -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:

Expand Down

0 comments on commit c4f7fc3

Please sign in to comment.