ROL configuration failure on nightly testing

[bmpersc]

@trilinos/rol There is a configuration issue in ROL that can be seen at https://testing.sandia.gov/cdash/viewConfigure.php?buildid=2604389. It looks like the issue is that some tests were moved to a different directory and the CMakeLists.txt in topo-opt was moved with them. It seems the solution is to change the add_subdirectory for topo-opt to point instead to the two subdirs in topo-opt. Does that sound correct to you?

[dridzal]

Thanks @bmpersc , confirmed. The commit

309caa: Forgot to add a new CMake file.

should fix it. I should note that current pre-checkin tests would not have caught this. @bartlettroscoe , has there been any progress on a pre-checkin system that will catch errors due to missed git adds? This has been a major source of problems for the @trilinos/rol team, in particular when we move large amounts of code. This issue is also related to testing against installation builds.

[bmpersc]

Thanks for the quick fix for this.

I don't agree that the checkin test script would not have caught this. ROL would have been enabled with that script and ROL can't configure without the missing CMakeLists.txt so the configure would have failed for the checkin test script in the same manner it did for the continuous testing.

[dridzal]

I disagree: The CMakeLists.txt file exists in my local directory. I built everything just fine and ran all tests before pushing. I simply forgot to 'git add' it. Clearly, 'git status' showed it as an untracked file, but there were a few dozen such files because of the massive changes, and I missed that one. The checkin test scripts needs to work with a separate repo if we are to catch these errors. Basically, it would push to this (new local) repo, run all the tests, and then push to the main repo. Is there another way to do this?

[bmpersc]

I believe there is a check for modifications to your repo that are not committed which would catch issues like you just described.

[dridzal]

Is this a check that the checkin script can execute, or is this a global git setting that I need to use on every development platform (and there are 6 or 7 in my case)?

[mhoemmen]

I disagree: The CMakeLists.txt file exists in my local directory. I built everything just fine and ran all tests before pushing. I simply forgot to 'git add' it.

If you run the check-in test script with the --push argument, it normally refuses to push unless all local changes have been committed. That can be helpful for detecting issues like not adding a new file.

[bmpersc]

It has been a while since I've run into that particular check so Mark may be right that it only happens when you use --push. However, it is a check that is done by the checkin test script and not an option to git so you will not have to update multiple clones of Trilinos for this feature.

[dridzal]

I see ... so I can no longer have temporary files in my local repo. It must be in a 'clean' state at all times, correct?

[bartlettroscoe]

checkin-test.py script by default requires a perfectly clean working directory. From checkin-test.py --help:

DETAILED DOCUMENTATION
-----------------------

The following approximate steps are performed by this script:

----------------------------------------------------------------------------

1) Check to see if the local repo(s) are clean:

  $ git status

  NOTE: If any modified or any unknown files are shown, the process will be
  aborted.  The local repo(s) working directory must be clean and ready to
  push *everything* that is not stashed away.

We need to compartmentalize and organize this documentation but it is there.

I need to find the time to complete #482 and its blocking Issues before the checkin-test-sems.sh script will be ready to go for everyone to use.

But we also have some other problems with basing the pre-push CI env on SEMS (see here). We need to discuss this at the Trilinos leaders level.

[mhoemmen]

I see ... so I can no longer have temporary files in my local repo. It must be in a 'clean' state at all times, correct?

Yes. The check-in test script tells you if your working directory is not clean.

[dridzal]

Thanks for all input. I'll discuss keeping the working directories clean with @trilinos/rol developers, and using .gitignore occasionally. I think this is better than pushing to a temporary repo just to catch missing files. Closing the issue.

[bartlettroscoe]

I see ... so I can no longer have temporary files in my local repo. It must be in a 'clean' state at all times, correct?

Yes. The check-in test script tells you if your working directory is not clean.

@dridzal , if this policy does not make sense then let's talk. But note that you can use the checkin-test.py script to test a "dirty" local working directory using --allow-no-pull (part of --local-do-all). It is just that it is not safe to pull updates or test changes to be pushed without having a 100% clean local working directory. Again, from the checkin-test.py --help documentation:

QUICKSTART
-----------

In order to do a safe push, perform the following recommended workflow
(different variations on this workflow are described in the COMMON USE CASES
section below):

1) Commit changes in the local repo:

  # 1.a) See what files are changed, newly added, etc.
  $ git status

  # 1.b) Stage the files you want to commit
  $ git stage <files you want to commit>

  # 1.c) Create your local commits
  $ git commit -- SOMETHING
  $ git commit -- SOMETHING_ELSE
  ...

  # 1.d) Stash whatever changes are left you don't want to test/push
  $ git stash

  NOTE: You can group your commits any way that you would like (see the basic
  git documentation).

  NOTE: When multiple repos are involved, use the 'gitdist' command instead of
  'git'.  This script is provided at tribits/python_utils/gitdist

[mhoemmen]

If I have unstashed, uncommitted changes, here is my typical workflow:

1. Run check-in test script with "--allow-no-pull --configure"
2. Build (run make) and run ctest manually in one of the build directories (e.g., MPI_DEBUG)
3. Once I get the changes right, commit, settle up my git history, and run the check-in test script

Rebuilds aren't generally expensive in packages you haven't changed.

[dridzal]

@mhoemmen , this is interesting, but I think I need more information. Can you please explain what the first step buys you? I assume that in the third step you run the check-in test script without --allow-no-pull.

[mhoemmen]

@dridzal wrote:

@mhoemmen , this is interesting, but I think I need more information. Can you please explain what the first step buys you?

The point of the first step, is that you can use the same CMake configuration process for normal development, that you use for running the check-in test script. I use my check-in test script(s) to manage different builds (e.g., MPI_DEBUG and SERIAL_RELEASE). It's handy to have one script that can manage different builds. You might want to change files without making commits, or you might have added files and not yet be sure enough to commit them. This model lets you develop with uncommitted changes or new files, until you are ready to commit them. Furthermore, if you add "--enable-all-packages=off", you can even develop in a different (e.g., temporary) branch with uncommitted changes.

I assume that in the third step you run the check-in test script without --allow-no-pull.

Yes. You would need to settle up your commits first, get on the develop branch, then run the check-in test script with "--do-all --push".