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

Autoconf: External FMS build configuration #362

Merged
merged 1 commit into from
May 28, 2023

Conversation

marshallward
Copy link
Member

This patch modifies the ac/deps Makefile used to build the FMS depedency. The autoconf compilation is now done entirely outside of the ac/deps/fms/src directory. This keeps the FMS checkout unchanged and allows us to better track any development changes in that library during development.

The .testing/Makefile was also modified to use existing rules in deps/Makefile rather than duplicating them.

@codecov
Copy link

codecov bot commented May 8, 2023

Codecov Report

Merging #362 (9d33f17) into dev/gfdl (50d8bda) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 9d33f17 differs from pull request most recent head 3a16927. Consider uploading reports for the commit 3a16927 to get more accurate results

@@             Coverage Diff              @@
##           dev/gfdl     #362      +/-   ##
============================================
- Coverage     38.13%   38.13%   -0.01%     
============================================
  Files           269      269              
  Lines         75743    75743              
  Branches      13926    13926              
============================================
- Hits          28883    28882       -1     
- Misses        41660    41662       +2     
+ Partials       5200     5199       -1     

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@marshallward marshallward changed the title Autoconf: Safer FMS build Autoconf: External FMS build configuration May 10, 2023
@marshallward
Copy link
Member Author

My simple edit of the commit's subject may have exposed some race conditions in the Makefiles (they are popping up in my personal repo, not on this PR). Looking into it...

@marshallward
Copy link
Member Author

A more explicit specification of the m4 dependency seems to have helped.

It was known to be an unhandled dependency, but I had thought other dependencies took care of it. Shifting work over to the copy of ac/deps/Makefile may have broken some of those assumptions.

This patch modifies the `ac/deps` Makefile used to build the FMS
depedency.  The autoconf compilation is now done entirely outside of the
`ac/deps/fms/src` directory.  This keeps the FMS checkout unchanged and
allows us to better track any development changes in that library during
development.

The .testing/Makefile was also modified to use existing rules in
deps/Makefile rather than duplicating them.

Dependency of the m4 directory is also now more explicit (albeit still
somewhat incomplete).
Copy link
Member

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

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

These changes look sensible to me, and they have passed pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/19308.

@Hallberg-NOAA Hallberg-NOAA merged commit 0fa10ad into NOAA-GFDL:dev/gfdl May 28, 2023
@marshallward marshallward deleted the deps_no_fms_diffs branch May 8, 2024 14:57
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.

2 participants