Use features in newer CTest versions to improve and robustify git version control updates for base repo #278
Comments
…#278) For some reason, ctest_update() says these commands fail. But when I run them myself, they work!
…Pub#278) I can't any ctest_update() invocation to work!
…BITSPub#278) So you need to use semi-colons for every argument in a **single** shell command!
…OM (TriBITSPub#278) This does not work. You can't use '&&' to string together commands. And that is not even portable. Therefore, we will need to move to another appraoch.
…SPub#278) I also added this to the notes files and refactored that a little to clean them up.
Can't actually do an update as of now with this code but perhaps we could figure out how to do that using a mock git?
…TSPub#278) This makes sure they run the commands they should in unit test mode.
…e some output (TriBITSPub#278) Now I need to just document one set of git commands and they apply to the base repo and the extra repos.
…cmake (TriBITSPub#278, TriBITSPub#154) This is needed to be able to read XML files out of the <bulid>/Testing/<buildstarttime>/ directory needed for (TriBITSPub#278) This will also make it easy to generate a CDash URL without knowing the build ID.
Can't actaully use this in checking the contents of the Notes.xml file because that file does not get generated unless ctest_submit() is called and we can't do CDash submits by default for lots of obvious reasons. I am going to set up some tests of tribits_ctest_driver() that actually do clones and updates and call ctest_update() that will then use this file.
Looks like the git version on TravisCI puts "'" around the branch and remote names.
This is a start but these really need to be converted into README.md files and make more complete.
…tronger testing (TriBITSPub#278, TRIL-260) This commit represents a fairly significant enhancement of the handing of git repo clones and updates. Previous to this commit, CDash would not report the correct git repo SHA1 if the branch was not the default branch on the initial clone or in checking out a new branch. Also, the git commands were not robust in use cases where the current branch in a base repo or an extra repo was not on a tracking branch or the branch in the remote repo did not exist (like occurred in TRIL-260 which changing from 'atdm-develop-nightly' to 'atdm-nightly' where 'atdm-develop-nightly' got deleted in the GitHub repo). The documentation for git repo updates and been improved and streamlined. This includes a huge step forward in automated testing of the tribits_ctest_driver() function in how it does real git repo clones and updates for various use cases. To accomplish this, a new set of repos were create under https://github.com/tribits/ for the example projects/repos TribitsExampleMetaProject, TribitsExampleProject and TribitsExampleProjectAddons. There is a branch 'for-testing' in each of those repos that have known stats that are used for strong testing. Tests are set up based on a base TribitsExampleMetaProject repo/project that clones the extra repos TribitsExampleProject and TribitsExampleProjectAddons so we cover use cases in the base repo and extra repos (which are mostly decoupled). This testing helped to catch invalid logic for the extra repos (for the case where the current branch is not a tracking branch). Some of the specific things done include: * Switched from CTEST_UPDATE_COMMAND to CTEST_GIT_COMMAND (this really is git only) * Added CTEST_GIT_UPDATE_CUSTOM which calls a ctest -P script to do the improved git repo cleanup and update commands (see updated documentation). * Allow a set of CTEST_NOTES_FILES be specified by the user and correctly added to other notes files created internally and (hopefully) submitted to CDash. There are now strong unit tests for the full listing of notes files that are generated (which requires running ctest_update() to generate a few of the files). * Added TRIBITS_READ_CTEST_TAG_FILE() and TribitsGetCTestTestXmlDir.cmake (TriBITSPub#278, TriBITSPub#154): This is needed to be able to read XML files out of the <bulid>/Testing/<buildstarttime>/ directory needed for (TriBITSPub#278). This will also make it easy to generate a CDash URL without knowing the build ID. But this is not hooked in yet. It was added here with the hope to check the contents of the Notes.xml file but that file does not get generated unless you actually submit to CDash (which we can't do yet and don't want to rely on). Also added the file Add list_cdash_notes_files_in_xml_file.sh in hopes of doing this testing. * Remove the function to get the extra repo tracking branch. The old code assumed that the current branch in an extra repo was on a tacking branch. That is a bad assumption. The updated code just uses '@{u}' as the tracking branch. This was a real defect that was found through careful automated testing with real git commands. This is why we need some tests with real git commands to validate our implementation. * Updated some files in the example repos TribitsExampleMetaProject, TribitsExampleProject and TribitsExampleProjectAddons some to get these testing use cases working. Also improved the README files some more, but more is needed.
|
PR #279 was merged. The snaphsot PR Trilinos is trilinos/Trilinos#4438. Once the Trilinos PR trilinos/Trilinos#4438 merges and is used for a few nights in ATDM Trilinos builds and testing, then we can close this Issue. |
Some of these typos were found during the review of trilinos/Trilinos#4438. I also fixed "PASSSED" with "PASSED" that was grepped for in a bunch of automated tests. All good now :-) Build/Test Cases Summary Enabled Packages: Enabled all Packages 0) MPI_DEBUG => passed: passed=341,notpassed=0 (0.86 min) 1) SERIAL_RELEASE => passed: passed=341,notpassed=0 (0.93 min)
|
Just realized that we could move the extra repo clone and update commands inside of the command run by ctest_update() so that if any of them fail, we could report an update failure to CDash and abort the build. This would also result in getting the extra repo commands to be sent to CDash as a notes file as well. I need to make this happen and add some strong automated tests of tribits_ctest_driver() to ensure that all forms of git clone and update failures of the base and extra repos are caught and reported correctly! |
|
FYI: Below are the notes I wrote from over a year ago that lead to several commits including in PR #279 working towards this. Notes from Feb 2019(2/14/2019) Now I want to test that the updated git clone command does what it is supposed to do. To test this out, I will try running a TriBITS ctest -S driver script to clone my TriBITS repo on a branch with a different remote name and see if that works. First, I had to write a ctest -S driver script for TriBITS. I only had one for TravisCI! After I created that commit, I tested this on 'crf450' with: That seemed to do the trick and it cloned the repo: That looks correct. Now let's clone on a branch with a different remote name and different repo: That gave the git repo: That looks correct to me :-) I think that is good enough testing for the clone part. After implementing the custom update, I ran: Darn, the ctest_update() command is failing and it is showing: If I revert back to the original version of the code with: and run as above I get: What I am noticing is that the modified version of the code is showing and empty: while the old version showed: What is going on with that? I need to go back to the orginal updating code that works and change the commands one step at a time and see what is triggering this messed up state that causes ctest_update() to fail! I am thinking it must be something different about the initial clone because ctest_update() is showing this strange behavior of now new or old git repo versions even before it tries to run the actual git commands! (2/15/2019) I reset the branch to just the commit with the updated clone instructions with the new commits: and ran the clone with: And I ran the update with an existing clone with: So that means that the checkout commands look to be okay. So that means that this version of the code will put the git repo on the right branch with the right remote name right from the initial clone. Now, what changes were made to the ctest_update() after commit a93bc1c that could break this? {noformat} ... Looks very minimal. I created the new local branch '278-improve-ctest-s-git-commands-tmp' and will work very incremnetally there. I will look at the diff above and make changes manually one statement at a time and see what happens ... So I switched from: to: and it works: Now, the question is if switching to but this time, I did not remove the var So you need to always set So that is it, ctest is broken w.r.t. I will go ahead and file a bug report for this to Kitware with reproducability instructions. Stop, wait, I read the documentation wrong. The variable Unfortunately that did not work :-( And that would not have even been portable anyway. Therefore, we need to move to a different approach. An approach that I think will work will be to run an execute_process() command in a cmake -P script. You can string together multiple commands in an execute_process() function call in a portable way. To make this visible, it would be good to capture the output from these git command and post this as a notes file to the build and also echo in the ctest -S STD output. Bingo! I got it to work with a cmake -P script with That seems to work pretty well and I verified manually with real git clones that it can change to a new branch even when the existing tracking branch has its remote tracking branch disappear! But I can't see the git output in case something fails. So to fix that, I need to create a wrapper cmake -P script that will run tribits_ctest_update_commands.cmake and then send its output to a file (UpdateCommandsOutput.txt) and then attach that a a CDash notes file. (2/16/2019) Okay, I have gotten everything done that I think needs to get done TODOS from Feb 2019TODOS:
|
Due to the configure attempt logic, this will also abort any actions in future ctest -S invocations as well (as long as update and configure are attempted in the same ctest -S invocation).
Build/Test Cases Summary Enabled Packages: Enabled all Packages 0) MPI_DEBUG => passed: passed=376,notpassed=0 (1.82 min) 1) SERIAL_RELEASE => passed: passed=376,notpassed=0 (1.61 min) Other local commits for this build/test group: 145ffb3
|
I believe that with commit bartlettroscoe@145ffb3 merged to 'master' in ee227d8 that this story is finally complete, at least for the base repo. What is not resolved, is making the update of the extra repos robust as well. To do that, the logic that updates the extra repos really needs to executed inside of the |
With the creation of #338 to address reporting and handling of errors cloning/updating extra repos, we can descope this story and close it. (Trilinos does not have any extra repos used in its testing so this story has provided real value there.) |
TriBITSPub#278) This was created by commit: 49d9c81 "tribits_ctest_driver(): Use more robust and better git commands and stronger testing (TriBITSPub#278, TRIL-260)" Author: Roscoe A. Bartlett <rabartl@sandia.gov> Date: Thu Feb 14 13:42:11 2019 -0700 A test/ctest_driver/TribitsExampleMetaProject/CMakeLists.txt
This were fixes from the previous comment: 49d9c81 "tribits_ctest_driver(): Use more robust and better git commands and stronger testing (TriBITSPub#278, TRIL-260)" Author: Roscoe A. Bartlett <rabartl@sandia.gov> Date: Thu Feb 14 13:42:11 2019 -0700
CC: @fryeguy52
Next Action Status
Got specialized more robust git commands for base git repo called under
ctest_update()correctly. Next: Move git commands for extra repos underctest_update()and test that update failures are caught and handled correctly ...Description
The current code in
tribits_ctest_driver()checks out correct branches outside of thectest_start()command (which does the initial git clone) and thectest_update()command (which does updates of the repo after a clone). There are several problems with this implementation:If the branch that is desired is not the selected branch that is checked out after the initial clone, then CDash reports the wrong git repo SHA1.
If an already checked-out repo is on a tracking branch that has its remote tracking branch changed, then the
git fetchwill fail andctest_update()will fail. (This is what just happended today with the change of the Trilinos branch 'atdm-develop-nightly' to 'atdm-nightly' as described in TRIL-260.)This makes for a fragile system and with incorrect version info on CDash in some cases.
Proposed Solution
To address these problems, we can make better usage of features in newer versions of CMake.
First, the TriBITS project (or user) defines the vars:
GIT_EXE: The git executable (found withfind_program(GIT_EXE git ...))${PROJECT_NAME}_REPOSITORY_LOCATION: The Git repo URL used ingit clone <repo-url>${PROJECT_NAME}_GIT_REPOSITORY_REMOTE: The name of the git repo remote after clone (default is 'origin') (this would be a new variable)${PROJECT_NAME}_BRANCH: The name of the branch to checkout. (Will be empty "" if to just use the default branch.)For the initail clone, we can specify:
If no branch is provied, then the command is just:
For updates, the command would be:
If there was no branch, the last command would instead be:
(but that requires that the current branch already is a tracking branch).
The text was updated successfully, but these errors were encountered: