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.

Sign up for GitHub

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

Jump to bottom

Several improvements to the Cmake #77

Merged
merged 5 commits into from
Oct 22, 2023
Merged

Several improvements to the Cmake #77

merged 5 commits into from
Oct 22, 2023

Conversation

micaeljtoliveira
Copy link
Contributor

@micaeljtoliveira micaeljtoliveira commented Oct 17, 2023 •
edited
Loading

Add the possibility to install the OM3 executables using CMake.

Also reorganized the CMakeLists.txt files. Among other things:

  • adopt a more uniform style
  • try to only declare link libraries as public if really necessary
  • declare compile definitions as private and use as much as possible target_compile_definitions over add_compile_definitions
  • list sources in the CMakeLists files
  • split share library into share and timing in order to make it clear where timing is used/needed
  • declare the linker language for our executables
  • install executables with CMake

All of these changes are aimed at making it easier to use the OM3 build system to build and install the OM3 executables and some of the components through Spack.

@micaeljtoliveira
Reorganize main CMakeLists.txt file into sections.
5e9cb97
@micaeljtoliveira
Prepend OM3_ to all CMake targets and options.
6ade4a8
@micaeljtoliveira
Reorganize CMakeLists:
c564d2b 
- adopt a more uniform style
- try to only declare link libraries as public if really necessary
- declare compile definitions as private and use as much as possible target_compile_definitions over add_compile_definitions
- list sources in the CMakeLists files
- split share library into share and timing in order to make it clear where timing is used/needed
- declare the linker language for our executables
@micaeljtoliveira micaeljtoliveira self-assigned this Oct 17, 2023
@micaeljtoliveira micaeljtoliveira added the build system Build system label Oct 17, 2023
@micaeljtoliveira micaeljtoliveira added this to the 0.3.x milestone Oct 17, 2023
aekiss
aekiss approved these changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@aekiss aekiss left a comment

Choose a reason for hiding this comment

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

LGTM on a quick look, but I don't know enough about CMake to give a proper review

@micaeljtoliveira
Copy link
Contributor Author

@aekiss Thanks for having a look. I was thinking maybe we should make the compilation of the WW3 utilities optional. What do you think?

@aekiss
Copy link
Contributor

aekiss commented Oct 18, 2023

I guess it could be made optional but enabled by default? It seems harmless to build those utilities, and may be handy to have versions that match ww3. @ezhilsabareesh8 what do you think?

@ezhilsabareesh8
Copy link
Contributor

Hi @aekiss, I agree with your suggestion. Enabling the WW3 utilities by default offers the flexibility to generate inputs and outputs, which can be quite handy.

In particular, the inclusion of the ww3_ounf executable, as seen in the current build of access-OM3, is necessary for generating the output NetCDF files from the out_grd WW3 binary file. Currently, the MOM6-CICE6-WW3 configuration doesn't provide any output files, and I'm working to rectify this issue.

@micaeljtoliveira
Copy link
Contributor Author

@aekiss No problems in having this on by default. This is more about giving users full control of what and how to build and install things.

In particular, the inclusion of the ww3_ounf executable, as seen in the current build of access-OM3, is necessary for generating the output NetCDF files from the out_grd WW3 binary file. Currently, the MOM6-CICE6-WW3 configuration doesn't provide any output files, and I'm working to rectify this issue.

Hmm, that's good to know. If that's the case, then maybe we should always build the utilities.

@micaeljtoliveira
Add installation of OM3 executables and WW3 utilities to CMakeLists. …
27f9590 
…Update build script to use "cmake --install" and rename all executables.
@micaeljtoliveira
Copy link
Contributor Author

I've added the WW3 binaries to the installation and updated the build script accordingly.

@micaeljtoliveira micaeljtoliveira mentioned this pull request Oct 22, 2023
Closed
@micaeljtoliveira micaeljtoliveira merged commit 611ef3b into main Oct 22, 2023
2 checks passed
@micaeljtoliveira micaeljtoliveira deleted the cmake_install branch October 23, 2023 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aekiss aekiss aekiss approved these changes

Assignees

@micaeljtoliveira micaeljtoliveira

Labels
build system Build system
Projects
None yet
Milestone
0.3.x
Development

Successfully merging this pull request may close these issues.
3 participants
@micaeljtoliveira @aekiss @ezhilsabareesh8