CMake Modernization: consolidate add library macros, patch targets, parallel options #20040
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
Added `DO_PARALLEL` option to visit_add_libary, indicating the library should be considered parallel. Added `SKIP_INSTALL` option to visit_add_library indicating the library isn't to be installed. Added `visit_patch_parallel_target` which adds all the parallel flags, defines, includes, libs, etc to a parallel target. Added `visit_patch_target` to add SOURCES, INCLUDES, DEFINES, etc to an already created target.
…_PARALLEL. Use visit_patch_target where feasible.
It now calls visit_add_library, then visit_patch_parallel_target.
…gas/consolidate_add_library_macros
JustinPrivitera
approved these changes
Jan 6, 2025
| set_target_properties(engine_par PROPERTIES OUTPUT_NAME enginelib_par) | ||
| visit_patch_target(NAME engine_par OUTPUT_NAME enginelib_par) | ||
| else() | ||
| # This keeps comm's exceptions visible when using -fvisibility=hidden |
There was a problem hiding this comment.
Just a curiosity...why is keeping comm's exceptions visible needed/important?
There was a problem hiding this comment.
I am not sure, it was added when we moved to using CMake for the build system. Perhaps there was similar in the autoconf way of doing things way back when?
markcmiller86
approved these changes
Jan 6, 2025
cyrush
approved these changes
Jan 6, 2025
biagas
merged commit 7a76dd0
into
task/biagas/cmake_modernization_with_blt
1 of 2 checks passed
# for free
to join this conversation on GitHub.
Already have an account?
# 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
Started down the road of updating creating visit_add_executable macro and updating CMakeLists as necessary, ran into a lot of duplication in regards to parallel settings, so decided to try to consolidate all the logic a bit better. This is the result of that effort. The actual executable changes will come later.
Added visit_patch_target to add additional settings to a target.
Added visit_patch_parallel_target which adds all the parallel flags/includes/links to a parallel target. Can be used for a library or an executable.
Modified visit_add_parallel_library to call visit_patch_parallel_target.
Modified the current PARALLEL settings to accommodate changes needed for the new. The old-style will eventually disappear.
Updated some libraries to utilize visit_patch_target.
Type of change
[ ] Bug fix[ ] New feature[ ] Documentation updateCMake modernization.
Checklist:
[ ] I have updated the release notes.[ ] I have made corresponding changes to the documentation.[ ] I have added debugging support to my changes.[ ] I have added tests that prove my fix is effective or that my feature works.[ ] I have confirmed new and existing unit tests pass locally with my changes.[ ] I have added new baselines for any new tests to the repo.[ ] I have NOT made any changes to protocol or public interfaces in an RC branch.