Implement support for pre-extra repositories

[bartlettroscoe]

The TriBITS projects SCALE/Exnihilo and MPACT need support for pre-extra repositories. Currently, they are pulling in upstream Trilinos packages using the NativeRepositoriesList.cmake file. That file is a hack and needs to disappear. Instead, projects should just use ExtraRepositoriesList.cmake. Also, pulling in TPLs built as packages (see #63) will require support for pre-extra repositories. This needs to be added to all parts of the TriBITS system that currently supports (post) extra repositories. This includes the basic CMake configure, the checkin-test.py script, and the code in TribitsCTestDriverCore.cmake.

The first decision that I have to make is how to handle pre-extra repositories. One approach is to develop completely separate data-structures and list for pre- and post-extra repositories. The other approach is to try to combine them in the ExtraRepositoriesList.cmake file and in other parts of the scripts. Note that the repository handling in checkin-test.py and TribitsCTestDriverCore.cmake does not care if a git repo is pre or post, it just does git pull, push, etc. It just needs to differentiate them once it passes them to the inner TriBITS CMake configure so that it can define and process the packages in the right order. This will be taken into consideration in the design of the user interfaces and the implementation.

[bartlettroscoe]

Okay, I think I have about all of the features for PRE extra repos that we need to support the majority of functionality in TriBITS (see detailed notes below). All of the changes are on a branch pre_extra_repos_73 in my Github repo. I will clean up and organize these commits before I push them to the master branch.

Now I am going to try to get MPACT using the ExtraRepositoriesList.cmake file and set up to use the checkin-test.py script. I will track that work in the PHI Kanban ticket 3708.

Once I am finished getting MPACT fixed up on a branch, I will:

• Update documentation for PRE extra repos
• Clean up the TriBITS commits
• Push TriBITS branch to the master branch in the casl-dev and github repos.

After that, we need to push the updated MPACT changes to the master MPACT branches.

---

Detailed Notes:

Here is my idea for the user interface design:

1. Add a new property to the PACKSTAT repo field of the function TRIBITS_PROJECT_DEFINE_EXTRA_REPOSITORIES(). So instead of empyt "" and "NOPACKAGES" you could write "PRE", "NOPACKAGES" or "PRE,NOPACKAGES" (note there is no space between the comma and the values). (NOTE: We need to support "PRE,NOPACKAGES" in the case where a git repo is cloned under the directory of a "PRE" repo with packages).

2. When configuring Trilinos from the commandline without an ExtraRepositoriesList.cmake file, the cache var <Project>_PRE_REPOSITORIES can be used, in addition to the existing cache var <Project>_EXTRA_REPOSITORIES. This allows tacking on pre-extra repos without needing an ExtraRepositoriesList.cmake file.

Internally, TriBITS currently stores the variables:

• ${PROJECT_NAME}_NATIVE_REPOSITORIES: Is '.' or defined from NavtiveRepositoriesList.cmake.
• ${PROJECT_NAME}_EXTRA_REPOSITORIES: This is defined from ExtraRepositoriesList.cmake or directly set from user as cache var.
• ${PROJECT_NAME}_EXTRA_REPOSITORIES_XXX: Properties of the extra repos in ${PROJECT_NAME}_EXTRA_REPOSITORIES where XXX is DIRS, REPOTYPES, REPOURLS, PACKSTATS, and CATEGORIES.
• ${PROJECT_NAME}_ALL_REPOSITORIES: Concatenation of ${PROJECT_NAME}_NATIVE_REPOSITORIES and ${PROJECT_NAME}_EXTRA_REPOSITORIES. (NOTE: this variable requires that all extra repos providing TriBITS packages must

The rules for how these variables work is described here and here.

So it looks like there is a lot of code in TriBITS that directly processes ${PROJECT_NAME}_EXTRA_REPOSITORIES:

$ cd TriBITS/tribits/

$ find . -name "*.cmake" -exec grep -nH EXTRA_REPOSITORIES {} \; | wc -l
157

$ find . -name "*.cmake" -exec grep -l EXTRA_REPOSITORIES {} \; 
./doc/developers_guide/ExtraReposList.cmake
./examples/MockTrilinos/cmake/ExtraRepositoriesList.cmake
./ci_support/TribitsGetExtraReposForCheckinTest.cmake
./ci_support/TribitsDumpDepsXmlScript.cmake
./ctest_driver/TribitsCTestDriverCore.cmake
./ctest_driver/TribitsAddDashboardTarget.cmake
./core/package_arch/TribitsProcessExtraRepositoriesList.cmake
./core/package_arch/TribitsGlobalMacros.cmake

Before I can decide how to implement this internally, I need to see exactly how ${PROJECT_NAME}_EXTRA_REPOSITORIES is used in all of those contexts.

To accommodate this implementation, I might consider adding the variables:

• ${PROJECT_NAME}_NATIVE_REPOSITORIES: Is either '.' or is determined by NativeRepositoriesList.cmake.
• ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES: Contains both pre and post extra repos.
• ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_PREPOST: 'PRE' or 'POST'

Then much of the code that currently loops through ${PROJECT_NAME}_EXTRA_REPOSITORIES would instead loop through ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES. And if you need to tell the difference between a pre-extra and a post-extra repo, you just check the value of ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_PREPOST (which lines up with it).

and we would change the names of the properties variables from ${PROJECT_NAME}_EXTRA_REPOSITORIES_XXX to ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_XXX.

Now, back to studying how ${PROJECT_NAME}_EXTRA_REPOSITORIES is being used.

So, for the checkin-test.py script it really only needs a list of extra git repos that it needs to pull from and push to for it to do its job. The list of extra repos for checkin-test.py is produced by the file:

./ci_support/TribitsGetExtraReposForCheckinTest.cmake

It think this is as easy as changing from ${PROJECT_NAME}_EXTRA_REPOSITORIES to ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES and that should be it.

Now the file:

./ci_support/TribitsDumpDepsXmlScript.cmake

takes in ${PROJECT_NAME}_EXTRA_REPOSITORIES as cache var argument. I think instead, this needs to accept the path of an ExtraRepositoriesList.cmake file to use instead. That would be better for VERA. But I guess that to support the --extra-repos argument to checkin-test.py, it needs to accept ${PROJECT_NAME}_EXTRA_REPOSITORIES as well as it does not.

The file:

./ctest_driver/TribitsCTestDriverCore.cmake

really just loops over ${PROJECT_NAME}_EXTRA_REPOSITORIES and reads from ${PROJECT_NAME}_EXTRA_REPOSITORIES_REPOTYPES and ${PROJECT_NAME}_EXTRA_REPOSITORIES_REPOURLS and uses that to clone the repos (or call other git commands). It does not care about the order of the packages. For this, you just change over to ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES and ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_XXX and that should be it.

Now for the file:

./core/package_arch/TribitsProcessExtraRepositoriesList.cmake

This file obviously needs more changes. But really, it just needs to read the 2-D array and populate ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES and ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_XXX (which now includes XXX = PREPOST) and that is it. It needs to be augmented to look for 'PRE' and 'POST' in the PACKSTAT field and use it to populate the PACKSTAT and PREPOST arrays and that is about it. I will need to create a bunch of unit tests to make sure that it processes the 2-D array correctly and catches lots of types of mistakes. That will be the most work I think.

Finally, there is the file:

./core/package_arch/TribitsGlobalMacros.cmake

So there are several places in this file where it is going to need to use ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES instead of ${PROJECT_NAME}_EXTRA_REPOSITORIES. Some of the more significant/complex functions to update in this file include:

• TRIBITS_GENERATE_REPO_VERSION_FILE_STRING(): Just needs to loop over ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES instead of ${PROJECT_NAME}_EXTRA_REPOSITORIES.
• TRIBITS_READ_PACKAGES_PROCESS_DEPENDENCIES_WRITE_XML(): Needs to loop over pre-extra repos first, then native repos, the post-extra repos last. Will need to break out a function that processes extra repos in two sets then call that function twice (once for pre-extra repos and once for post-extra repos).

That does not look too bad to me.

Now for the plan. How to go about implementing this?

1) Add the new variable ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES and change the names of the variables ${PROJECT_NAME}_EXTRA_REPOSITORIES_XXX to ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_XXX

Get all of the TriBITS tests working at the end.

Here is the plan:

• Add the new variables ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES and ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_XXX [Done]
• Replace all usage of ${PROJECT_NAME}_EXTRA_REPOSITORIES and ${PROJECT_NAME}_EXTRA_REPOSITORIES_XXX with ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES and ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_XXX [Done]
• Finally, remove the variables ${PROJECT_NAME}_EXTRA_REPOSITORIES_XXX [Done]

2. Add support for 'PRE' and (optional) 'POST' values to the 'PACKSTAT' field in the 2D array read by TRIBITS_PROCESS_EXTRAREPOS_LISTS() and add unit tests for all the valid use cases and to catch common errors (like having all 'PRE' repos followed by all 'POST' repos).

Refactor ToDos:

• Change name of ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_PACKSTATS ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_HASPKGSto differentiate withPACKSTATfield in 2D array (that hasHASPKGSandPREPOST` fields).
• Change name of ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_REPOTYPE to ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_VCTYPE to make more clear. [Done]
• Change name of variable ${PROJECT_NAME}_EXTRAREPOS_DIR_REPOTYPE_REPOURL_PACKSTAT_CATEGORY to ${PROJECT_NAME}_EXTRAREPOS_DIR_VCTYPE_REPOURL_PACKSTAT_CATEGORY but set latter if empty if former is set and issue CMake warning. Add unit test for this use case. [Done]
• Try to remove the variable ${PROJECT_NAME}_NO_CHECK_FOR_MISSING_EXTRA_REPOS and instead use only ${PROJECT_NAME}_CHECK_EXTRAREPOS_EXIST! (Will need to conditionally set variable SET(${PROJECT_NAME}_CHECK_EXTRAREPOS_EXIST TRUE) in TribitsGlobalMacros.cmake. [Done]
• Add ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES_PREPOST to TRIBITS_FILTER_OR_ASSERT_EXTRA_REPOS() and add to unit tests. Need to call the function TRIBITS_GET_AND_PROCESS_EXTRA_REPOSITORIES_LISTS() to be able to check this (with ${PROJECT_NAME}_CHECK_EXTRAREPOS_EXIST=FALSE). [Done]

ToDos:

• Add helper function TRIBITS_PARSE_EXTRAREPO_PACKSTAT() to parse all combinations of PACKSTAT into HASPKGS and PREPOST and write good unit tests. [Done]
• Update TRIBITS_PROJECT_DEFINE_EXTRA_REPOSITORIES() to use TRIBITS_PARSE_EXTRAREPO_PACKSTAT() and add unit tests to make sure it parses PACKSTAT out into HASPKGS and PREPOST with the new values 'HASPACKAGES' and 'PRE'. [Done]
• Update TRIBITS_FILTER_OR_ASSERT_EXTRA_REPOS() so that it support 'PRE' repos and test with ${PROJECT_NAME}_PRE_REPOSITORIES!='' test cases. [Done]
• Add error checks and unit tests to make sure all PRE repos are defined before post repos. [Done]
• Update documentation for TRIBITS_PROJECT_DEFINE_EXTRA_REPOSITORIES() to mention new values of PACKSTAT. [Done]
• Update TRIBITS_READ_PACKAGES_PROCESS_DEPENDENCIES_WRITE_XML() to process both pre-extra and post-extra repos and add tests to prove that it works as it should.
  • Break off function TRIBITS_SET_ALL_EXTRA_REPOSITORIES() to set
    ${PROJECT_NAME}_ALL_EXTRA_REPOSITORIES from
    ${PROJECT_NAME}_PRE_REPOSITORIES and ${PROJECT_NAME}_POST_REPOSITORIES
    when it is empty. [Done]
  • Create macro TRIBITS_PROCESS_EXTRA_REPOSITORIES(PRE_OR_POST_REPOS) that
    will process the 'PRE' or 'POST' extra repos. [Done]
  • Create unit tests that tests pre-extra repos [Done]
• Add new example repo TribitsExampleProjectAddons:
  • Create TribitsExampleProjectAddons as a TriBITS repo with one package (for
    now) [Done]
  • Add a test to build it as a POST extra repo for TribitsExampleProject
    [Done]
  • Make TribitsExampleProjectAddons its own TriBITS project with PRE extra
    repo TribitsExampleProject. Add test case for this. [Done]
• Change name of ExternalPkg to InsertedPkg and update documentation [Done]
• Add TribitsExampleMetraProject that pulls in TribitsExampleProject and
  TribitsExampleProjectAddons as PRE and POST extra repos and add tests [Done]
• Add unit tests for pre-extra repos for checkin-test.py [Done]
• Add unit tests for pre-extra repos for TribitsCTestDriverCore.cmake [Done]

[bartlettroscoe]

With the branch pre_extra_repos_73 now merged in to 'master' through the merge commits:

d75c9dc "Merge remote branch 'casl-dev/master'"
Author: Roscoe A. Bartlett <[email protected]>
Date:   Wed Jun 10 15:26:30 2015 -0400 (3 hours ago)

36cf770 "Merge branch 'pre_extra_repos_73'"
Author: Roscoe A. Bartlett <[email protected]>
Date:   Wed Jun 10 13:01:23 2015 -0400 (4 hours ago)

This issue is now resolved. MPACT changes have already been made on a branch to take advantage of this and it seems to be working perfectly.

[bartlettroscoe]

Closing and deployed.