search

TriBITSPub / TriBITS

TriBITS: Tribal Build, Integrate, and Test System,
http://tribits.org
Other
36 stars 47 forks source link

EPIC: Features to remove to simplify TriBITS #569

Open bartlettroscoe opened 1 year ago

bartlettroscoe commented 1 year ago

Description

There a number of features supported by TriBITS that if removed would (greatly) simplify TriBITS. Some of these major features that could be removed without seriously degrading the main features of TriBITS are:

  1. Removing support for subpackages (a lot of complexity here) [Minor break in backward comparability]
  2. Remove support for auto-linking to libraries in upstream packages and require manual linking of libraries (and not just the <UpstreamPkg>::all_libs targets but intermediate targets in dependent upstream packages)
  3. Switch to passing in include directories in for each target (the be added with target_include_directories()) and stop using the global include_directories() command.
  4. Remove all handling of compiler options (also mentioned in #411) [Minor break in backward comparability]
  5. Remove support for generating consistent explicit template instantiation (ETI) sets based on package dependencies (which sweeps backwards from downstream to upstream packages)
  6. Remove support for legacy TriBITS TPLs that take in lists of include directories, header file names, library directories, and library names and does a find (or allows the user to pass in the exact include directories and library files in order). [Major break in backward comparability]

NOTES:

Even with loosing all of the functionality listed above (and doing all of the refactorings in #411), what remains is TriBITS is very significant and not duplicated by anything in raw CMake or in any of the provided CMake modules. (Not even VTK Modules can do what TriBITS does with the flexible handling of internal and external packages.)

The improvements in dependency handling and streamlining builds provided by subpackages lost by removing support for subpackages in #1 is mitigated by moving to manual linking of intermediate targets in #2 (and also, manually linking makes the CMakeLists.txt files look more like raw CMake). As a consequence, for example, you would have to configure all of the Teuchos package but downstream packages could choose to just link against Teuchos::teuchoscore instead of Teuchos::all_libs. This means you are configuring more software that you might need but you are still only building the upstream libraries needed to build downstream targets. (But doing the install of a package would require building everything in that package, including all of its required upstream dependencies and needing to find all required upstream TPLs.) To mitigate the all-or-nothing build of existing packages currently broken into subpackages like Teuchos and SEACAS, package developers would be tempted to add their own conditional enable/disable logic which would create a lot of complexity in downstream packages to compensate for this. (This would shift complexity from the framework/tribits to the package developers which may not be a good idea and would likely make less-than-everything builds much more fragile to maintain.) This will break backward compatibility for existing Trilinos user configure scripts but will be easy and safe to fix in their scripts.

Switching to passing in include directories in #3 would allow ripping some code in code in TriBITS that gets the include directories directory property (set by the include_directories()) command and yields CMakeLists.txt files that look more like raw CMake. This would not impact Trilinos users at all.

Removing support for subpackages in #1 and some compiler option handling in #4, however, would break the ability to target specific compiler options to specific subpackages (see https://github.com/TriBITSPub/TriBITS/issues/442, https://github.com/TriBITSPub/TriBITS/pull/453 and https://github.com/trilinos/Trilinos/issues/10111, for example).

The code that implements #5 is pretty complex but it greatly simplifies creating consistent explicit template instantiation sets across dependent packages. Loosing this would likely make the Trilinos CMakeLists.txt files more complex and make Trilinos harder to configure for Trilinos customers.

For removing support for legacy TriBITS TPLs in #6, that requires that all external packages/TPLs be found with a simple find_package(<ExternalPkg>) command and it requires those to find 100% modern CMake packages (i.e. provides all modern CMake targets and calls find_dependency() on all of their upstream dependencies). But there is still a problem where the name of the imported target may change or may not even exist depending on the version of the external package installed or how it is found (see the expended discussion below). However, of all of the changes listed above, this change will most strongly impact Trilinos users and break all of their existing configure scripts for Trilinos. All of the other changes would have minor impact on Trilinos users.

bartlettroscoe commented 1 year ago

NOTE: One might consider removing support for external packages/TPLs with TriBITS and force packages to call find_package(<UpstreamPkg>). That would remove a good bit of complexity from TriBITS and would make it easier to pull out package subdirectories and build them as their own CMake project with little-to-know framework support. But the problem with that is that it would break the pattern of allowing the flexibility to decide, at runtime, if a package should be treated as internal or external.

bartlettroscoe commented 1 year ago

But if one could not completely remove support for external packages/TPLs in TriBITS as it would destroy the ability to treat internal and external packages in a flexible manner (as discussed above), TriBITS could be greatly simplified by removing support for the TriBITS Legacy TPL System (i.e. that accepted lists of include directories, library directories, and library names and build modern CMake targets and packages out of these) and instead just rely on find_package(<ExternalPkg>) calls. This would require moving to explicit linking of all targets since there is no standard target name for "all the library targets" (e.g. #2 above and bellow bullet). In this case, every external package/TPL pulled in with find_package(<ExternalPkg>) would need to be provided by either a 100% modern CMake compliant find module Find<ExternalPkg>.cmake or package config file <ExternalPkg>Config.cmake (and the names of the IMPORTED targets would have to be stable and consistent no matter if a find module or a package config file was used, see below).

With those changes and assumptions, all that the TriBITS system would need to do is to call find_package(<ExternalPkg>) and there would be no need for the FindTPL<ExternalPkg>.cmake files. That would remove 95% of the external packages/TPL software in TriBITS and Trilinos and other TriBITS projects (because you eliminate all of the FindTPL<ExternalPkg>.cmake files).

However, this beautify utopia of CMake packages is not currently practical right now because of issues like:

  1. The current ecosystem of CMake package with find modules Find<ExternalPkg>.cmake and package config files <ExternalPkg>Config.cmake are not 100% modern CMake compliant. (That is, they don't provide modern imported CMake targets and they don't correct call find_dependency() on their upstream dependencies.) That should be the goal in the future but it is not possible right now (e.g. see the NetCDF and HDF5 example in https://github.com/TriBITSPub/TriBITS/pull/535#issuecomment-1299219712).

  2. The names of the IMPORTED targets is not always the same when calling find_package(<ExternalPkg>) between find modules and package config files. For example, the FindHDF5.cmake module shipped with CMake 3.26 provides the HDF5::HDF5 interface imported target for all provided HDF5 libraries but the HDF5Config.cmake file installed by HDF5 1.11 does not seem to provide that target and instead provides a variable HDF5_EXPORT_LIBRARIES (see here) where HDF5_EXPORT_LIBRARIES is not supported by the FindHDF5.cmake module in CMake 3.26. (Therefore, it is impossible to write simple downstream code in CMakeLists.txt files that calls target_link_libraries(<libName> PRIVATE HDF5::HDF5).

Because of constraints and problems like above, we have to have TriBITS TPL find wrapper modules like FindTPLHDF5.cmake. These wrapper FindTPL<ExternalPkg>.cmake modules provide a way to take external pakages/TPLs and provide a 100% uniform modern CMake target <ExternalPkg>::all_libs can be linked against as easy as target_link_libraries(<libName> PRIVATE HDF5::all_libs) that that works no matter how HDF5 is found externally.