search

TriBITSPub / TriBITS

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

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

Open bartlettroscoe opened 9 years ago

bartlettroscoe commented 9 years ago

Parent Issue:

Child Issues:

Blocked by:

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

nschloe commented 9 years ago

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.

sethrj commented 4 years ago

@bartlettroscoe Any progress? 😬

bartlettroscoe commented 4 years ago

@bartlettroscoe Any progress? 😬

@sethrj, not in a couple of weeks. Need to get a few other things out of the way before getting back into this. Sorry for the delays.

sethrj commented 4 years ago

Ross, I hate to keep bugging you, but is there any way of bumping the priority on this?

bartlettroscoe commented 4 years ago

Ross, I hate to keep bugging you, but is there any way of bumping the priority on this?

@sethrj, I am working on getting some other deliverables pushed back so I can work on this sooner rather than later. Also, I may be getting some help with this trough ECP xSDK.

bartlettroscoe commented 3 years ago

Relates to:

jwillenbring commented 3 years ago

@bartlettroscoe I was asked about this at a meeting today. Is there any update or current estimate as to when this work will happen? I am just passing on a question I received. Thanks.

bartlettroscoe commented 3 years ago

@bartlettroscoe I was asked about this at a meeting today. Is there any update or current estimate as to when this work will happen? I am just passing on a question I received. Thanks.

@jwillenbring I am wrapping up some work with SPARC ATDM Trilinos configuration over the next week or so (see ATDV-408) and then will will be working on this and #299 at a rate of 50% of my time until the basics are done.

bartlettroscoe commented 3 years ago

With the merge commit 2566a2b00987d18a779c2e789edb886a12d81e50, the outstanding branch '63-merge-tpls-packages-1' is now on TriBITS 'master'. Note that there is now the beginnings of an appendix for a TriBITS System Maintainers Guide:

which includes a macro/function call graph for the generation of the package dependency data structures:

which includes file names and line numbers.

The notes that I took in the creation and merging of the branch are given below.

