Complete deprecation work (#429) #552
Merged
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
Will help in future refactorings. I mostly just added this as a precursor to token-replace.py.
I will use this tool to replace deprecated functions/macros.
* Update the deprecation message to mention the cache var
TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE and how to set it to remove
deprecation warnings.
* Add non-cache var TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE_DEFAULT so that a
TriBITS project can provide its own default for cache var
TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE.
* Add documentation for the cache var TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE
in the TriBITS build reference and developers guides.
* Fix TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE_ALL_VALID_VALUES to pull in
${TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE_VALUES_THAT_CALL_MESSAGE} and
${TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE_VALUES_THAT_DONT_CALL_MESSAGE}
instead of pulling in
${TRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE_VALUES_THAT_CALL_MESSAGE} twice.
(If user would have tried to set
-DTRIBITS_HANDLE_TRIBITS_DEPRECATED_CODE=IGNORE, it would have errored out
of the configure.)
This allows hiding the override of the raw CMake command include_directories() and stop calling _include_directories() in tribits_include_directories() when TRIBITS_HIDE_DEPRECATED_INCLUDE_DIRECTORIES_OVERRIDE=TRUE. I also updated some documentation and did some reformatting to fit in 90 char width.
…_TESTING ...) (#429) This is needed to test tribits_include_directories(REQUIRED_DURING_INSTALLATION_TESTING ...) and why it is needed. There was no example in TriBITS that showed the usage of this feature. This is also a nice example for users (even if it makes SimpleCxx a little more complex).
…429) This shows that the -I for the lib source tree is being added incorrectly. To get this error to show, I set TRIBITS_HIDE_DEPRECATED_INCLUDE_DIRECTORIES_OVERRIDE=TRUE for TribitsExampleProject. Other changes: * Added subdir 'BUILD' beside copied TribitsExampleProject source tree. (It is not good to intermingle the source and build trees at all.) * Removed redundant ALWAYS_FAIL_ON_NONZERO_RETURN: When not specifying some other pass/fail criteria, a non-zero return code means fail.
The file CMakeOverrides.cmake contained tribits_include_directories() and the (terrible) override of include_directories(). It is unclear from TriBITS git history when the mdoule TribitsIncludeDirectories.cmake and why CMakeOverrides.cmake was not deleted but there are no current includes of the module "CMakeOverrides" in TriBITS anywhere. It appears that the include of the module CMakeOverrides.cmake was removed 9 years ago and replaced with the include of TribitsIncludeDirectories.cmake way back in the commit: cd8e3e8 "Moving TriBITS-specific funtion into tribits/package_arch/" Author: Roscoe A. Bartlett <[email protected]> Date: Mon Mar 10 16:21:51 2014 -0400 (9 years ago) So nice to get rid of dead code!
This makes the code work correctly when TRIBITS_HIDE_DEPRECATED_INCLUDE_DIRECTORIES_OVERRIDE=TRUE is set (which I also set in the project TribitsExampleProject2). This change was made by running the script replace_include_directories_r.sh in the subdirs: test/core/ tribits/examples/ This change makes the install test pass again.
…ity (#429) The only remaining deprecation warnings are for tests that run an grep the STDOUT for the deprecation warning text and test TriBITS_CTestDriver_TribitsExMetaProj_version_date that pulls in older versions of the repos (that still use include_directories()). In a later commit, I will update the snapshots of the TriBITS example repos to update the test TriBITS_CTestDriver_TribitsExMetaProj_version_date to make these deprecation warnings go away.
|
Hello @KyleFromKitware, can you please give this PR a quick review? I would like to get this PR merged as part of the TriBITS update of Trilinos in trilinos/Trilinos#11380. |
rabartlett1972
approved these changes
Dec 19, 2022
There was a problem hiding this comment.
I will go ahead and allow this to merge and set up a post-merge review by @KyleFromKitware when he gets back from vacation.
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
There was a good bit of work left over from #429 that I noticed after I updated the snapshot of TriBITS in Trilinos and saw a bunch of deprecation warnings. I also saw that TriBITS examples were using deprecated code as well.
The main commit in this PR is 69fb01c which updates the documentation and fixes some bugs.
This PR also does some other things (see the individual commits for details).
Instructions for reviewers
Please review the commits one-by-one. That will make the review the easiest to do and provide the best feedback.