EPIC: Merge TriBITS concepts and implementation of Packages and TPLs #63

bartlettroscoe · 2015-04-14T11:36:48Z

Parent Issue:

EPIC: Initial TriBITS Modernization and Generalization #367

Child Issues:

Provide feedback on TriBITS package dependencies data-structures #377
Address some issues for initial implementation for treating internal packages as external (#63) #562
Address relocatable package installs and usage of <Package>Config.cmake files #563

Blocked by:

Move to modern CMake target-based propagation of build information #299

Description

The current design of TriBITS has TriBITS Packages and TriBITS TPLs and distinct separate entities. TriBITS Packages are always built from source in the current TriBITS design while TriBITS TPLs are always pre-built and just pointed to at configure time. This has been a successful model for Trilinos, CASL VERA, and other related projects for several years. However, this model does not scale well to larger meta-projects and does not provide sufficient flexibility to handle more efficient alternative development workflows and other use cases.

The basic idea being proposed in this story is to extend TriBITS to allow some pre-built (and possibly installed) TriBITS packages to be treated as TPLs and then configure/build some downstream TriBITS packages from source. This would be accomplished through the usage of <Package>Config.cmake files to get the needed information. Likewise, this work would allow some TPLs could be built from inside of the TriBITS project like other TriBITS packages.

The details of the of the plan, the motivating use cases, and the design implications are given in this Google Doc.

This refactoring should implement and respect this proposed standard for <Package>Config.cmake files.

Remaining Tasks

The text was updated successfully, but these errors were encountered:

nschloe · 2015-08-30T22:44:35Z

If I may comment on this from the perspective of a (distro) package maintainer:

Every piece of software has a bunch of dependencies, in former times only listed in a README, and more recently rigorously checked at configure time.
Some projects ship third-party software (for example gmsh with the idea to make it easier for users to install the software. Unfortunately, in all cases I'm aware of, the bundled software is barely updated and doesn't encourage pushing changes upstream. This way, factual forks of the dependent libraries are created.

Constrasting this, PETSc does not bundle its dependencies, but has its (custom) build system download them for you from the appropriate source. From a package maintainer's point of view, it's not quite clear why the fetch-&-build of TPL should be part of the build system for the actual code. Because, in fact, what you're doing is the job of a distribution (dependency managment, installation etc.).

A cleaner and equally easy way helping the user install TPLs is to provide a separate script that installs all deps (e.g., install_trilinos_dependencies.sh). This way, the lazy user can use this convenience script, or install the dependencies via the distribution (e.g., apt-get install libsuperlu-dev libnetcdf-dev [...]). FEniCS goes that way.
The advantage that I see in this approach is a separation of concerns and consequently an easier-to-maintain codebase.

This is bit of a hack but, for the most part, this was pretty easy. Here is how I did this: 1) Added the necessary TriBITS project files ProjectName.cmake, PackagesList.cmake, TPLsList.cmake and Version.cmake. 2) Created a new TPL called TrilinosTpl and made xSDKTrilinos only have a required dependence on this TPL, in addition to PETSc and HYPRE. 3) The FindTPLTrilinosTpl.cmake file just calls FIND_PACKAGE(Trilinos). There is a slight hack relating to the need to call LINK_DIRECTORIES(). This is really unfortunate. Hopefully the <Package>Config.cmake files genreated by TriBITS don't require this hack. We will address that as pat of TriBITSPub/TriBITS#63. 4) Had to copy in Teuchos_StandardParallelUnitTestMain.cpp because Trilinos does not install this file. 5) Had to enabled variables like ${PACKAGE_NAME}_ENABLE_MueLu since these packages don't really exist in the stand-alone build of xSDKTrilinos (at least as TriBITS is concerned). When I get around to the major refactoring and enhancement TriBITSPub/TriBITS#63 then these hacks can be removed and the build of xSDKTrilinos will be much cleaner.

) This commit adds the direct TPL dependencies for the 'Boost' and 'BoostLibs' TPLs to all of the STK subpackages. This has been confirmed to result in '-isystem' being used instead of '-I' for the Boost include directory for all object files compiled for Boost libraries (except STKClassic which I did not bother updating since no-one is using anymore). Note that this does *not* currently result in '-isystem' getting used for Boost includes for most of the object files related to test executbles in the STK subpackages under: * stk/stk_doc_tests/ * stk/stk_unit_tests/ This appears to be because this approach only works for exectuables that have upstream libraries in the same SE package. However, I don't think this is worthwile to fix because the driving use case in #1137 should only need the STK libraries built, not the STK tests and executables. The major TriBITS refactoring in TriBITSPub/TriBITS#63 will likely fix this automatically.

