Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DO NOT MERGE: Show TriBITS test failures with CMake 3.21.0 that don't occur with CMake 3.17.5 (#363) #394

Closed

Conversation

bartlettroscoe
Copy link
Member

This PR is used to demonstrate the different behavior of CMake 3.21.0 vs. CMake 3.17.5 with the TriBITS test suite. It uses a GitHub Actions driver that posts to CDash two otherwise identical builds that differ only in the CMake version that they use. These test failures are blocking the deployment of the GitHub Actions testing in PR #393 to implement #363.

@bartlettroscoe bartlettroscoe force-pushed the 363-github-actions-tests-show-ctest-3.21-failures branch 2 times, most recently from ce8e62f to 90662cf Compare July 21, 2021 17:30
@bartlettroscoe
Copy link
Member Author

The CMake 3.21 failures compared against the identical CMake 3.17 passing tests are shown in the GHA build iteration shown here with results on CDash shown here:

showing:

Site Build Name Conf Error Conf Warn Conf Test Time Build Error Build Warn Build Test Time Test Not Run Test Fail Test Pass Test Time Start Test Time
ubuntu-latest tribits_cmake-3.17.5_makefiles_python-3.8_g++-9 0 0 12s 0 0 1s 0 0 374 6m 21s 4 hours ago
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 0 0 6s 0 0 0s 0 10 364 5m 17s 4 hours ago

The failing tests for the cmake-3.21 build are shown at:

showing:

Site Build Name Test Name Status Time Details Build Time
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_PT_ALL_PASS_CALLS_2 Failed 8s 750ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_PT_ALL_PASS_CALLS_4 Failed 11s 660ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_COVERAGE Failed 11s 130ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_MEMORY Failed 12s 760ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_PASS Failed 11s 280ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakBuildAllOptionalPkg Failed 11s 340ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakBuildLibOptionalPkg Failed 9s 780ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakConfigureOptionalPkg Failed 9s 230ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg Failed 5s 770ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakTestPkg Failed 11s 550ms Completed (Failed) 2021-07-21T13:30:43 EDT

These failures are broken down into two sets of failures of 5 tests in each set.

The first set of 5 failures are those that show runs of ctest -S scripts that should be passing but otherwise return 255 as the return code shown in:

(slick "Show Matching Output") showing:

Site Build Name Test Name Status Time Details Build Time
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_PT_ALL_PASS_CALLS_2 Failed 8s 750ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_PT_ALL_PASS_CALLS_4 Failed 11s 660ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_COVERAGE Failed 11s 130ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_MEMORY Failed 12s 760ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_PASS Failed 11s 280ms Completed (Failed) 2021-07-21T13:30:43 EDT

These are all cases where the ctest -S driver reports all passing and there is no hint in the output of anything failing but yet have the return code 255. If you compare those tests to the same tests with the cmake-3.17.5 build shown at:

you can see that ctest -S returns 0 in all of those cases.

The second set of 5 failing tests are shown in:

showing:

Site Build Name Test Name Status Time Details Build Time
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakBuildAllOptionalPkg Failed 11s 340ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakBuildLibOptionalPkg Failed 9s 780ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakConfigureOptionalPkg Failed 9s 230ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg Failed 5s 770ms Completed (Failed) 2021-07-21T13:30:43 EDT
ubuntu-latest tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakTestPkg Failed 11s 550ms Completed (Failed) 2021-07-21T13:30:43 EDT

These are all cases that have build errors on purpose. (The purpose of the tests is to ensure that tribits_ctest_driver() can detect the errors and that the remaining executables build and associated tests run and pass correctly.) It looks like CMake 3.21 is refusing to generate tests when there is a build error where CMake 3.17 still generated tests even when there was a build error. That is not good because we don't want to take down all of our testing just because one test executable does not build. I will need to report these errors.

@bartlettroscoe
Copy link
Member Author

Note that I was also able to manually reproduce these failures on my local RHEL 7 machine using a locally installed CMake 3.21.0 as described in #389 (comment) (see DETAILED NOTES in that comment).

@bartlettroscoe
Copy link
Member Author

NOTE: I created:

to get this on the SNL Kitware board.

