Fix CMake path semantics and error handling in ocean.cmake#8354
Open
Copilot wants to merge 3 commits into
Open
Fix CMake path semantics and error handling in ocean.cmake#8354Copilot wants to merge 3 commits into
Copilot wants to merge 3 commits into
Conversation
Agent-Logs-Url: https://github.com/E3SM-Project/E3SM/sessions/9ac9f1a1-ea34-4fab-8f6b-0bb4077597de Co-authored-by: xylar <4179064+xylar@users.noreply.github.com>
xylar
added
MPAS-ocean
Copilot
AI
changed the title
[WIP] Fix wrong cmake semantics for ocean MARBL
Fix CMake path semantics for MARBL submodule existence check
May 3, 2026
Contributor
|
@jonbob, I did a little hunting around https://github.com/E3SM-Project/E3SM/blob/c24b628c1d50718429284e76af32bc7ef20b9f59/cime_config/tests.py and didn't see a test for OECO to use to test MARBL. Any recommendations? I'm also happy to review by inspection if you think that's sufficient. I'm also not aware of GOTM being used in E3SM at all so I guess there won't be any tests for that. |
mahf708
requested changes
May 3, 2026
Contributor
mahf708
left a comment
mahf708
left a comment
There was a problem hiding this comment.
@copilot a few more potential issues in this file worth fixing while at it --- please review the entire file.
- Line 6 uses a bare relative path; is that correct?
- Like MARBL, GOTM blocks near line 203 need fixing; let's fix as well?
- FATAL isn't valid cmake, do we want FATAL_ERROR here?
…AL, gotm include path Agent-Logs-Url: https://github.com/E3SM-Project/E3SM/sessions/5e3f90f5-3137-4c10-9ccf-ee1efd2096f8 Co-authored-by: mahf708 <122953255+mahf708@users.noreply.github.com>
Contributor
Author
Fixed all three issues in e03f109:
|
Copilot
AI
changed the title
Fix CMake path semantics for MARBL submodule existence check
Fix CMake path semantics and error handling in ocean.cmake
May 3, 2026
mahf708
approved these changes
May 3, 2026
mahf708
approved these changes
May 3, 2026
Contributor
|
@mahf708, great, thanks for pointing out the other issues. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
Several CMake issues in
components/mpas-ocean/src/ocean.cmakewere causing unreliable submodule existence checks and silent no-op error messages.Changes
${CMAKE_CURRENT_SOURCE_DIR}so theNOT EXISTScheck resolves against the source tree rather than the working directory. Changedmessage(FATAL ...)tomessage(FATAL_ERROR ...)—FATALis not a valid CMake message mode and was a silent no-op.${CMAKE_CURRENT_SOURCE_DIR}/prefix to theNOT EXISTSpath and changedmessage(FATAL ...)tomessage(FATAL_ERROR ...).core_ocean/gotm/includeto${CMAKE_CURRENT_SOURCE_DIR}/core_ocean/gotm/includeso the compiler include path is absolute and reliable.All fixes are consistent with the pattern already used by the CVMix, BGC, and PPR submodule checks in the same file.