This provides useful info about TriBITS-compliant external packages that are found and it also asserts that <Package>_DIR is non-empty. It should be impossible for this to happen but this was observed on a user's machine as described in: https://gitlab.kitware.com/cmake/cmake/-/issues/24667 I was not able to reproduce this issue myself but having this check would have saved us at least 4 FTE hours and a lot of frustration.

…make (#63) I also removed the unused macro tribits_exclude_autotools_files() which is not used in Trilinos anymore.

This provides useful info about TriBITS-compliant external packages that are found and it also asserts that <Package>_DIR is non-empty. It should be impossible for this to happen but this was observed on a user's machine as described in: https://gitlab.kitware.com/cmake/cmake/-/issues/24667 I was not able to reproduce this issue myself but having this check would have saved us at least 4 FTE hours and a lot of frustration.

Some refinements and fixes (#63, trilinos/Trilinos#11976)

"

…/TriBITS#63"" This reverts commit ae19a59. This brings back the version of TriBITS snapshotted in Trilinos PR trilinos/Trilinos#11380.

"

…/TriBITS#63"" This reverts commit ae19a59. This brings back the version of TriBITS snapshotted in Trilinos PR trilinos/Trilinos#11380.

"

…/TriBITS#63"" This reverts commit ae19a59. This brings back the version of TriBITS snapshotted in Trilinos PR trilinos/Trilinos#11380.

"

…/TriBITS#63"" This reverts commit ae19a59. This brings back the version of TriBITS snapshotted in Trilinos PR trilinos/Trilinos#11380.

"

…/TriBITS#63"" This reverts commit ae19a59. This brings back the version of TriBITS snapshotted in Trilinos PR trilinos/Trilinos#11380.

…/undefined packages (TriBITSPub#63) This is a better solution to the failure reported in trilinos/Trilinos#12291 and applied in the reverted commit: 05d4429 "Only assert defined vars conditionally" Author: Samuel Browne <[email protected]> Date: Thu Sep 28 13:51:05 2023 -0600 (6 days ago) (which I had suggested at the time). This new implementation sets HAVE_<PACKAGE_NAME_UC>_<UPSTREAM_PACKAGE_NAME_UC>=OFF at the project level right away when the dependencies are being read in and <upstreamPackageName> is not defined. This makes for more robust downstream CMake code that works with optional upstream dependencies when those dependencies are not defined (like when the package <upstreamPackageName> was removed from the reduced source tree by the reduced tarball feature). A downstream package should not care whether an upstream dependence is disabled or is missing entirely; the reaction should be the same and it should be able to rely on the same variables.

…ME_UC> (TriBITSPub#63) I noticed this while working on the last commit. Part of this was changing the name of: OPTIONAL_DEP_PACKAGE_NAME to: UPSTREAM_PACKAGE_NAME in: ${PACKAGE_NAME}_ENABLE_${UPSTREAM_PACKAGE_NAME} Why would anyone try to disable support for a required package so trying to set ${PACKAGE_NAME}_ENABLE_${UPSTREAM_PACKAGE_NAME}=OFF where ${UPSTREAM_PACKAGE_NAME} is a required upstream dependency of ${PACKAGE_NAME} makes no sense (but TriBITS sets these vars to 'ON' even for required dependencies to simplify certain types of logic).

…/undefined packages (TriBITSPub#63) This is a better solution to the failure reported in trilinos/Trilinos#12291 and applied in the reverted commit: 05d4429 "Only assert defined vars conditionally" Author: Samuel Browne <[email protected]> Date: Thu Sep 28 13:51:05 2023 -0600 (6 days ago) (which I had suggested at the time). This new implementation sets HAVE_<PACKAGE_NAME_UC>_<UPSTREAM_PACKAGE_NAME_UC>=OFF at the project level right away when the dependencies are being read in and <upstreamPackageName> is not defined. This makes for more robust downstream CMake code that works with optional upstream dependencies when those dependencies are not defined (like when the package <upstreamPackageName> was removed from the reduced source tree by the reduced tarball feature). A downstream package should not care whether an upstream dependence is disabled or is missing entirely; the reaction should be the same and it should be able to rely on the same variables.

…ME_UC> (TriBITSPub#63) I noticed this while working on the last commit. Part of this was changing the name of: OPTIONAL_DEP_PACKAGE_NAME to: UPSTREAM_PACKAGE_NAME in: ${PACKAGE_NAME}_ENABLE_${UPSTREAM_PACKAGE_NAME} Why would anyone try to disable support for a required package so trying to set ${PACKAGE_NAME}_ENABLE_${UPSTREAM_PACKAGE_NAME}=OFF where ${UPSTREAM_PACKAGE_NAME} is a required upstream dependency of ${PACKAGE_NAME} makes no sense (but TriBITS sets these vars to 'ON' even for required dependencies to simplify certain types of logic).

Noticed this while working on earlier commits.

Noticed some of these issues while working on earlier commits. Some stuff I did here: * Update documentation for printing about missing packages to include reduced tarballs. * Added another configure case test to TribitsExampleProject_NoFortran_reduced_tarball to test only printing notes about missing packages (so configure takes place twice and deletes the build directory each time). * Changed logic for when warning about missing packages to not just be when setting ${depPkg}_ALLOW_MISSING_EXTERNAL_PACKAGE=TRUE but also for values of ${PROJECT_NAME}_ASSERT_DEFINED_DEPENDENCIES not in ${PROJECT_NAME}_ASSERT_DEFINED_DEPENDENCIES_ERROR_VALUES_LIST. * Added prefix "-- " top some "NOTE:" print lines. * Update TribitsReadAllProjectDepsFilesCreateDepsGraph.cmake unit tests to read in info about the TPLs to avoid printing about missing external packages/tpls * Update the documentation for tribits_process_tpls_lists() and tribits_process_packages_and_dirs_lists() to correctly describe usage.

This was left over from the main refactoring with TriBITS TriBITSPub#63 to combine the handling of internal and external packages.

Noticed some of these issues while working on earlier commits. Some stuff I did here: * Update documentation for printing about missing packages to include reduced tarballs. * Added another configure case test to TribitsExampleProject_NoFortran_reduced_tarball to test only printing notes about missing packages (so configure takes place twice and deletes the build directory each time). * Changed logic for when warning about missing packages to not just be when setting ${depPkg}_ALLOW_MISSING_EXTERNAL_PACKAGE=TRUE but also for values of ${PROJECT_NAME}_ASSERT_DEFINED_DEPENDENCIES not in ${PROJECT_NAME}_ASSERT_DEFINED_DEPENDENCIES_ERROR_VALUES_LIST. * Added prefix "-- " top some "NOTE:" print lines. * Update TribitsReadAllProjectDepsFilesCreateDepsGraph.cmake unit tests to read in info about the TPLs to avoid printing about missing external packages/tpls * Update the documentation for tribits_process_tpls_lists() and tribits_process_packages_and_dirs_lists() to correctly describe usage.

This was left over from the main refactoring with TriBITS TriBITSPub#63 to combine the handling of internal and external packages.

…/undefined packages (TriBITSPub#63) This is a better solution to the failure reported in trilinos/Trilinos#12291 and applied in the reverted commit: 05d4429 "Only assert defined vars conditionally" Author: Samuel Browne <[email protected]> Date: Thu Sep 28 13:51:05 2023 -0600 (6 days ago) (which I had suggested at the time). This new implementation sets HAVE_<PACKAGE_NAME_UC>_<UPSTREAM_PACKAGE_NAME_UC>=OFF at the project level right away when the dependencies are being read in and <upstreamPackageName> is not defined. This makes for more robust downstream CMake code that works with optional upstream dependencies when those dependencies are not defined (like when the package <upstreamPackageName> was removed from the reduced source tree by the reduced tarball feature). A downstream package should not care whether an upstream dependence is disabled or is missing entirely; the reaction should be the same and it should be able to rely on the same variables.

…ME_UC> (TriBITSPub#63) I noticed this while working on the last commit. Part of this was changing the name of: OPTIONAL_DEP_PACKAGE_NAME to: UPSTREAM_PACKAGE_NAME in: ${PACKAGE_NAME}_ENABLE_${UPSTREAM_PACKAGE_NAME} Why would anyone try to disable support for a required package so trying to set ${PACKAGE_NAME}_ENABLE_${UPSTREAM_PACKAGE_NAME}=OFF where ${UPSTREAM_PACKAGE_NAME} is a required upstream dependency of ${PACKAGE_NAME} makes no sense (but TriBITS sets these vars to 'ON' even for required dependencies to simplify certain types of logic).

Noticed some of these issues while working on earlier commits. Some stuff I did here: * Update documentation for printing about missing packages to include reduced tarballs. * Added another configure case test to TribitsExampleProject_NoFortran_reduced_tarball to test only printing notes about missing packages (so configure takes place twice and deletes the build directory each time). * Changed logic for when warning about missing packages to not just be when setting ${depPkg}_ALLOW_MISSING_EXTERNAL_PACKAGE=TRUE but also for values of ${PROJECT_NAME}_ASSERT_DEFINED_DEPENDENCIES not in ${PROJECT_NAME}_ASSERT_DEFINED_DEPENDENCIES_ERROR_VALUES_LIST. * Added prefix "-- " top some "NOTE:" print lines. * Update TribitsReadAllProjectDepsFilesCreateDepsGraph.cmake unit tests to read in info about the TPLs to avoid printing about missing external packages/tpls * Update the documentation for tribits_process_tpls_lists() and tribits_process_packages_and_dirs_lists() to correctly describe usage.

This was left over from the main refactoring with TriBITS TriBITSPub#63 to combine the handling of internal and external packages.

…AGE_NAME (TriBITSPub#63) In an earlier refactoring to combine the handling of internal and external packages, only a partial renaming of vars in the documentation was completed correctly. I just noticed this while reading the documentation while doing a Trilinos PR review where I referred to this documentation.

…tion (TriBITSPub#63) I noticed this while doing a Trilinos PR review while (indirectly) referring to this documentation.

bartlettroscoe added the type: enhancement label Apr 14, 2015

bartlettroscoe self-assigned this Apr 14, 2015

bartlettroscoe added this to the 2_selected milestone Apr 14, 2015

bartlettroscoe mentioned this issue Apr 14, 2015

Prune export libraries and package lists #60

Open

This was referenced May 22, 2015

Extend TriBITS TPL system to better accommodate Find<TPLNAME>.cmake modules #67

Closed

Implement support for pre-extra repositories #73

Closed

bartlettroscoe mentioned this issue Aug 30, 2015

export more system/compiler info #75

Merged

bartlettroscoe mentioned this issue Nov 12, 2015

static vs shared library #95

Closed

bartlettroscoe mentioned this issue Dec 24, 2015

Implement CMake performance tips to reduce configure times #18

Closed

bartlettroscoe mentioned this issue Feb 2, 2016

NetCDF library configuration trilinos/Trilinos#99

Closed

bartlettroscoe added the ready label Feb 19, 2016

bartlettroscoe modified the milestones: 1_backlog, 2_selected Feb 19, 2016

bartlettroscoe removed the ready label Feb 19, 2016

bartlettroscoe removed this from the 1_backlog milestone Feb 19, 2016

bartlettroscoe mentioned this issue Mar 3, 2016

Netcdf and HDF5 ordering trilinos/Trilinos#156

Closed

bartlettroscoe mentioned this issue Apr 11, 2016

Create a SEMSDevEnv.cmake file to automatically use loaded SEMS dev env trilinos/Trilinos#158

Closed

rppawlo mentioned this issue Apr 13, 2016

Fix TPL link ordering when all TPL libs currently exist on link line #115

Open

bartlettroscoe mentioned this issue Jun 29, 2016

Incorporating Makefile.kokkos into codebase that uses cmake kokkos/kokkos#155

Closed

bartlettroscoe mentioned this issue Aug 1, 2016

(Trilinos) build warning in atomic_assign, with Kokkos::complex kokkos/kokkos#177

Closed

bartlettroscoe mentioned this issue Nov 8, 2016

CMake mods for smarter Netcdf, HDF5, PNetcdf, and CGNS TPL config trilinos/Trilinos#787

Closed

bartlettroscoe mentioned this issue Dec 8, 2016

Remove duplication and improve reuse of MiniTensor code in Trilinos trilinos/Trilinos#609

Closed

This was referenced Feb 24, 2017

Netcdf and related Find TPL modules refactoring to improve default find #156

Merged

Decoupling include/link dirs from 'TRIBITS_ADD_LIBRARY' #182

Closed

bartlettroscoe mentioned this issue Mar 15, 2017

Moving boost to a system include trilinos/Trilinos#1137

Closed

bartlettroscoe added the ready label Mar 31, 2017

bartlettroscoe mentioned this issue Apr 12, 2017

MiniTensor, Domi don't properly install their configs trilinos/Trilinos#1225

Closed

bartlettroscoe added a commit that referenced this issue Jun 27, 2023

Factor out TribitsConfigureTiming.cmake and TribitsPackagingSupport.c…

8a0563b

…make (#63) I also removed the unused macro tribits_exclude_autotools_files() which is not used in Trilinos anymore.

bartlettroscoe added a commit that referenced this issue Jun 27, 2023

Merge pull request #585 from TriBITSPub/63-refinements-2023-06

6d9eaef

Some refinements and fixes (#63, trilinos/Trilinos#11976)

cabejackson pushed a commit to trilinos/Moertel that referenced this issue Aug 16, 2023

Revert "TriBITS Snapshot 2022-12-12 working toward TriBITSPub/TriBITS#63

c61dd65

"

cabejackson pushed a commit to trilinos/FEI that referenced this issue Aug 16, 2023

Revert "TriBITS Snapshot 2022-12-12 working toward TriBITSPub/TriBITS#63

a8c7c7c

"

cabejackson pushed a commit to trilinos/Rythmos that referenced this issue Aug 16, 2023

Revert "TriBITS Snapshot 2022-12-12 working toward TriBITSPub/TriBITS#63

b797941

"

cabejackson pushed a commit to trilinos/Komplex that referenced this issue Aug 16, 2023

Revert "TriBITS Snapshot 2022-12-12 working toward TriBITSPub/TriBITS#63

33b0001

"

cabejackson pushed a commit to trilinos/Pike that referenced this issue Aug 16, 2023

Revert "TriBITS Snapshot 2022-12-12 working toward TriBITSPub/TriBITS#63

3765c85

"

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Oct 9, 2023

Improve printing and documentation on missing packages (TriBITSPub#63)

eaad800

Noticed this while working on earlier commits.

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Oct 10, 2023

Remove unused arg pkgsOrTpls (TriBITSPub#63)

14a3f2e

This was left over from the main refactoring with TriBITS TriBITSPub#63 to combine the handling of internal and external packages.

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Oct 10, 2023

Remove unused arg pkgsOrTpls (TriBITSPub#63)

3001297

This was left over from the main refactoring with TriBITS TriBITSPub#63 to combine the handling of internal and external packages.

bartlettroscoe changed the title ~~Merge TriBITS concepts and implementation of Packages and TPLs~~ EPIC: Merge TriBITS concepts and implementation of Packages and TPLs Jan 5, 2024

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this issue Jan 26, 2024

Remove unused arg pkgsOrTpls (TriBITSPub#63)

48d93e7

This was left over from the main refactoring with TriBITS TriBITSPub#63 to combine the handling of internal and external packages.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EPIC: Merge TriBITS concepts and implementation of Packages and TPLs #63

EPIC: Merge TriBITS concepts and implementation of Packages and TPLs #63

bartlettroscoe commented Apr 14, 2015 •

edited

Loading

nschloe commented Aug 30, 2015

EPIC: Merge TriBITS concepts and implementation of Packages and TPLs #63

EPIC: Merge TriBITS concepts and implementation of Packages and TPLs #63

Comments

bartlettroscoe commented Apr 14, 2015 • edited Loading

Description

Remaining Tasks

nschloe commented Aug 30, 2015

bartlettroscoe commented Apr 14, 2015 •

edited

Loading