@bartlettroscoe bartlettroscoe force-pushed the 363-github-actions-tests-show-ctest-3.21-failures branch 2 times, most recently from 82f3725 to ca9aa79 Compare July 22, 2021 17:36
@bartlettroscoe
Copy link
Member Author

CC: @marcinwrobel1986

NOTE: I took your advice and updated this branch to test all versions of CMake between 3.17 and 3.21 and it posted to CDash at:

and showed the build and test summary:

Site Build Name Conf Error Conf Warn Conf Test Time Build Error Build Warn Build Test Time Test Not Run Test Fail Test Pass Test Time Start Test Time
ubuntu-latest pr-394_tribits_cmake-3.17.5_makefiles_python-3.8_g++-9 0 0 7s 0 0 0s 0 0 374 5m 24s 13 minutes ago
ubuntu-latest pr-394_tribits_cmake-3.18.6_makefiles_python-3.8_g++-9 0 0 8s 0 0 0s 0 0 374 6m 30s 13 minutes ago
ubuntu-latest pr-394_tribits_cmake-3.19.8_makefiles_python-3.8_g++-9 0 0 6s 0 0 1s 0 1 373 5m 25s 13 minutes ago
ubuntu-latest pr-394_tribits_cmake-3.20.5_makefiles_python-3.8_g++-9 0 0 10s 0 0 0s 0 1 373 5m 57s 13 minutes ago
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 0 0 7s 0 0 1s 0 10 364 5m 28s 7 minutes ago

and the failing tests are shown at:

showing (sorted by build name then test name):

Site Build Name Test Name Status Time Details Build Time
ubuntu-latest pr-394_tribits_cmake-3.19.8_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg Failed 5s 600ms Completed (Failed) 2021-07-22T13:36:47 EDT
ubuntu-latest pr-394_tribits_cmake-3.20.5_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg Failed 6s 50ms Completed (Failed) 2021-07-22T13:36:57 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_PT_ALL_PASS_CALLS_2 Failed 9s 140ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_PT_ALL_PASS_CALLS_4 Failed 12s 390ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_COVERAGE Failed 11s 830ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_MEMORY Failed 13s 520ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_ALL_PASS Failed 11s 490ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakBuildAllOptionalPkg Failed 11s 890ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakBuildLibOptionalPkg Failed 10s 150ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakConfigureOptionalPkg Failed 9s 420ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg Failed 5s 830ms Completed (Failed) 2021-07-22T13:43:17 EDT
ubuntu-latest pr-394_tribits_cmake-3.21.0_makefiles_python-3.8_g++-9 TriBITS_CTestDriver_PBP_ST_BreakTestPkg Failed 11s 830ms Completed (Failed) 2021-07-22T13:43:17 EDT

That shows that all of the TriBITS tests passed with CMake 3.18 but in CMake 3.19, one test:

  • TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg

started failing and then in CMake 3.21, the other 9 tests started failing.

In those earlier versions, that test fails like shown for cmake 3.19 here showing:

...

Building target: 'WithSubpackages_libs' ...

Build project
   Each symbol represents 1024 bytes of output.
    . Size of output: 0K
Error(s) when building project
   1 Compiler errors
   0 Compiler warnings
Build return: RETURN_VALUE=2, NUMBER_ERRORS=1

FAILED library build for package 'WithSubpackages'!

WithSubpackages: Skipping tests since libray build failed!

...

TEST_2: Pass criteria = Match REGEX {WithSubpackages: Libs build passed} [FAILED]
TEST_2: Pass criteria = Match REGEX {WithSubpackages: All build passed} [FAILED]
TEST_2: Pass criteria = Match REGEX {No tests were found} [FAILED]
 
...

That same test scenario with cmake 3.17 shown here showed:

...

Building target: 'WithSubpackages_libs' ...

Build project
   Each symbol represents 1024 bytes of output.
    . Size of output: 0K
Error(s) when building project
   0 Compiler errors
   1 Compiler warnings
Build return: RETURN_VALUE=2, NUMBER_ERRORS=0

WithSubpackages: Libs build passed!

Build ALL target for 'WithSubpackages' ...

...