TASK LIST 12/23/2019 through 5/24/2021 (click to expand) . * Create new TriBITS branch '63-merge-tpls-packages' [Done] * Read over all all of the material in https://github.com/TriBITSPub/TriBITS/issues/63 and https://docs.google.com/document/d/1T9KKrtgFm8-TSEuq119gdDR9tJMlBN3nAVjcN15Ge6k and update anything that needs updated [Done] * Read over https://cmake.org/cmake/help/v3.13/command/find_package.html and take notes [Done] * Read over https://cmake.org/cmake/help/v3.13/manual/cmake-packages.7.html and take notes [Done] * Write up beginning of data-structures in *.rst file to be sucked into appendices of the TriBITS Developers Guide for managing TriBITS TPLs and Packages and determine how to manage TPLs inside of the Packages data-structures with minor additions. (The goal here is to eliminate a lot of duplicate code for managing TPLs and Packages.) [Done] * Read in /TPLsList.cmake before /PackagesList.cmake [Done] * Add documentation for ${PROJECT_NAME}_PACKAGE_DIRS and ${PACKAGE_NAME}_SOURCE_DIR [Done] * Research if there really is a need for ${PROJECT_NAME}_PACKAGE_DIRS and ${PROJECT_NAME}_SE_PACKAGE_DIRS where one needs to look up an (SE) Package name given the path to its source tree [Done] * Replace ${PROJECT_NAME}_[SE_]PACKAGE_DIRS with ${PACKAGE_NAME}_REL_SOURCE_DIR: - Add ${PACKAGE_NAME}_REL_SOURCE_DIR for package and subpackages [Done] - Replace usage of ${PROJECT_NAME}_PACKAGE_DIRS with ${PACKAGE_NAME}_REL_SOURCE_DIR [Done] - Factor PACKAGE_DIR out of TRIBITS_READ_PACKAGE_DEPENDENCIES() macros and just use ${PACKAGE_NAME}_REL_SOURCE_DIR [Done] - Remove setting or referencing ${PROJECT_NAME}_[SE_]PACKAGE_DIRS vars [Done] - Remove unused PACKAGE_IDX vars [Done] * Back up current branch to rab-github (63-merge-tpls-packages-2020-01-16), clean up commits so far, and force push [Done] * Factor out a macro TRIBITS_READ_TOPLEVEL_DEFINED_EXTERNAL_AND_INTENRAL_PACKAGES() from TRIBITS_READ_PACKAGES_PROCESS_DEPENDENCIES_WRITE_XML() [Done] * Change name of TribitsPackageDependenciesDataStructures.rst to TribitsSystemDataStructures.rst [Done] * Factor out function TRIBITS_READ_PACKAGES_PROCESS_DEPENDENCIES_WRITE_XML() and the functions it calls into its own source file TribitsReadPackageProcessDepenenciesWriteXml.cmake [Done] * Change name of TRIBITS_READ_PACKAGE_DEPENDENCIES_CREATE_GRAPH() to TRIBITS_READ_ALL_PACKAGE_DEPENDENCIES_AND_CREATE_GRAPH() [Done] * Rename TRIBITS_READ_PACKAGE_DEPENDENCIES() to TRIBITS_READ_TOPLEVEL_PACKAGE_DEPENDENCIES_AND_ADD_TO_GRAPH() [Done] * Rename TRIBITS_READ_SUBPACKAGE_DEPENDENCIES() to TRIBITS_READ_SUBPACKAGE_DEPENDENCIES_AND_ADD_TO_GRAPH() [Done] * Rename TRIBITS_READ_ALL_PACKAGE_DEPENDENCIES_AND_CREATE_GRAPH() to TRIBITS_READ_ALL_PACKAGE_DEPS_AND_CREATE_DEPS_GRAPH() [Done] * Rename file TribitsReadPackageProcessDepenenciesWriteXml.cmake to TribitsReadPackagesProcessDepenenciesWriteXml.cmake [Done] * Change name of branch '63-merge-tpls-packages' to '63-merge-tpls-packages-1' [Done] * Extend CMake rST documentation tool to provide file name and line number for the location of each function and macro in the source code [Done] * Add section to TriBITS System Developers guide that shows high-level functions call stack with links to the major functions (and have each function's documentation clearly document what they read and what they compute) ... I documented the macros and functions that create the dependency data-structure way down to the bare bones [Done] * Read over functions that create the dependency data-structures and document them (all the way down to bare bones) [Done] * Change name of TRIBITS_READ_DEFINED_EXTERNAL_AND_INTENRAL_TOPLEVEL_PACKAGES_LISTS() to TRIBITS_READ_DEFINED_EXTERNAL_AND_INTERNAL_TOPLEVEL_PACKAGES_LISTS() [Done] * Change name of TRIBITS_PROCESS_ALL_REPOSTIORY_DEPENDENCY_SETUP_LOGIC() to TRIBITS_PROCESS_ALL_REPOSITORY_DEPENDENCY_SETUP_LOGIC() [Done] * Merge branch 'master' into branch '63-merge-tpls-packages-1' and address any merge conflicts: - Work through initial set of conflicts [Done] - Enable full test suite with CMake 3.17 and run [Passed] - Verify patch in commit 2dcd09d is applied [Done] - Verify patch in commit 9235eef is applied [Done] * Run full set of checkin-test-crf452.sh tests [PASSED] * Test out updated TriBITS with Trilinos 'develop' branch and ATDM Trilinos 'clang' build [PASSED] * Merge to 'master' [Done]
DETAILS NOTES 12/23/2019 through 5/24/2021 (click to expand) . **DETAILED NOTES:** **(2019/12/23)** Let's focus on CMake 3.14 documentation for find_package(). I think we can consider making that the new required version of CMake after we get Spack installs of CMake working very well. It looks like CMake 3.14 find_package() doucmentation is much closer to the current CMake 3.16 documentation `find_package()` documentation. In particular, the CMake 3.14 documenation states that `find_package()` must use look in the directory specified by `_ROOT` first. Looking at the other versions of CMake, it looks like special handling of `_ROOT` was added in CMake 3.12. Therefore, I think that might be the new minimum version of CMake required for a refactored TriBITS. So now I am reading the documentation at https://cmake.org/cmake/help/v3.13/command/find_package.html carefully. That version of the documentation looks to be the same as for the newer CMake versions thorugh 3.16. That is a pretty new version of CMake where CMake 3.13.0 was put out on 11/21/2018 (over a year ago). Hopefully that is new enough. So what can I learn from reading this: * The big problem is that CMake does not define any standard for how the "found" parts of an external package are pulled in. All it says is "When the package is found, package-specific information is provided through variables and Imported Targets documented by the package itself." That is why we need TriBITS!!! * Another problem is the squishy statement "Available components and their influence on whether a package is considered to be found are defined by the target package." How can you write general processing for something like that? * When using FIND_PACKAGE() by default it uses "Module Mode" which means that a `Find.cmake` file must exist to look for the parts of the external package. If that find operation can't find the external package, if falls back to "Config mode". This fallback to "Config Mode" is disabled if the `MODULE` option is given. * Looks like CMake `FIND_PACKAGE()` itself cannot be made to understand the semantic versioning standard as the documentation says "CMake does not establish any convention for the meaning of version numbers." And given that, how is it supposed to implement "When the [version] argument is given Config mode will only find a version of the package that claims compatibility with the requested version (format is major[.minor[.patch[.tweak]]])."? => So it is the external package's own `ConfigVersion.cmake` that interprets and determines compatibility with the requested version. That is great! That means that the external package itself can define logic for the semantic versioning standard. This just means that you can't use the utilities that CMake provides to generate the `ConfigVersion.cmake` file. TriBITS will need to generate its own (for packages that accept that standard). * It occurred to me that one way that I might be able to manipulate FIND_PACKAGE() to prefer TriBITS-defined `Config.cmake` files is to pass in the version string `TRIBITS-PKG` and then have TriBITS write logic in generated/installed `ConfigVersion.cmake` files that only sets `PACKAGE_VERSION_COMPATIBLE=TRUE` if this is a TriBITS-generated file. That way, TriBITS framework code can first search for the package using `FIND_PACKAGE( TRIBITS-PKG CONFIG ...)`. That is what will allow two-level `FIND_PACAKGE()` logic. * There is actually a built-in CMake cache var called `CMAKE_DISABLE_FIND_PACKAGE_` that is undefined but if set to `TRUE` then a non-`REQUIRED` optional `FIND_PACKAGE( ...)` will be skipped. So it seems that CMake does have a standard way to disable optional upstream external packages! So completely reading the documentation https://cmake.org/cmake/help/v3.13/command/find_package.html was pretty educational. Now it is time to carefully read over https://cmake.org/cmake/help/v3.13/manual/cmake-packages.7.html and take notes and see what I can learn: * After a package is found with `FIND_PACKAGE( ...)`, the variable `_FOUND` is set to `TRUE` and the variable `_DIR` is set the location of the found `Config.cmake` file (not the base install location of the package)! * It says "A NAMESPACE with double-colons is specified when exporting the targets for installation." So it seems that is why Jeremy W. prefixed the exported targets with `Kokkos::` in the updated Kokkos CMake build system. It seems that CMake has built-in logic for this as it will issue an error with a nice error message with such a target is not defined yet. * I find the info in the section "Creating Packages" (https://cmake.org/cmake/help/v3.13/manual/cmake-packages.7.html#creating-packages) to be pretty confusing. I am just not sure what all of that is doing. It will take me time inspecting the TriBITS code for generating these files and the CMake examples using the standard modules `GenerateExportHeader.cmake` and `CMakePackageConfigHelpers.cmake` to really understand what CMake is doing with these. However, I think it is really critical to understand these before I start refactoring too much here. * The function `write_basic_package_version_file()` in the standard CMake module `CMakePackageConfigHelpers.cmake` (https://cmake.org/cmake/help/v3.13/module/CMakePackageConfigHelpers.html#generating-a-package-version-file) is used to help automate the generation of a `ConfigVersion.cmake` file. It takes in the argument `COMPATIBILITY ` which does not include a semantic versioning option (which is not the same thing as `SameMajorVersion`). Therefore, to support semantic versioning, you will need to generate your own version of that file. But that is all youhav to do, no big deal. It is clear that I am going to need to do a lot of studying, experiments, and inspections to understand how CMake expects one to define and use `Config.camke` and `ConfigVersion.camke` files. I will need to do this before I make any changes to the way the current TriBITS export files system works. Also, I will need to really understand this to make sure that I can keep generating `Makefile.export.` files correctly. The full set of refactorings that I want to do is going to take a lot of time. I really need to figure out an incremental path to doing these refactorings to make sure that this does not turn into a big all-or-nothing refactoring. I fear that if I don't bite the bullet right now that it will be too late for TriBITS (and it may already be too late). **(2020/01/14)** I have gotten way off track doing other things. It is now or never with this. I have to get this done before the ECP annual meeting or this is never going to happen. I fear that I am going to have to re-read over everything to get into this ... I read over some of this but it is time to get to work. I need to get my hands dirty and get into this to get motivated to get this done. I have take a file README.DEPENDENCIES and turned it into a file TribitsPackageDependenciesDataStructures.rst that I sucked into TribitsDevelopersGuide.rst as a start to official documentation for the internals of the TriBITS system. The first thing I want to do is to merge TPL data-structures into package data-structures so that they are one and the same. This will also be a chance to allow TPLs to define dependencies on each other recursively. I will do that with new files called FindDependencies.cmake and it will just call TRIBITS_TPL_DEFINE_DEPENDENCIES() (which will just call TRIBITS_PACKAGE_DEFINE_DEPENDENCIES()). That would be so great to get that done! The code that reads the /Dependencies.cmake file is package data-structures is the functions: * TRIBITS_PACKAGE_DEFINE_DEPENDENCIES() * TRIBITS_READ_PACKAGE_DEPENDENCIES() Since we need to maintain backward compatibility, the logic that combines the "XXX_TPLS" vars into the "XXX_PACKAGES" vars needs to happen in the function TRIBITS_READ_PACKAGE_DEPENDENCIES() initially because a lot of Dependencies.cmake files are still just directly setting the raw CMake vars :-( So basically, I want to get rid of persistent data-structures that keep track of lists of TPLs. Instead, I want to be able to separate out the list of internal packages (i.e. regular TriBITS packages) and external packages (e.g. TPLs) on demand in various logic like printing them out. The less derived data-structures, the better. I need to come up with an incremental refactoring plan because if I try to just update the existing data-structures then this is going to turn into a nighmare big-bang refactoring. Therefore, what I think I should do is to define new variables with long ugly names that I can then replace at the end to clean everything up. I could use a postfix called something like "_REF63" to mean this refactor and then I could start by fulling up those data-structures, adding unit tests for those, and then slowly replace the existing algorithms that use "XXX" vars with "XXX_REF63" vars. Then, when I have gotten rid of the usage of all of the "XXX" vars, then I can just replace "_REF63" with "" and the refactoring will essentially be done! I can do this in waves to makes sure that I catch everything. If I do this carefully and incrementally, there will be no risk here and I can drive forward quickly. So it seems clear how this refactoring should be done. We should start by adding these variables upstream and then using them downstream flowing forward and verifying that the updated code works as we go. Then after the entire refactoring is done, we can delete the usage of the old "XXX" vars and then remove the "_REF63" from the "XXX_REF63" vars and the refactoring should be done. If we do this type of refactoring in stages as we go, we can do this safely and incrementally. Lets get on it. Hum, I am trying to change the code to read and process TPLsList.cmake before PackagesList.cmake (because I want them to both read into the same list of variables) but I am immediately getting a ton of failing tests :-( ... Boy, there was some bad programming, even in this very simple code :-( I just realized that we will need to keep separate arrays for the external packages (TPLs) and the internal packages as we loop over the TriBITS repositories. That is not a big deal. **(2020/01/15)** I am looking at eliminating the array ${PROJECT_NAME}_PACKAGE_DIRS because that array does not make sense for external packages. Instead, I would like to replace the need for relative paths with a new variable ${PACKAGE_NAME}_REL_SOURCE_DIR and use that in various places instead. But first I need to see if you really need a list like ${PROJECT_NAME}_PACKAGE_DIRS to search ... So I can't find any places where one would want to look up a package given it relative directory. But there are places where we do just want the relative package directory. For example: ``` ./ci_support/TribitsDumpXmlDependenciesFiles.cmake:109: LIST(GET ${PROJECT_NAME}_SE_PACKAGE_DIRS ${PACKAGE_IDX} PACKAGE_DIR) ``` Therefore, I think it makes sense to create the variable ${PACKAGE_NAME}_REL_SOURCE_DIR and use that instead of ${PROJECT_NAME}_PACKAGE_DIRS. **(2020/01/16)** I have done the refactor to get rid of ${PROJECT_NAME}_PACKAGE_DIRS and ${PROJECT_NAME}_SE_PACKAGE_DIRS by introducing ${PACKAGE_NAME}_REL_SOURCE_DIR. That should pave the way to creating a single set of data-structures combining TPLs and Packages together. I cleaned up the commits so far and pushed the updated branch. Before I move forward, I want to test Trilinos with what I have so far to make sure I have not broken anything badly. For that, I will just run one of the ATDM Trilinos builds on cee-rhel6. For the repo versions and state: ``` $ gitdist-repo-versions *** Base Git Repo: Trilinos a093268bcc [Thu Jan 16 08:15:31 2020 -0700] Merge Pull Request #6591 from trilinos/Trilinos/tpetra_removedUnusedField *** Git Repo: TriBITS 7ffb872e [Thu Jan 16 10:11:15 2020 -0700] WIP: Update doc about data-structures (#63) $ gitdist-status --------------------------------------------------------------------------------------------- | ID | Repo Dir | Branch | Tracking Branch | C | M | ? | |----|-----------------|------------------------|-------------------------------|---|---|---| | 0 | Trilinos (Base) | develop | github/develop | | | | | 1 | TriBITS | 63-merge-tpls-packages | github/63-merge-tpls-packages | | | | --------------------------------------------------------------------------------------------- ``` on 'ews00232' I ran: ``` $ cd /scratch/rabartl/Trilinos.base/BUILDS/ATDM/CEE-RHE6/CHECKIN/ $ env ATDM_CHT_DEFAULT_ENV=cee-rhel6-default \ /scratch/rabartl/Trilinos.base/Trilinos/cmake/std/atdm/checkin-test-atdm.sh "$@" $ ./checkin-test-atdm-cee-rhel6.sh \ gnu-shared-opt \ --enable-all-packages=on --configure $ cd gnu-shared-opt/ $ . load-env.sh Hostname 'ews00232' matches known ATDM host 'sems-rhel7' and system 'sems-rhel7' Setting compiler and build options for build-name 'gnu-shared-opt' Using SEMS RHEL7 compiler stack GNU-7.2.0 to build RELEASE code with Kokkos node type SERIAL $ rm -r CMake* $ rm ctest.out ; time ./do-configure -DTrilinos_TRIBITS_DIR:STRING=TriBITS/tribits &> configure.out && time ninja -j8 &> make.out && time ctest -j8 &> ctest.out ; grep -A 100 "failed out of" ctest.out ; ~/mailmsg.py "Finished Trilinso testting of 63-merge-tpls-packages" ??? ``` Wow, that just gave a ton of OpenMPI startup test errors. That can't be related to the TriBITS refactorings. Now to get on with the refactoring ... The next thing to do is to create a new list var called ${PROJECT_NAME}_ALL_PACKAGES. It looks like that variable is not being used: ``` $ find . -name "*.cmake" -exec grep -nH _ALL_PACKAGE {} \; | grep -v _ENABLE_ALL_PACKAGES ./core/package_arch/TribitsGlobalMacros.cmake:1636: TRIBITS_READ_ALL_PACKAGE_DEPENDENCIES() ./core/package_arch/TribitsAdjustPackageEnables.cmake:771:MACRO(TRIBITS_READ_ALL_PACKAGE_SUBPACKAGE_DEPENDENCIES PACKAGE_NAME) ./core/package_arch/TribitsAdjustPackageEnables.cmake:773: #MESSAGE("TRIBITS_READ_ALL_PACKAGE_SUBPACKAGE_DEPENDENCIES: ${PACKAGE_NAME}") ./core/package_arch/TribitsAdjustPackageEnables.cmake:883: TRIBITS_READ_ALL_PACKAGE_SUBPACKAGE_DEPENDENCIES(${PACKAGE_NAME}) ./core/package_arch/TribitsAdjustPackageEnables.cmake:934:# @MACRO: TRIBITS_READ_ALL_PACKAGE_DEPENDENCIES() ./core/package_arch/TribitsAdjustPackageEnables.cmake:938:# TRIBITS_READ_ALL_PACKAGE_DEPENDENCIES() ./core/package_arch/TribitsAdjustPackageEnables.cmake:958:MACRO(TRIBITS_READ_ALL_PACKAGE_DEPENDENCIES) ./core/package_arch/TribitsAdjustPackageEnables.cmake:1786:MACRO(TRIBITS_SET_ALL_PACKAGES_PACKAGE_ENABLE_VARIABLE PACKAGE_ARCH_VAR PACKAGE_VAR) ./core/package_arch/TribitsAdjustPackageEnables.cmake:1790: MESSAGE("TRIBITS_SET_ALL_PACKAGES_PACKAGE_ENABLE_VARIABLE:") ./core/package_arch/TribitsAdjustPackageEnables.cmake:1831:MACRO(TRIBITS_APPLY_ALL_PACKAGE_ENABLES PACKAGE_NAME) ./core/package_arch/TribitsAdjustPackageEnables.cmake:1836: TRIBITS_SET_ALL_PACKAGES_PACKAGE_ENABLE_VARIABLE( ./core/package_arch/TribitsAdjustPackageEnables.cmake:1851: TRIBITS_SET_ALL_PACKAGES_PACKAGE_ENABLE_VARIABLE( ./core/package_arch/TribitsAdjustPackageEnables.cmake:1853: TRIBITS_SET_ALL_PACKAGES_PACKAGE_ENABLE_VARIABLE( ./core/package_arch/TribitsAdjustPackageEnables.cmake:2361: TRIBITS_APPLY_ALL_PACKAGE_ENABLES(${TRIBITS_PACKAGE}) ``` This will be a simple concatenation of ${PROJECT_NAME}_TPLS and ${PROJECT_NAME}_PACKAGES. I will add whatever other variables needed to fill out this data-structure so that I can replace the direct usage of the variables ${PROJECT_NAME}_TPLS and ${PROJECT_NAME}_PACKAGES. A few things that come to mind include * The variable ${PROJECT_NAME}_ALL_PACKAGES_FIRST_INTERNAL_PKG_IDX will need to be added wich will be the index in ${PROJECT_NAME}_ALL_PACKAGES of the first internal TriBITS package that will be built. That menas that everything up to ${PROJECT_NAME}_ALL_PACKAGES_FIRST_INTERNAL_PKG_IDX will be considered to be a TPL that can be enabled and will be looked for on the system. * The varaible ${PROJECT_NAME}_ENABLE_${PACKAGE_NAME} will need to be used to enable/disable both internal packages and external (TPL) packages. But we will need to be careful to preserve the current enable/disable logic for TPLs vs. packages when it comes to optional TPLs (i.e. they don't get enabled by default). That means that we will need to allow the user to set _ENABLE_=[ON|OFF] and that will need to mean the same thing as TPL_ENABLE_=[ON|OFF]. That is okay but we will need to be careful in the execution of enable/disable logic. But internally, ${PROJECT_NAME}_ENABLE_${PACKAGE_NAME} and not TPL_ENABLE_${PACKAGE_NAME} must be used to determine if a TPL is enabled or not. I will need to docuemnt this carefully. So lets get to it ... **(2020/01/17)** Wow, I am making painfully slow progress on this. I think I just need to spend more time reading over and documenting existing code before I dive into any more refactorings. I really need to understand what data gets created and used and then I can determine how I can do the refactorings ... **(2020/02/07)** Testing against Trilinos on 'crf450' with ATDM Trilinos 'sems-rhel6'. The repo vesions: ``` $ gitdist-status --dist-repos=.,TriBITS --------------------------------------------------------------------------------------------- | ID | Repo Dir | Branch | Tracking Branch | C | M | ? | |----|-----------------|------------------------|-------------------------------|---|---|---| | 0 | Trilinos (Base) | develop | github/develop | | | | | 1 | TriBITS | 63-merge-tpls-packages | github/63-merge-tpls-packages | 3 | | | --------------------------------------------------------------------------------------------- $ gitdist-repo-versions --dist-repos=.,TriBITS *** Base Git Repo: Trilinos d25fb5e [Thu Feb 6 07:16:47 2020 -0700] Merge Pull Request #6762 from bartlettroscoe/Trilinos/atdm-kokkos-promotion-fixes-2020-02-04 *** Git Repo: TriBITS 28c8755 [Thu Feb 6 21:59:40 2020 -0700] WIP: Fix spelling TRIBITS_READ_SUBPACKAGE_DEPENDENCIES_AND_ADD_TO_GRAPH() (#63) ``` I tested this on 'crf450' with: ``` $ cd ??? $ ./checkin-test-atdm.sh gnu-openmp-opt --enable-all-packages=on --configure $ cd gnu-openmp-opt $ . load-env.sh $ rm -r CMake* configure.* make.* ctest.* $ time ./do-configure -DTrilinos_TRIBITS_DIR:STRING=TriBITS/tribits &> configure.out \ && time ninja &> make.out \ && time ctest -j16 &> ctest.out real 0m51.322s user 0m38.979s sys 0m9.329s real 26m55.814s user 748m30.681s sys 71m49.038s real 3m9.244s user 53m26.830s sys 4m26.540s ``` which returned: ``` $ grep -A 100 "failed out of" ctest.out 100% tests passed, 0 tests failed out of 1951 Subproject Time Summary: Amesos2 = 2.09 sec*proc (6 tests) Belos = 67.71 sec*proc (99 tests) Ifpack2 = 17.15 sec*proc (44 tests) Intrepid2 = 79.69 sec*proc (267 tests) Kokkos = 128.20 sec*proc (26 tests) KokkosKernels = 163.39 sec*proc (8 tests) MueLu = 132.75 sec*proc (63 tests) NOX = 76.21 sec*proc (105 tests) Panzer = 1743.00 sec*proc (171 tests) Phalanx = 4.29 sec*proc (31 tests) Rythmos = 17.88 sec*proc (83 tests) SEACAS = 7.96 sec*proc (22 tests) STK = 0.87 sec*proc (3 tests) Sacado = 32.99 sec*proc (299 tests) Stratimikos = 11.15 sec*proc (39 tests) Teko = 23.87 sec*proc (18 tests) Tempus = 157.26 sec*proc (96 tests) Teuchos = 52.37 sec*proc (139 tests) Thyra = 20.80 sec*proc (82 tests) Tpetra = 91.43 sec*proc (221 tests) Xpetra = 8.28 sec*proc (15 tests) Zoltan2 = 107.75 sec*proc (114 tests) Total Test time (real) = 189.12 sec ``` **(2021/05/19)** I am finally getting back to this after over a year!!!!! I am going to focus on merging this branch with what is on 'master' and make sure that I don't loose anything before moving forward. Let me get a sense of where we are. What are all of the files that have changed on this branch '63-merge-tpls-packages' compared to 'master'? ``` $ git diff --name-status github/63-merge-tpls-packages \ --not $(git merge-base github/63-merge-tpls-packages github/master) M test/ci_support/CMakeLists.txt M test/ci_support/DumpXmlDepsTests/CMakeLists.txt M test/core/CMakeLists.txt M test/core/ExamplesUnitTests/CMakeLists.txt A test/core/MiniMockTrilinosFiles/PackagesList.cmake A test/core/MiniMockTrilinosFiles/TPLsList.cmake M test/core/TribitsAdjustPackageEnablesHelpers.cmake M test/core/TribitsAdjustPackageEnables_UnitTests.cmake M test/core/TribitsProcessPackagesAndDirsLists_UnitTests.cmake M test/core/TribitsWriteClientExportFiles_UnitTests.cmake M tribits/ci_support/TribitsDumpXmlDependenciesFiles.cmake M tribits/core/package_arch/TribitsAdjustPackageEnables.cmake M tribits/core/package_arch/TribitsGlobalMacros.cmake M tribits/core/package_arch/TribitsProcessPackagesAndDirsLists.cmake M tribits/core/package_arch/TribitsProcessTplsLists.cmake A tribits/core/package_arch/TribitsReadPackagesProcessDepenenciesWriteXml.cmake A tribits/core/package_arch/TribitsSystemDataStructures.rst D tribits/doc/README.DEPENDENCIES M tribits/doc/developers_guide/.gitignore M tribits/doc/developers_guide/TribitsDevelopersGuide.rst M tribits/doc/developers_guide/TribitsMacroFunctionDocTemplate.rst A tribits/doc/developers_guide/TribitsSystemMacroFunctionDocTemplate.rst M tribits/doc/developers_guide/UtilsMacroFunctionDocTemplate.rst M tribits/doc/developers_guide/generate-dev-guide.sh M tribits/examples/RawHelloWorld/CMakeLists.txt M tribits/examples/RawHelloWorld/hello_world/CMakeLists.txt M tribits/examples/TribitsExampleProject/packages/with_subpackages/CMakeLists.txt A tribits/examples/TribitsExampleProject/packages/with_subpackages/b/ExcludeFromRelease.txt A tribits/examples/TribitsExampleProject/packages/with_subpackages/b/src/AlsoExcludeFromTarball.txt M tribits/examples/TribitsHelloWorld/CMakeLists.txt M tribits/examples/TribitsHelloWorld/hello_world/CMakeLists.txt ``` What are the changes to the tests? ``` $ git log-short --name-status github/63-merge-tpls-packages \ --not $(git merge-base github/63-merge-tpls-packages github/master) \ -- test/ 996fcb1 "Fix name of TribitsReadPackagesProcessDepenenciesWriteXml.cmake (#63)" Author: Roscoe A. Bartlett Date: Wed May 19 16:02:52 2021 -0600 (8 minutes ago) M test/core/TribitsAdjustPackageEnablesHelpers.cmake a2efb7e "WIP: Clean up TRIBITS_READ_PACKAGE_DEPENDENCIES() some (#63)" Author: Roscoe A. Bartlett Date: Sat Jan 18 16:44:17 2020 -0700 (1 year, 4 months ago) M test/core/CMakeLists.txt M test/core/TribitsAdjustPackageEnables_UnitTests.cmake bd990c6 "WIP: Change name to TRIBITS_READ_PROJECT_AND_PACKAGE_DEPENDENCIES_CREATE_GRAPH_PRINT_DEPS() (#63)" Author: Roscoe A. Bartlett Date: Sat Jan 18 10:44:32 2020 -0700 (1 year, 4 months ago) M test/core/TribitsAdjustPackageEnablesHelpers.cmake M test/core/TribitsAdjustPackageEnables_UnitTests.cmake 23045be "Add new 'DEFINED' internal and external package list vars (#63)" Author: Roscoe A. Bartlett Date: Sat Jan 18 08:36:16 2020 -0700 (1 year, 4 months ago) M test/core/CMakeLists.txt M test/core/TribitsAdjustPackageEnables_UnitTests.cmake 40f593b "WIP: Change name to TRIBITS_READ_PACKAGE_DEPENDENCIES_AND_CREATE_GRAPH(), update docs (#63)" Author: Roscoe A. Bartlett Date: Sat Jan 18 08:17:56 2020 -0700 (1 year, 4 months ago) M test/core/TribitsAdjustPackageEnablesHelpers.cmake M test/core/TribitsAdjustPackageEnables_UnitTests.cmake c8a81fa "Refactored tests to be able to use TRIBITS_READ_PACKAGES_PROCESS_DEPENDENCIES_WRITE_XML() (#63)" Author: Roscoe A. Bartlett Date: Sat Jan 18 04:24:35 2020 -0700 (1 year, 4 months ago) M test/core/CMakeLists.txt A test/core/MiniMockTrilinosFiles/PackagesList.cmake A test/core/MiniMockTrilinosFiles/TPLsList.cmake M test/core/TribitsAdjustPackageEnablesHelpers.cmake M test/core/TribitsAdjustPackageEnables_UnitTests.cmake 1ac716a "WIP: Commented tests for new global vars (#63)" Author: Roscoe A. Bartlett Date: Thu Jan 16 17:20:18 2020 -0700 (1 year, 4 months ago) M test/core/CMakeLists.txt M test/core/TribitsAdjustPackageEnables_UnitTests.cmake a300c00 "Eliminae ${PROJECT_NAME}_[SE_]PACKAGE_DIRS and replace with ${PACKAGE_NAME}_REL_SOURCE_DIR (#63)" Author: Roscoe A. Bartlett Date: Wed Jan 15 12:37:02 2020 -0700 (1 year, 4 months ago) M test/core/CMakeLists.txt M test/core/ExamplesUnitTests/CMakeLists.txt M test/core/TribitsAdjustPackageEnablesHelpers.cmake M test/core/TribitsAdjustPackageEnables_UnitTests.cmake M test/core/TribitsProcessPackagesAndDirsLists_UnitTests.cmake M test/core/TribitsWriteClientExportFiles_UnitTests.cmake 9e0d1b2 "Add test for TRIBITS_EXCLUDE_FILES() and remove usage of _PACKAGE_DIRS (#63)" Author: Roscoe A. Bartlett Date: Wed Jan 15 09:41:44 2020 -0700 (1 year, 4 months ago) M test/core/ExamplesUnitTests/CMakeLists.txt 60e4cf8 "Repalce cmake -E copy_directory with cp -r" Author: Roscoe A. Bartlett Date: Wed Jan 15 12:30:21 2020 -0700 (1 year, 4 months ago) M test/ci_support/CMakeLists.txt M test/ci_support/DumpXmlDepsTests/CMakeLists.txt ``` I think I should be able to pretty safely merge changes in those test/ directories. The rest of the code changes are focused on files under tribits/core/package_arch/. The only other code change was: ``` $ git log-short --name-status github/63-merge-tpls-packages \ --not $(git merge-base github/63-merge-tpls-packages github/master) \ -- tribits/ci_support/ a300c00 "Eliminae ${PROJECT_NAME}_[SE_]PACKAGE_DIRS and replace with ${PACKAGE_NAME}_REL_SOURCE_DIR (#63)" Author: Roscoe A. Bartlett Date: Wed Jan 15 12:37:02 2020 -0700 (1 year, 4 months ago) M tribits/ci_support/TribitsDumpXmlDependenciesFiles.cmake [rabartl@crf450 TriBITS (63-merge-tpls-packages)]$ ``` and that is a very minor patch. So I think we can focus on the changes to files under the tribits/core/ directory. What modified files are those? ``` $ git diff --name-status github/63-merge-tpls-packages \ --not $(git merge-base github/63-merge-tpls-packages github/master) \ -- tribits/core/ | grep "^M" M tribits/core/package_arch/TribitsAdjustPackageEnables.cmake M tribits/core/package_arch/TribitsGlobalMacros.cmake M tribits/core/package_arch/TribitsProcessPackagesAndDirsLists.cmake M tribits/core/package_arch/TribitsProcessTplsLists.cmake ``` So let's look at what changes have been made to those files on the 'master' branch since this branch was first created? ``` $ git log-oneline github/master --not $(git merge-base github/master github/63-merge-tpls-packages) -- tribits/core/package_arch/TribitsAdjustPackageEnables.cmake tribits/core/package_arch/TribitsGlobalMacros.cmake tribits/core/package_arch/TribitsProcessPackagesAndDirsLists.cmake tribits/core/package_arch/TribitsProcessTplsLists.cmake 07158ad "Remove conditional logic for cmake versions less than 3.17 (#299)" [Fri Mar 12 15:59:51 2021 -0700] (10 weeks ago) 092ff4f "Fix install_package_by_package behavior for new CMP0082 behavior with CMake 3.17 upgrade (#299)" [Fri Mar 12 14:00:54 2021 -0700] (10 weeks ago) 2418f31 "Change the default for _ENABLE_EXPLICIT_INSTANTIATION to ON" [Thu Nov 12 06:54:50 2020 -0700] (6 months ago) 2dcd09d "Fix enable of any subpackage triggers _ENABLE_TESTS=ON (ATDV-399)" [Mon Sep 28 10:46:10 2020 -0600] (7 months ago) 5b9c840 "Add ctest var initialization for CUDA GPU allocation (trilinos/Trilinos#2422)" [Fri May 15 11:18:36 2020 -0600] (1 year ago) 31c025a "Auto-generate resource spec file (trilinos/Trilinos#2422)" [Tue Apr 7 16:55:13 2020 -0400] (1 year ago) 7579e18 "Merge remote-tracking branch 'bradking/cxx-std' (ATDV-303)" [Tue May 12 17:31:50 2020 -0600] (1 year ago) 9235eef "Add options _SKIP_CTEST_ADD_TEST and _INNER_ENABLE_TEST (#317)" [Thu May 7 14:41:03 2020 -0600] (1 year ago) 889fa64 "Implement recursive chgrp and chmod for installed files/directories (#314, ATDV-241)" [Thu Apr 23 17:42:54 2020 -0600] (1 year, 1 month ago) 6b082ca "Propagate the C++ standard to dependents via CMake features" [Thu Feb 6 16:38:19 2020 -0500] (1 year, 1 month ago) ebcea2d "Enable C++11 support via CMake standard settings" [Thu Feb 6 15:23:54 2020 -0500] (1 year, 1 month ago) 8b16479 "Remove option ${PROJECT_NAME}_ENABLE_CXX11" [Thu Feb 6 11:05:01 2020 -0500] (1 year, 1 month ago) ``` So I think there are a few commits on 'master' that may need to be manually applied to the '63-merge-tpls-packages' branch from looking over these commits in detail. These are the commits: ``` 2dcd09d "Fix enable of any subpackage triggers _ENABLE_TESTS=ON (ATDV-399)" Author: Roscoe A. Bartlett Date: Mon Sep 28 10:46:10 2020 -0600 (7 months ago) M tribits/core/package_arch/TribitsAdjustPackageEnables.cmake 9235eef "Add options _SKIP_CTEST_ADD_TEST and _INNER_ENABLE_TEST (#317)" Author: Roscoe A. Bartlett Date: Thu May 7 14:41:03 2020 -0600 (1 year ago) M tribits/core/package_arch/TribitsAdjustPackageEnables.cmake M tribits/core/package_arch/TribitsGlobalMacros.cmake ``` It looks like only the commit 2dcd09d "Fix enable of any subpackage triggers _ENABLE_TESTS=ON (ATDV-399)" is non-trivial. So it looks like the merge between these two branches, amazingly, should not be too bad. So I can work a little longer on this branch and then merge it into master when it is in little better shape. **(2021/05/24)** Okay, I have done as much as I want to do on this branch '63-merge-tpls-packages-1' (and in fact, I likely did too much). Now I need merge in 'master', run the entire TriBITS test suite (with the checkin-test.py script) and make sure everything passes. I tried the simple merge and tried to resolve some conflicts and I am getting a configure error: ``` CMake Error at tribits/core/package_arch/TribitsProjectImpl.cmake:321 (TRIBITS_SETUP_FOR_INSTALLATION): Unknown CMake command "TRIBITS_SETUP_FOR_INSTALLATION". Call Stack (most recent call first): tribits/core/package_arch/TribitsProjectImpl.cmake:364 (TRIBITS_PROJECT_IMPL) CMakeLists.txt:17 (TRIBITS_PROJECT_ENABLE_ALL) ``` So what is up with that? How did that function change on 'master' and on '63-merge-tpls-packages-1'? Turns out that there was a new copy of the macro TRIBITS_SETUP_FOR_INSTALLATION() created on 'master' and I did a terrible job of resolving the conflicts ... So I think I have that fixed. I just appended to the merge commit. Now to make sure the patches in the two commits 2dcd09d and 9235eef are there correctly. First the commit 2dcd09d includes in the file `TribitsAdjustPackageEnables.cmake` the new macro: ``` MACRO(TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_ENABLES PACKAGE_NAME) ... + TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_TEST_EXAMPLE_ENABLES( + ${PACKAGE_NAME} TESTS) + TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_TEST_EXAMPLE_ENABLES( + ${PACKAGE_NAME} EXAMPLES) ... +MACRO(TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_TEST_EXAMPLE_ENABLES + PACKAGE_NAME TESTS_OR_EXAMPLES + ) ... +ENDMACRO() ``` we can see this in the current file: ``` $ grep -n TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_TEST_EXAMPLE_ENABLES tribits/core/package_arch/TribitsAdjustPackageEnables.cmake 1915: TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_TEST_EXAMPLE_ENABLES( 1917: TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_TEST_EXAMPLE_ENABLES( 1931:MACRO(TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_TEST_EXAMPLE_ENABLES 1934: #MESSAGE("TRIBITS_POSTPROCESS_PACKAGE_WITH_SUBPACKAGES_TEST_EXAMPLE_ENABLES '${PACKAGE_NAME}'") ``` So that looks good. Now for the commit: ``` $ git log-short --name-status -1 9235eef 9235eef "Add options _SKIP_CTEST_ADD_TEST and _INNER_ENABLE_TEST (#317)" Author: Roscoe A. Bartlett Date: Thu May 7 14:41:03 2020 -0600 (1 year, 1 month ago) M test/core/CMakeLists.txt M test/core/ExamplesUnitTests/CMakeLists.txt M test/core/TestingFunctionMacro_UnitTests.cmake M test/ctest_driver/TribitsExampleProject/CMakeLists.txt M tribits/core/package_arch/TribitsAddAdvancedTest.cmake M tribits/core/package_arch/TribitsAddTest.cmake M tribits/core/package_arch/TribitsAddTestHelpers.cmake M tribits/core/package_arch/TribitsAdjustPackageEnables.cmake M tribits/core/package_arch/TribitsGlobalMacros.cmake M tribits/ctest_driver/TribitsCTestDriverCore.cmake M tribits/ctest_driver/TribitsCTestDriverCoreHelpers.cmake M tribits/doc/build_ref/TribitsBuildReferenceBody.rst ``` The files that changed under tribits/core are: ``` M tribits/core/package_arch/TribitsAddAdvancedTest.cmake M tribits/core/package_arch/TribitsAddTest.cmake M tribits/core/package_arch/TribitsAddTestHelpers.cmake M tribits/core/package_arch/TribitsAdjustPackageEnables.cmake M tribits/core/package_arch/TribitsGlobalMacros.cmake ``` Let's diff the file test-related files compared to what is on 'master': ``` $ git diff HEAD --not github/master -- TribitsAddAdvancedTest.cmake TribitsAddTest.cmake TribitsAddTestHelpers.cmake [empty] ``` Okay, so no difference. Now, what about the changes in the files: ``` M tribits/core/package_arch/TribitsAdjustPackageEnables.cmake M tribits/core/package_arch/TribitsGlobalMacros.cmake ``` ? Looking at those patches: ``` $ git log-short -p -1 9235eef -- TribitsAdjustPackageEnables.cmake TribitsGlobalMacros.cmake diff --git a/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake b/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake index d56858a..6c46da6 100644 --- a/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake +++ b/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake @@ -1533,6 +1533,13 @@ MACRO(TRIBITS_ADD_OPTIONAL_PACKAGE_ENABLES PACKAGE_NAME) ) SET_CACHE_ON_OFF_EMPTY( ${PACKAGE_NAME}_ENABLE_EXAMPLES "" ${DOCSTR} ) + MULTILINE_SET(DOCSTR + "Build examples for the package ${PACKAGE_NAME}. Set to 'ON', 'OFF', or leave empty ''" + " to allow for other logic to decide." + ) + SET( ${PACKAGE_NAME}_SKIP_CTEST_ADD_TEST + "${${PROJECT_NAME}_SKIP_CTEST_ADD_TEST}" CACHE BOOL ${DOCSTR} ) + ELSE() IF (NOT DEFINED ${PACKAGE_NAME}_ENABLE_TESTS) diff --git a/tribits/core/package_arch/TribitsGlobalMacros.cmake b/tribits/core/package_arch/TribitsGlobalMacros.cmake index 9fdaf01..2c06a17 100644 --- a/tribits/core/package_arch/TribitsGlobalMacros.cmake +++ b/tribits/core/package_arch/TribitsGlobalMacros.cmake @@ -254,10 +254,13 @@ MACRO(TRIBITS_DEFINE_GLOBAL_OPTIONS_AND_DEFINE_EXTRA_REPOS) "Disable (and printing warning) for enabled packages that have hard-disabled upstream dependencies. Otherwise, is to raises a fatal configure failure." ) SET_CACHE_ON_OFF_EMPTY( ${PROJECT_NAME}_ENABLE_TESTS "" - "Enable tests in all packages (set to ON, OFF, or leave empty)." ) + "Enable tests (execs and ctest add_test()) in all packages (set to ON, OFF, or leave empty)." ) SET_CACHE_ON_OFF_EMPTY(${PROJECT_NAME}_ENABLE_EXAMPLES "" - "Enable examples in all packages (set to ON, OFF, or leave empty). If left empty, then this will be set to ON if ${PROJECT_NAME}_ENABLE_TESTS=ON" ) + "Enable examples (exec and ctest add_test()) in all packages (set to ON, OFF, or leave empty). If left empty, then this will be set to ON if ${PROJECT_NAME}_ENABLE_TESTS=ON" ) + + SET(${PROJECT_NAME}_SKIP_CTEST_ADD_TEST OFF CACHE BOOL + "Skipp ctest add_test() for all defined tests (but still build any enabled test or example executable targets)." ) IF (${PROJECT_NAME}_ENABLE_TESTS AND ${PROJECT_NAME}_ENABLE_EXAMPLES STREQUAL "") MESSAGE(STATUS "Setting ${PROJECT_NAME}_ENABLE_EXAMPLES=ON because ${PROJECT_NAME}_ENABLE_TESTS=ON") ``` Let's search for 'SKIP_CTEST_ADD_TEST' in the current source code and see what comes up: ``` $ find tribits test -type f -exec grep -l SKIP_CTEST_ADD_TEST {} \; tribits/core/package_arch/TribitsGlobalMacros.cmake tribits/core/package_arch/TribitsAddTest.cmake tribits/core/package_arch/TribitsAddTestHelpers.cmake tribits/core/package_arch/TribitsAddAdvancedTest.cmake tribits/core/package_arch/TribitsAdjustPackageEnables.cmake tribits/ctest_driver/TribitsCTestDriverCore.cmake tribits/ctest_driver/TribitsCTestDriverCoreHelpers.cmake tribits/doc/build_ref/TribitsBuildReference.html tribits/doc/build_ref/TribitsBuildReference.rst tribits/doc/build_ref/TribitsBuildReferenceBody.rst test/core/ExamplesUnitTests/CMakeLists.txt test/core/TestingFunctionMacro_UnitTests.cmake test/ctest_driver/TribitsExampleProject/CMakeLists.txt ``` Okay, I am pretty convinced that this came over okay. Just to be sure, I will disable support for this option and make sure that test fail. I commented out a line of code in TribitsAdjustPackageEnables.cmake responding to `${PACKAGE_NAME}_SKIP_CTEST_ADD_TEST` and I got the failing tests: ``` The following tests FAILED: 201 - TriBITS_TribitsExampleProject_SKIP_CTEST_ADD_TEST_Project (Failed) 203 - TriBITS_TribitsExampleProject_SKIP_CTEST_ADD_TEST_Package_Blacklist (Failed) 346 - TriBITS_CTestDriver_AAOP_ST_SKIP_CTEST_ADD_TEST (Failed) ``` so I think this is good. Now to test against Trilinos 'develop'. The 'cee-rhel7' clang-9.0.l build looks pretty good today. Let's try that. Testing the versions: ``` $ gitdist log-short -1 *** Base Git Repo: Trilinos 1d14031 "Merge pull request #9163 from hkthorn/develop" Author: Heidi Thornquist Date: Mon May 24 09:18:01 2021 -0600 (7 hours ago) *** Git Repo: TriBITS a683dc7 "Fix group on installation test case" Author: Roscoe A. Bartlett Date: Mon May 24 15:07:20 2021 -0600 (43 minutes ago) ``` on 'ceerws1113' with: ``` $ cd /scratch/rabartl/Trilinos.base/BUILDS/ATDM/CEE-RHEL7/CHECKIN/cee-rhel7-clang-openmp-shared-opt/ $ cat load-env.sh source /scratch/rabartl/Trilinos.base/Trilinos/cmake/std/atdm/load-env.sh cee-rhel7-clang-openmp-shared-opt $ . load-env.sh Hostname 'ceerws1113' matches known ATDM host 'ceerws1113' and system 'cee-rhel7' Setting compiler and build options for build-name 'cee-rhel7-clang-openmp-shared-opt' Using CEE RHEL7 compiler stack CLANG-9.0.1_OPENMPI-4.0.3 to build RELEASE code with Kokkos node type OPENMP $ cat do-configure.base cmake \ -GNinja \ -DTrilinos_TRIBITS_DIR:STRING=TriBITS/tribits \ -DTrilinos_ENABLE_TESTS:BOOL=ON \ -DTrilinos_TEST_CATEGORIES:STRING=NIGHTLY \ -DTrilinos_ALLOW_NO_PACKAGES:BOOL=OFF \ -DDART_TESTING_TIMEOUT:STRING=600.0 \ -GNinja \ -DTrilinos_CONFIGURE_OPTIONS_FILE:STRING=cmake/std/atdm/ATDMDevEnv.cmake \ -DTrilinos_TRACE_ADD_TEST=ON \ "$@" \ /scratch/rabartl/Trilinos.base/Trilinos/cmake/std/atdm/../../.. $ cat do-configure ./do-configure.base \ -DTrilinos_ENABLE_ALL_OPTIONAL_PACKAGES:BOOL=ON \ -DTrilinos_ENABLE_ALL_PACKAGES:BOOL=ON \ -DTrilinos_ENABLE_ALL_FORWARD_DEP_PACKAGES:BOOL=OFF \ "$@" $ time ./do-configure &> configure.out real 0m5.463s user 0m4.530s sys 0m4.335s ``` Shoot, I got a configure error: ``` -- Could NOT find OpenMP (missing: OpenMP_C_FOUND OpenMP_CXX_FOUND) (found version "4.5") CMake Error at TriBITS/tribits/core/package_arch/TribitsGlobalMacros.cmake:1870 (MESSAGE): Could not find OpenMP, try setting OpenMP_C_FLAGS and OpenMP_CXX_FLAGS directly Call Stack (most recent call first): TriBITS/tribits/core/package_arch/TribitsProjectImpl.cmake:193 (TRIBITS_SETUP_ENV) TriBITS/tribits/core/package_arch/TribitsProject.cmake:93 (TRIBITS_PROJECT_IMPL) CMakeLists.txt:104 (TRIBITS_PROJECT) ``` I need to go back and see if I get a configure error with just Trilinos 'develop' with its own version of TriBITS ... Yea, that does not work either. So openmp does now work with clang. I will just use plain clang then ... ``` $ cd /scratch/rabartl/Trilinos.base/BUILDS/ATDM/CEE-RHEL7/CHECKIN/cee-rhel7-clang-shared-opt-tribits/ $ cat load-env.sh source /scratch/rabartl/Trilinos.base/Trilinos/cmake/std/atdm/load-env.sh cee-rhel7-clang-shared-opt $ cat do-configure.base cmake \ -GNinja \ -DTrilinos_TRIBITS_DIR:STRING=TriBITS/tribits \ -DTrilinos_ENABLE_TESTS:BOOL=ON \ -DTrilinos_TEST_CATEGORIES:STRING=NIGHTLY \ -DTrilinos_ALLOW_NO_PACKAGES:BOOL=OFF \ -DDART_TESTING_TIMEOUT:STRING=600.0 \ -GNinja \ -DTrilinos_CONFIGURE_OPTIONS_FILE:STRING=cmake/std/atdm/ATDMDevEnv.cmake \ -DTrilinos_TRACE_ADD_TEST=ON \ "$@" \ /scratch/rabartl/Trilinos.base/Trilinos/cmake/std/atdm/../../.. $ cat do-configure ./do-configure.base \ -DTrilinos_ENABLE_ALL_OPTIONAL_PACKAGES:BOOL=ON \ -DTrilinos_ENABLE_ALL_PACKAGES:BOOL=ON \ -DTrilinos_ENABLE_ALL_FORWARD_DEP_PACKAGES:BOOL=OFF \ "$@" $ . load-env.sh Hostname 'ceerws1113' matches known ATDM host 'ceerws1113' and system 'cee-rhel7' Setting compiler and build options for build-name 'cee-rhel7-clang-shared-opt' Using CEE RHEL7 compiler stack CLANG-9.0.1_OPENMPI-4.0.3 to build RELEASE code with Kokkos node type SERIAL $ time ./do-configure &> configure.out real 1m55.233s user 1m9.080s sys 0m30.450s $ grep TRIBITS_DIR configure.out -- Trilinos_TRIBITS_DIR='/scratch/rabartl/Trilinos.base/Trilinos/TriBITS/tribits' $ time ninja -j16 &> make.out real 74m7.465s user 1104m2.125s sys 31m0.733s $ ctest -j16 &> ctest.out $ grep -A 50 "failed out of" ctest.out 100% tests passed, 0 tests failed out of 2399 Subproject Time Summary: Adelus = 11.75 sec*proc (4 tests) Amesos2 = 9.66 sec*proc (8 tests) Belos = 246.90 sec*proc (137 tests) Ifpack2 = 72.53 sec*proc (46 tests) Intrepid2 = 735.87 sec*proc (163 tests) Kokkos = 398.63 sec*proc (32 tests) KokkosKernels = 371.94 sec*proc (14 tests) MueLu = 753.47 sec*proc (97 tests) NOX = 754.74 sec*proc (108 tests) Panzer = 2369.26 sec*proc (193 tests) Phalanx = 32.43 sec*proc (33 tests) ROL = 2914.24 sec*proc (207 tests) Rythmos = 330.33 sec*proc (83 tests) SEACAS = 384.78 sec*proc (63 tests) STK = 7.15 sec*proc (2 tests) Sacado = 105.84 sec*proc (299 tests) ShyLU_Node = 20.19 sec*proc (1 test) Stratimikos = 120.49 sec*proc (40 tests) Teko = 165.07 sec*proc (18 tests) Tempus = 3096.96 sec*proc (153 tests) Teuchos = 171.57 sec*proc (140 tests) Thyra = 72.34 sec*proc (82 tests) Tpetra = 324.50 sec*proc (260 tests) TrilinosATDMConfigTests = 73.64 sec*proc (12 tests) TrilinosBuildStats = 1.76 sec*proc (1 test) Xpetra = 19.83 sec*proc (18 tests) Zoltan2 = 364.24 sec*proc (185 tests) Total Test time (real) = 894.52 sec ``` So that passed!
bartlettroscoe commented 3 years ago

@keitat,

I posted https://github.com/trilinos/Trilinos/pull/9171 to merge the updated TriBITS 'master' to Trilinos 'develop' to get it tested to make sure this incremental refactoring does not break anything. Can you please approve that PR so it can merge?

I want to be merging these incremental refactorings as we go so we don't take any huge steps and then have huge debugging tasks in case something major breaks.

bartlettroscoe commented 3 years ago

I did another round of refactoring to further clean up the code that creates the package dependency graph in PR #374. I merged that to 'master' and created the Trilinos PR https://github.com/trilinos/Trilinos/pull/9178 to get this tested and merged there.

I think this is enough of #63 and now I need to do some of #299 to simplify how targets are managed before moving forward.

bartlettroscoe commented 1 year ago

We are finally getting to the finish line (at least for Use Case 3: Configure/build pointing to a subset of already installed TriBITS packages in same repo). A significant issue I realized is that the way that you are told to create <Package>Config.cmake files will create unusable installations of packages by Spack (see CMake Issue #24378). I will create a TriBITS GitHub issue shortly that addresses this with TriBITS. (But other CMake-based Spack packages will need to do something likewise.)