-
Notifications
You must be signed in to change notification settings - Fork 45
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
Convert TriBITS to lower-case CMake command, macro, and function names (#274) #379
Merged
bartlettroscoe
merged 22 commits into
TriBITSPub:master
from
bartlettroscoe:274-lower-case-cmake
Jun 18, 2021
Merged
Convert TriBITS to lower-case CMake command, macro, and function names (#274) #379
bartlettroscoe
merged 22 commits into
TriBITSPub:master
from
bartlettroscoe:274-lower-case-cmake
Jun 18, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will be used to refactor all TriBITS code and documentation to lower case command names.
See the updated documentation and unit tests for details.
Turns out it was required to have MACRO( and FUNCTION( in upper-case for any macros and functions that were documented. Now that is not the case.
Here I just ran the tool lower-case-cmake-tree.sh. I will fix any issues in future commits. This version fails 97 tests locally with the way I have the tests configured.
…_DEPRECATED_MSG (TriBITSPub#73) This is the problem with the lower_case_cmake.py tool. If there is non-CMake code in these files, then they will get treated like CMake code and the pattern '<identifier>(' will get lower-cased.
…riBITSPub#274, TriBITSPub#200) As part of this, I also fixed how the error messages were formatted. Function and macro names should have () after them and I fixed other formatting problems.
We want this upper case for testing purposes.
Not really related to TriBITSPub#274 but I noticed these things when looking at the document.
This is a case of C code being listed in a *.cmake file and getting made lower-case by accident. NOTE: There is not test in TriBITS for this code. This code should not be in TriBITS, it should be in Trilinos. That needs to be a refactoring done in the future to clear out TriBITS.
This is a case of C code being listed in a *.cmake file and getting made lower-case by accident. NOTE: There is not test in TriBITS for this code. This code should not be in TriBITS, it should be in Trilinos. That needs to be a refactoring done in the future to clear out TriBITS.
bartlettroscoe
force-pushed
the
274-lower-case-cmake
branch
from
June 18, 2021 03:25
3f6cd81
to
df8e98c
Compare
I tested this with Trilinos with an ATDM Trilinos 'sems-rhel7' configuration on my machine 'crf450' and it built and gave test results:
(NOTE: This is using the flaky gather method for the build-stats wrapper that will be fixed by Trilinos PR trilinos/Trilinos#8638. This failure is unrelated to this issue.) TEST DETAILS: (click to expand).
|
bartlettroscoe
added a commit
to bartlettroscoe/TriBITS
that referenced
this pull request
Jun 23, 2021
…riBITSPub#382) This is a follow-on from the lower-casing in TriBITSPub#274 with PR TriBITSPub#379. It lower-cased the function and macro names at the opening macro(<name> ...) and function(<name> ...) but not the endmacro(<name> ...) and endfunction(<name> ...) calls. (I did not realize the TriBITS had any of those still.) Turns out that when the text in the opening macro(<text>) and function(<text>) does not exactly match the endmacro(<text>) and endfunction(<text>), you get a nasty warning message: A logical block opening on the line ... closes on the line ... with mis-matching arguments. (see TriBITSPub#382). The TriBITS test suite does not actually run any of this code so this was never seen prior to the creation of TriBITSPub#382. Therefore, I don't know that this solves the problem but I strongly suspect that it will. (I will run a test with an ATDM Trilinos build that should trigger this code.)
bartlettroscoe
added a commit
to bartlettroscoe/TriBITS
that referenced
this pull request
Jun 23, 2021
…riBITSPub#382) This is a follow-on from the lower-casing in TriBITSPub#274 with PR TriBITSPub#379. It lower-cased the function and macro names at the opening macro(<name> ...) and function(<name> ...) but not the endmacro(<name> ...) and endfunction(<name> ...) calls. (I did not realize the TriBITS had any of those still.) Turns out that when the text in the opening macro(<text>) and function(<text>) does not exactly match the endmacro(<text>) and endfunction(<text>), you get a nasty warning message: A logical block opening on the line ... closes on the line ... with mis-matching arguments. (see TriBITSPub#382). The TriBITS test suite does not actually run any of this code so this was never seen prior to the creation of TriBITSPub#382. Therefore, I don't know that this solves the problem but I strongly suspect that it will. (I will run a test with an ATDM Trilinos build that should trigger this code.)
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new tool
tribits/python_utils/lower_case_cmake.py
and drivertribits/refactoring/lower-case-cmake-tree.sh
that I ran on all of TriBITS (which completes #274). I had to fix several things to get the TriBITS test suite to pass after this but this looks pretty good.I tested this locally with the TriBITS test suite and I am also testing this locally against Trilinos using the ATDM Trilinos 'sems-rhel7' configuration with 'gnu-shared-dbg'. (There is some code that got broken that is not tested in TriBITS!)