No tests were found!!!

...

TEST_2: Pass criteria = Match REGEX {WithSubpackages: Libs build passed} [PASSED]
TEST_2: Pass criteria = Match REGEX {WithSubpackages: All build passed} [PASSED]
TEST_2: Pass criteria = Match REGEX {No tests were found} [PASSED]
...

Everything else is identical here. Just the CMake versions should be different.

@marcinwrobel1986
Copy link
Collaborator

@bartlettroscoe superb Ross! I am happy, we could already benefit from that!

…re failures (#363)

This is to make it so that it is easy for Kitware to reproduce the CMake
3.21.0 issues.  This commit should only exist on this side branch
'363-github-actions-tests-show-ctest-3.21-failures' in a temp PR for testing
purposes.

I also commented out the disables of the tests that show the CMake 3.21 failures.
…well (#363)

This should help show when CMake introduced the changes that broke these
TriBITS tests.
@bartlettroscoe
Copy link
Member Author

It was determined in:

that the majority of the test failures were due to a regression in CMake from 3.20 to 3.21 which was fixed on CMake 'mastesr' in:

merged 2 weeks ago.

The remainig failing test is TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg which started failing after the merge of:

which was first deployed in CMake 3.19.0 as shown at:

I suspect this may not be a defect in CTest but might be a defect in this test.

The test TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg runs a package-by-package build and test scendario where the configure of the upstream package SimpleCxx by breaking the file packages/simple_cxx/CMakeLists.txt. This causes the configure of the package SimpleCxx to fail and therefore causes the SimpleCxx package to get disabled in all downstream packages. The packages that have direct required dependencies on SimpleCxx are WithsubpackagesA and WithsubpackagesB. But this takes out WithsubpackagesC and also WrapExternal as shown in the below generated depenency table (from configuring TribitsExampleProject with -DTribitsExProj_DEPS_DEFAULT_OUTPUT_DIR:PATH=${PWD}):

image

But the package MixedLang has no dependencies on SimpleCxx so we should expect it configure, build and test successfully. But we should not expect the package Withsubpackages or WrapExternal to even configure (unless <Project>_DISABLE_ENABLED_FORWARD_DEP_PACKAGES is set to ON). And it looks like it is!

$ grep DISABLE_ENABLED_FORWARD_DEP_PACKAGES test/ctest_driver/TribitsExampleProject/TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg/BUILD/CMakeCache.txt 
TribitsExProj_DISABLE_ENABLED_FORWARD_DEP_PACKAGES:BOOL=ON

I submitted this case to CDash with results shown at:

What is strange is that the configure failure for the SimpleCxx package is not even reported there, but it should have been. I think this may be a defect in CTest and/or CDash. CTest said configure results for the SimpleCxx package got submitted as shown from the local test output:

338: Configuring TRIBITS_PACKAGE='SimpleCxx'
338: 
338: CONFIGURE_OPTIONS = '-DTribitsExProj_TRIBITS_DIR=/ascldap/users/rabartl/Trilinos.base/Trilinos/TriBITS/tribits;-DCTEST_USE_LAUNCHERS:BOOL=1;-DTribitsExProj_ENABLE_ALL_OPTIONAL_PACKAGES:BOOL=ON;-DTribitsExProj_WARNINGS_AS_ERRORS_FLAGS:STRING=;-DTribitsExProj_ALLOW_NO_PACKAGES:BOOL=ON;-DTribitsExProj_DISABLE_ENABLED_FORWARD_DEP_PACKAGES=ON;-DTribitsExProj_DEPS_XML_OUTPUT_FILE:FILEPATH=;-DTribitsExProj_ENABLE_SECONDARY_TESTED_CODE:BOOL=TRUE;-DTribitsExProj_EXTRAREPOS_FILE:STRING=;-DTribitsExProj_IGNORE_MISSING_EXTRA_REPOSITORIES:BOOL=ON;-DTribitsExProj_ENABLE_KNOWN_EXTERNAL_REPOS_TYPE:STRING=Continuous;-DTribitsExProj_ENABLE_TESTS:BOOL=ON;-DBUILD_SHARED_LIBS:BOOL=ON;-DCMAKE_BUILD_TYPE=DEBUG;-DCMAKE_C_COMPILER=gcc;-DCMAKE_CXX_COMPILER=g++;-DCMAKE_Fortran_COMPILER=gfortran;-DTribitsExProj_ENABLE_Fortran=ON;-DTribitsExProj_TRACE_ADD_TEST=ON;-DTribitsExProj_ENABLE_INSTALL_CMAKE_CONFIG_FILES=OFF;-DTribitsExProj_ENABLE_SimpleCxx:BOOL=ON'
338: Configure project
338:    Each . represents 1024 bytes of output
338:     ......... Size of output: 8K
338: Error(s) when configuring the project
338: Generating the file '/home/rabartl/Trilinos.base/BUILDS/GCC-4.8.3/TRIBITS_MPI_DEBUG_CMAKE_3.21/test/ctest_driver/TribitsExampleProject/TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg/BUILD/CMakeCache.clean.txt' ...
338: 
338: SimpleCxx FAILED to configure!
338: 
338: -- CTEST_NOTES_FILES=';/home/rabartl/Trilinos.base/BUILDS/GCC-4.8.3/TRIBITS_MPI_DEBUG_CMAKE_3.21/test/ctest_driver/TribitsExampleProject/TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg/BUILD/CMakeCache.clean.txt'
338: 
338: Submitting configure and notes ...
338: info: using retry_args='RETRY_COUNT;5;RETRY_DELAY;3' for _ctest_submit call
338: 	Add file: /home/rabartl/Trilinos.base/BUILDS/GCC-4.8.3/TRIBITS_MPI_DEBUG_CMAKE_3.21/test/ctest_driver/TribitsExampleProject/TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg/BUILD/CMakeCache.clean.txt
338:    Use HTTP Proxy: http://proxy.sandia.gov:80
338: Submit files
338:    SubmitURL: http://testing.sandia.gov/cdash/submit.php?project=TribitsExProj
338:    Uploaded: /home/rabartl/Trilinos.base/BUILDS/GCC-4.8.3/TRIBITS_MPI_DEBUG_CMAKE_3.21/test/ctest_driver/TribitsExampleProject/TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg/BUILD/Testing/20210812-1517/Configure.xml
338:    Uploaded: /home/rabartl/Trilinos.base/BUILDS/GCC-4.8.3/TRIBITS_MPI_DEBUG_CMAKE_3.21/test/ctest_driver/TribitsExampleProject/TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg/BUILD/Testing/20210812-1517/Notes.xml
338:    Submission successful
338: -- CTEST_CUSTOM_MAXIMUM_PASSED_TEST_OUTPUT_SIZE=''
338: -- CTEST_CUSTOM_MAXIMUM_FAILED_TEST_OUTPUT_SIZE=''
338: -- PBP_CONFIGURE_PASSED='FALSE'
338: 
338: SimpleCxx: Skipping build due to configure failing!

It looks like those configure results in:

BUILD/Testing/20210812-1517/Configure.xml

just got dropped from CDash or something. What happened to them?

In any case the next configure of the individual packages Withsubpackages and WrapExteranl is shown on CDash and both show:

***
*** WARNING:  There were no packages configured so no libraries or tests/examples will be built!
***

For the case of Withsubpackages, it show:

Disabling forward required SE packages and optional intra-package support that have a dependancy on disabled SE packages TribitsExProj_ENABLE_<TRIBITS_PACKAGE>=OFF ...

-- Setting TribitsExProj_ENABLE_WithSubpackagesA=OFF because WithSubpackagesA has a required library dependence on disabled package SimpleCxx
-- Setting TribitsExProj_ENABLE_WithSubpackagesB=OFF because WithSubpackagesB has a required library dependence on disabled package SimpleCxx
-- Setting TribitsExProj_ENABLE_WithSubpackagesC=OFF because WithSubpackagesC has a required library dependence on disabled package WithSubpackagesA
 ***
 *** NOTE: Setting TribitsExProj_ENABLE_WithSubpackages=OFF which was 'ON' because WithSubpackages has a required library dependence on disabled package WithSubpackagesA but TribitsExProj_DISABLE_ENABLED_FORWARD_DEP_PACKAGES=ON!
 ***

-- Setting TribitsExProj_ENABLE_WrapExternal=OFF because WrapExternal has a required library dependence on disabled package WithSubpackagesA

That is what it should show. But then why is there an attempt to build any targets? The build error shoiwn here shows:

gmake: *** No rule to make target `WithSubpackages_libs'.  Stop.

And we see something similar for the build error with the WrapExternal package here showing:

gmake: *** No rule to make target `WrapExternal_libs'.  Stop.

That is very interesting. So these are global targets that never get defined because the <packageDir>/CMakeLists.txt file that defines them never gets processed because the package never got enabled. Before CMake 3.19, types of global targets that don't directly invoke the compiler or linker (and therefore never got run through the cmake launcher) never never got reported as errors. But now they are.

So this brings up an interesting problem. The outer ctest -S driver has no idea what pacakges are actaully enabled in the inner CMake project due to TribitsExProj_DISABLE_ENABLED_FORWARD_DEP_PACKAGES=ON being set so it does not know that the target WithSubpackages_libs' or WrapExternal_libs' is not even defined.

The purpose of this test was to test the behavior of tribits_ctest_driver() in cases where a configure failure of an upstream package gracefully takes out downstream packages. Now, we could make this test pass again by implementing a hack in TriBITS where we define all of the <packageName>_libs targets even if the packages are not defined. But, that seems like a terrible hack.

Instead, I propose that I change this test to accept this current behavior, document it, and kick the can down the road until a later time. Really no one is using the package-by-package build process anymore because it is too expensive. It is better to just keep everything working and configure, build, and test faster.

@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Aug 12, 2021

I could fix TriBITS to make the test TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg pass again by defining the missing targets <PackageName>_lib but that feels like a hack. I would like to update the test to match the current CMake 3.19+ behavior but then it will fail with versions prior to CMake 3.19 (and I currently have to support CMake 3.17+ with TriBITS).

Here are my options:

  • option-1: Change the test TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg to run the old way with versions of CMake before 3.19.0 and the new way with later versions. But this breaks the use case of the package-by-package mode where a configure or library build failure of an upstream package gracefully takes out downstream packages so they show no errors. And I would have to maintain two versions of this test.

  • option-2: Add an option to the inner TriBITS configure <Project>_DEFINE_MISSING_PACKAGE_LIBS_TARGETS that if set to ON will define <PackageName>_lib targets for all defined top-level packages even if they are not defined. That will make it so that the target <PackageName>_lib is defined so that tribits_ctest_driver() can call it even if it is not defined. This preserves the package-by-package mode of gracefully taking out downstream packages when they get disabled due to disabled required upstream packages. And we only maintain one version of the test.

  • option-3: Just remove this test all together and start giving up on package-by-package mode since no one is really using this mode anyway since it is expensive with lots of reconfigures, rebuilds, and tests running individually for each package.

So, what do I do here? I may need to think about this for a few minutes and come back to this later today.

@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Aug 12, 2021

I think I have decided to go with option-2 above. I will not even bother to document the option <Project>_DEFINE_MISSING_PACKAGE_LIBS_TARGETS as it is just a way for the outer tribits_ctest_driver() code to interact correctly with the inner TriBITS project configure. And it preserves that really cool property of the robust package-by-package mode where if an upstream package fails to configure or build any of its libraries, then it will gracefully disable downstream packages and no configure, build or test failures will show up (and no test will show up).

I will implement that on this PR branch, test locally, and push to this PR branch so it can be tested with the different CMake version to verify. Then if that works out, I will cherry-pick that commit into a new branch and create a new PR to merge this (and undo the disable of this test for the CMake 3.21 GHA builds).

* Removed initial '#' that I used to add (not sure why)

* Fixed some problems in comment text that I happened to notice
….19+ (#363, #394)

I also added another test case for when libs are broken in required upstream
package.

See the detailed comments, especially in the file
TribitsCTestDriverCoreHelpers.cmake.
@bartlettroscoe bartlettroscoe force-pushed the 363-github-actions-tests-show-ctest-3.21-failures branch from ca9aa79 to 3db1690 Compare August 12, 2021 22:05
@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Aug 12, 2021

I just pushed commit 3db1690 which implements option-2 above that fixes the test TriBITS_CTestDriver_PBP_ST_BreakConfigureRequiredPkg and added a new related test TriBITS_CTestDriver_PBP_ST_BreakBuildLibRequiredPkg that passes for of the CMake versions as shown at:

showing:

image

Only the 9 other tests for CMake 3.21 still fail that will be fixed once CMake 3.21.2 is released and the GitHub Actions workflow is updated to use that version.

I will now post a new PR for just the fixes in commit 3db1690 (along with re-enabling this test) so we can get that resolved.

bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this pull request Aug 12, 2021
….19+ (TriBITSPub#363, TriBITSPub#394)

I also added another test case for when libs are broken in required upstream
package.

See the detailed comments, especially in the file
TribitsCTestDriverCoreHelpers.cmake.
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this pull request Aug 12, 2021
…for CMake 3.21 (TriBITSPub#363, TriBITSPub#394)

Now that TriBITS has a workaround for new CMake 3.19 behavior, this test
passes.

The rest of the tests can be enabled once CMake 3.21.2 is released and the
GitHub Actions workflow is updated to use that version (see TriBITSPub#394).
bartlettroscoe added a commit that referenced this pull request Aug 12, 2021
…ckage-cmake-3.19-fix

Fix tribits_ctest_driver() package-by-package mode for CMake 3.19+ (#363, #394)
@bartlettroscoe
Copy link
Member Author

I will now leave this in review, waiting for CMake 3.21.2 to release and then I will test with CMake 3.21.2 and close this out (like an issue).

This should fix problem with accumulation of labels with ctest_test().
…tests-show-ctest-3.21-failures (#363, #394)

I resolved conflicts in the following files:

* cmake/ctest/github_actions/tribits_cmake-3.21.0_makefiles_python-3.8_g++-9_tweaks.cmake:
  * Removed the commented out fixed test
* test/ctest_driver/TribitsExampleProject/CMakeLists.txt:
  * Took what is on 'master'
* tribits/core/package_arch/TribitsGlobalMacros.cmake:
  * Took what is on 'master'
bartlettroscoe added a commit to bartlettroscoe/TriBITS that referenced this pull request Aug 27, 2021
Also remove all of the test disables as cmake 3.21.2 has a patch that fixes
all of the test failures.
bartlettroscoe added a commit that referenced this pull request Aug 30, 2021
Upgrade from cmake 3.21.0 to 3.21.2 (#363, #394)

Re-enables tests that were disabled due to regression in cmake 3.21.0 and 3.21.1.
bartlettroscoe added a commit to bartlettroscoe/Trilinos that referenced this pull request Sep 4, 2021
Brings in numerous refactorings to TriBITS over the last 3 months, but there
should be no breaks in backward compatibility.  Almost every file in TriBITS
is changed due to the lower-casing of command, macro and function names in PR
TriBITSPub/TriBITS#379.  But the main driver for this snapshot is to bring in
the change in PR TriBITSPub/TriBITS#413 that should make it so that Kokkos
INTERFACE_COMPILE_OPTIONS get propagated to downstream targets in TriBITS and
therefore to external customers through installed <Package>Config.cmake files
and IMPORTED targets.  I should have done several snapshots in the last few
months and not done a big snapshot like this (but I have been testing with
Trilinos locally along the way).

Overall, this merge brings in changes from a bunch of TriBITS PRs including
(from most recent):

* TriBITSPub/TriBITS#413: Change internal TriBITS target_link_libraries() to
  PUBLIC (TriBITSPub/TriBITS#299) component: core type: enhancement

* TriBITSPub/TriBITS#410: Upgrade from cmake 3.21.0 to 3.21.2
  (TriBITSPub/TriBITS#363, TriBITSPub/TriBITS#394)

* TriBITSPub/TriBITS#394: DO NOT MERGE: Show TriBITS test failures with CMake
  3.21.0 that don't occur with CMake 3.17.5 (TriBITSPub/TriBITS#363)

* TriBITSPub/TriBITS#409: Add getTestDictStatusField() to handle empty
  'status' field (SESW-383) component: ci_support type: enhancement

* TriBITSPub/TriBITS#408: Deal with spaces in CDash url parts (SESW-383)
  component: ci_support type: enhancement

* TriBITSPub/TriBITS#403: Spelling fixes

* TriBITSPub/TriBITS#407: Move tribits_get_build_url_and_write_to_file() to
  stand-alone module (TriBITSPub/TriBITS#154) component: ctest_driver type:
  enhancement

* TriBITSPub/TriBITS#388: Fixing typos (TriBITSPub/TriBITS#377)

* TriBITSPub/TriBITS#406: Fix tribits_ctest_driver() package-by-package mode
  for CMake 3.19+ (TriBITSPub/TriBITS#363, TriBITSPub/TriBITS#394) component:
  ctest_driver type: bug

* TriBITSPub/TriBITS#401: Improve GitHub Actions and CDash integration
  (TriBITSPub/TriBITS#154) type: enhancement

* TriBITSPub/TriBITS#366: CI: draft action yaml

* TriBITSPub/TriBITS#402: Revert some incorrect uppercase->lowercase changes

* TriBITSPub/TriBITS#387: Build and deploy TriBITS documentation with Sphinx
  (TriBITSPub/TriBITS#386) component: documentation type: enhancement

* TriBITSPub/TriBITS#398: Deal with pr null in not preprending build name
  (TriBITSPub/TriBITS#363) type: bug

* TriBITSPub/TriBITS#396: Send PR results to 'Pull Request' CDash group and
  add PR ID to build names (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#397: Print the ninja path and version
  (TriBITSPub/TriBITS#363)

* TriBITSPub/TriBITS#393: GitHub Actions based testing for TriBITS
  (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#389: TriBITS CI testing with GitHub Actions
  (TriBITSPub/TriBITS#363) type: enhancement

* TriBITSPub/TriBITS#392: Fix broken tests for non-Fortran and CMake 3.21
  builds (TriBITSPub/TriBITS#363) component: core

* TriBITSPub/TriBITS#391: Fix up build_docs.sh for Sphinx doc generation
  (TriBITSPub/TriBITS#386) component: documentation type: enhancement

* TriBITSPub/TriBITS#390: Add test for doc generation and fix usage of Python3
  component: documentation type: bug

* TriBITSPub/TriBITS#385: Replace last few references to
  TribitsDevelopersGuide.html (TriBITSPub/TriBITS#381) component:
  documentation type: enhancement

* TriBITSPub/TriBITS#384: Split TribitsDevelopersGuide.* into
  TribitsUsersGuide.* and TribitsMaintainersGuide.* (TriBITSPub/TriBITS#381)
  component: documentation type: enhancement

* TriBITSPub/TriBITS#383: Remove endfunction(<string>) and endmacro(<string>)
  (TriBITSPub/TriBITS#274, TriBITSPub/TriBITS#382) component: common_tpls
  type: bug

* TriBITSPub/TriBITS#380: More package-arch data-structure documentation
  updates (TriBITSPub/TriBITS#63) component: documentation type: enhancement

* TriBITSPub/TriBITS#379: Convert TriBITS to lower-case CMake command, macro,
  and function names (TriBITSPub/TriBITS#274)

The following files were conflicting where I went with what is on the Trilinos
'develop' branch:

* cmake/tribits/common_tpls/FindTPLBLAS.cmake
* cmake/tribits/common_tpls/FindTPLLAPACK.cmake
* cmake/tribits/common_tpls/FindTPLNetcdf.cmake

(It looks like the above changes never made it back into TriBITS proper.  The
conflicts were due to the case changes in cmake command calls in these files
due to TriBITSPub/TriBITS#379.)

There was also a conflict in the file:

* cmake/tribits/core/installation/TribitsProjectConfigTemplate.cmake.in

I looked at that one carefully and I think that may have been due to fixes on
both sides and then the case changes from TriBITSPub/TriBITS#379.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants