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

Change internal flag organization in CMake build to not be global #2053

Merged
merged 5 commits into from
Oct 14, 2024

Conversation

islas
Copy link
Collaborator

@islas islas commented May 28, 2024

TYPE: enhancement

KEYWORDS: cmake, flags, compilation

SOURCE: internal

DESCRIPTION OF CHANGES:
Problem:
The current iteration of the cmake build places all configuration flags in the global properties of the project. While this works when just building WRF, integration with other projects' cmake builds if placed under WRF pollutes their respective build flags.

Solution:
Adjust the layout of the flags to instead carry them in a variable, and prefer using target_* calls for flag usage.

LIST OF MODIFIED FILES:
M CMakeLists.txt
M chem/CMakeLists.txt
M external/CMakeLists.txt
M external/io_adios2/CMakeLists.txt
M external/io_netcdf/CMakeLists.txt
M external/io_netcdfpar/CMakeLists.txt
M external/io_pnetcdf/CMakeLists.txt
M frame/CMakeLists.txt
M main/CMakeLists.txt
M phys/CMakeLists.txt
M tools/CMakeLists.txt

RELEASE NOTE:
Change internal flag organization in CMake build to not be global

@islas islas requested review from a team as code owners May 28, 2024 17:59
@islas islas force-pushed the cmake-restructureFlags branch from e909e57 to 6b9d27e Compare August 6, 2024 02:30
@islas islas requested a review from a team as a code owner August 6, 2024 02:30
@islas
Copy link
Collaborator Author

islas commented Aug 6, 2024

Requires #2056

amstokely
amstokely previously approved these changes Sep 18, 2024
Copy link
Collaborator

@amstokely amstokely left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Configuring compilation settings at the target level instead of the global level is an excellent decision and will definitely improve the maintainability of the CMake build. I approve these changes.

mgduda
mgduda previously approved these changes Oct 12, 2024
@islas islas dismissed stale reviews from mgduda and amstokely via 247cf59 October 12, 2024 00:27
@mgduda mgduda self-requested a review October 14, 2024 20:23
@amstokely amstokely self-requested a review October 14, 2024 20:38
@islas islas merged commit cd8e545 into wrf-model:release-v4.6.1 Oct 14, 2024
1 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants