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

update esmf bld to use official esmf action #239

Merged
merged 8 commits into from
Aug 17, 2023

Conversation

jedwards4b
Copy link
Contributor

Description of changes

Updates the esmf build action to use one supported by the esmf developers.

Specific notes

Contributors other than yourself, if any:

CDEPS Issues Fixed (include github issue #):

Are there dependencies on other component PRs (if so list):

Are changes expected to change answers (bfb, different to roundoff, more substantial):

Any User Interface Changes (namelist or namelist defaults changes):

Testing performed (e.g. aux_cdeps, CESM prealpha, etc):

Hashes used for testing:

@jedwards4b jedwards4b self-assigned this Aug 16, 2023
@billsacks
Copy link
Member

Thanks for doing this, @jedwards4b ! It would be best to get a review from @danrosen25 . Dan, can you review these changes from Jim?

Copy link
Collaborator

@danrosen25 danrosen25 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 work but I have two suggestions. Also, I learned today that my ESMF cache checks are VERY thorough. I found a bug on one of the GitHub runners. The documentation says that Ubuntu installs cmake 3.27.1 but one of the runners has cmake 3.27.2 installed and this caused a cache miss and rebuild/reinstallation of ESMF. I'm thinking about adding a simplified check that doesn't check the system for changes. What are your thoughts?

And sorry, the documentation should have mentioned that this sets ESMFMKFILE for you.

.github/workflows/extbuild.yml Outdated Show resolved Hide resolved
.github/workflows/extbuild.yml Outdated Show resolved Hide resolved
@jedwards4b jedwards4b requested review from danrosen25 and removed request for billsacks August 17, 2023 16:32
@jedwards4b
Copy link
Contributor Author

@danrosen25 @billsacks can one of you approve this so that it can be merged.

Copy link
Collaborator

@danrosen25 danrosen25 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 good.

One note. I'm going to push an update to the v1 tag (like I said it's a pre-release). You won't have to change your code but it will cause an esmf installation cache miss once the next time the workflow runs. The reason is that I experienced a cache miss due to a different version of cmake on runner fv-az627-98. So this is a bug from GitHub and I removed make and cmake from the esmf-cache-key that I'm generating.

@jedwards4b jedwards4b merged commit f7e657e into ESCOMP:main Aug 17, 2023
1 check passed
@jedwards4b jedwards4b deleted the github_esmf_bld_update branch August 17, 2023 19:29
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