Fix change logic in get-changed-trilinos-packages.sh and PullRequestLinuxDriver.sh (#3133)
Created by: bartlettroscoe
@trilinos/framework
Description
This PR contains two sets of commits.
The first set of commits fixes the behavior of the get-changed-trilinos-packages.sh
script to apply the correct global change logic for Trilinos. This updates TriBITS snapshot and adds unit test to ensure that the logic in the file Trilinos/cmake/ProjectCiFileChangeLogic.py
is getting correctly applied in the script get-changed-trilinos-packages.sh
used by the Trilinos PR tester. The unit testing of get-changed-trilinos-packages.sh
was not strong enough.
The second set of changes in the commit b2cccbf5f5b2f56d31ce7fa2ae6c4957c938427c which passes in the correct SHA1 for selecting the set of changed files in the script PullRequestLinuxDriver.sh
. See #3265 (closed).
Motivation and Context
The TriBITS scripts called by get-changed-trilinos-packages.sh
were not pickup up the logic in Trilinos/cmake/ProjectCiFileChangeLogic.py
and was instead likely using the logic in TribitsExampleProject/cmake/ProjectCiFileChangeLogic.py
(see TriBITSPub/TriBITS@9b9822f5f2ab7183bdc47eec2193a5fbf6ab83d1). As a result, PRs like #3251 were incorrectly triggering the enable of all Trilinos packages.
Also the modification to PullRequestLinuxDriver.sh
in PR #3218 was using the wrong base SHA1 and was resulting in testing more packages than needed for the PR in many cases (see #3265 (closed)).
The merge of this PR should complete #3133 (closed).
How Has This Been Tested?
I expanded the unit tests for get-changed-trilinos-packages.sh
to ensure it ignores a change to a *.cmake
file under the cmake/std/adm/
directory. I also ran the script manually for a few PRs, including #3251, and saw that it is selecting the right packages to test (and no package in the case of PR #3251).
To test this updated script PullRequestLinuxDriver.sh
, I did the following.
First, I created the separate dummy PR testing repo where the PR branch merges are done:
$ cd ~/junk/DummyTrilinosPRDriver
$ ls
Trilinos
$ cd Trilinos/
$ git remote -v
origin git@github.com:trilinos/Trilinos.git (fetch)
origin git@github.com:trilinos/Trilinos.git (push)
The I just ran the script PullRequestLinuxDriver.sh
on this for a few real PRs.
First, I tested this for the PR #3260:
$ env \
TRILINOS_SOURCE_BRANCH=tril-215-white-ride-cuda-9.2 \
TRILINOS_SOURCE_REPO=https://github.com/bartlettroscoe/Trilinos \
TRILINOS_SOURCE_SHA=c96c475fbbc3162e4618050c3e893dcb107a73e1 \
TRILINOS_TARGET_BRANCH=develop \
TRILINOS_TARGET_REPO=https://github.com/trilinos/Trilinos \
TRILINOS_TARGET_SHA=45adc9002091ba1fc6ae296c72696f8717b8b885 \
PULLREQUESTNUM=111 \
JOB_BASE_NAME=Trilinos_pullrequest_gcc_4.8.4 \
BUILD_NUMBER=222 \
WORKSPACE=$PWD \
~/Trilinos.base/Trilinos/cmake/std/PullRequestLinuxDriver.sh \
&> console.out
$ cmake -P packageEnables.cmake
-- NOTE: No packages are being enabled!
That is the correct set of enables (i.e. no enables). That will save me huge amounts of time (and would have allowed PR #3260 to be merged immediately).
Then I tested this for a few other PRs ...
Testing for PR #3275:
$ env \
TRILINOS_SOURCE_BRANCH=sacado_cppunit_fix \
TRILINOS_SOURCE_REPO=https://github.com/etphipp/Trilinos \
TRILINOS_SOURCE_SHA=3fce9c6d41039f78d6f8def4d404e234ea06449a \
TRILINOS_TARGET_BRANCH=develop \
TRILINOS_TARGET_REPO=https://github.com/trilinos/Trilinos \
TRILINOS_TARGET_SHA=4956217a66dba575a6c422dd43b6b2586fe85476 \
PULLREQUESTNUM=111 \
JOB_BASE_NAME=Trilinos_pullrequest_gcc_4.8.4 \
BUILD_NUMBER=222 \
WORKSPACE=$PWD \
~/Trilinos.base/Trilinos/cmake/std/PullRequestLinuxDriver.sh \
&> console.out
$ cmake -P packageEnables.cmake
-- Setting Trilinos_ENABLE_Sacado = ON
That looks correct.
Testing for PR #2930:
$ env \
TRILINOS_SOURCE_BRANCH=import_stk \
TRILINOS_SOURCE_REPO=https://github.com/prwolfe/Trilinos \
TRILINOS_SOURCE_SHA=b2674b6c78f0ca1b0f6c3b79be1e7f5a18d9a34a \
TRILINOS_TARGET_BRANCH=develop \
TRILINOS_TARGET_REPO=https://github.com/trilinos/Trilinos \
TRILINOS_TARGET_SHA=45adc9002091ba1fc6ae296c72696f8717b8b885 \
PULLREQUESTNUM=111 \
JOB_BASE_NAME=Trilinos_pullrequest_gcc_4.8.4 \
BUILD_NUMBER=222 \
WORKSPACE=$PWD \
~/Trilinos.base/Trilinos/cmake/std/PullRequestLinuxDriver.sh \
&> console.out
$ cmake -P packageEnables.cmake
-- Setting Trilinos_ENABLE_STK = ON
-- Setting Trilinos_ENABLE_STKDoc_tests = ON
-- Setting Trilinos_ENABLE_STKExprEval = ON
-- Setting Trilinos_ENABLE_STKIO = ON
-- Setting Trilinos_ENABLE_STKMesh = ON
-- Setting Trilinos_ENABLE_STKSearch = ON
-- Setting Trilinos_ENABLE_STKSearchUtil = ON
-- Setting Trilinos_ENABLE_STKSimd = ON
-- Setting Trilinos_ENABLE_STKTools = ON
-- Setting Trilinos_ENABLE_STKTopology = ON
-- Setting Trilinos_ENABLE_STKTransfer = ON
-- Setting Trilinos_ENABLE_STKUnit_test_utils = ON
-- Setting Trilinos_ENABLE_STKUnit_tests = ON
-- Setting Trilinos_ENABLE_STKUtil = ON
That looks correct. (FYI @prwolfe)
Testing for PR #3258: (Prior to updating the branch and the PR)
$ env \
TRILINOS_SOURCE_BRANCH=3133-fix-logic \
TRILINOS_SOURCE_REPO=https://github.com/bartlettroscoe/Trilinos \
TRILINOS_SOURCE_SHA=b2cccbf5f5b2f56d31ce7fa2ae6c4957c938427c \
TRILINOS_TARGET_BRANCH=develop \
TRILINOS_TARGET_REPO=https://github.com/trilinos/Trilinos \
TRILINOS_TARGET_SHA=45adc9002091ba1fc6ae296c72696f8717b8b885 \
PULLREQUESTNUM=111 \
JOB_BASE_NAME=Trilinos_pullrequest_gcc_4.8.4 \
BUILD_NUMBER=222 \
WORKSPACE=$PWD \
~/Trilinos.base/Trilinos/cmake/std/PullRequestLinuxDriver.sh \
&> console.out
$ cmake -P packageEnables.cmake
-- Setting Trilinos_ENABLE_ALL_PACKAGES = ON
-- Setting Trilinos_ENABLE_TrilinosFrameworkTests = ON
That looks like the correct set of enables for the set of files that are being changed in that (this) PR.
Checklist
-
My commit messages mention the appropriate GitHub issue numbers. -
I have added tests to cover my changes. -
All new and existing tests passed.