Skip to content

Fix change logic in get-changed-trilinos-packages.sh and PullRequestLinuxDriver.sh (#3133)

James Willenbring requested to merge bartlettroscoe:3133-fix-logic into develop

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.

Merge request reports