Skip to content

Add git diff to fleximod_test#317

Merged
jedwards4b merged 3 commits intoESCOMP:cesm3.0-alphabranchfrom
mnlevy1981:ci_for_clean_sandbox
Feb 13, 2025
Merged

Add git diff to fleximod_test#317
jedwards4b merged 3 commits intoESCOMP:cesm3.0-alphabranchfrom
mnlevy1981:ci_for_clean_sandbox

Conversation

@mnlevy1981
Copy link
Copy Markdown
Contributor

Description of changes

If the tags used by git fleximod in the .gitmodules do not match the externals checked in to the various components, then git diff --exit-code will fail. In this case, whichever components are triggering the failure should be updated to ensure .gitmodules and the actual externals match.

Specific notes

Contributors other than yourself, if any:

Fixes: [Github issue #s] And brief description of each issue.

User interface changes?: No

Testing performed (automated tests and/or manual tests):

I verified that git diff --exit-code will return 1 if there are differences in any of the submodules (including differences in submodules of submodules) and 0 if everything is clean; I'm 99% sure this branch will fail the fleximod-test for non-zero return codes.

If the tags used by git fleximod in the .gitmodules do not match the externals
checked in to the various components, then "git diff --exit-code" will fail. In
this case, whichever components are triggering the failure should be updated to
ensure .gitmodules and the actual externals match.
If the git fleximod test fails in 3.7 due to a python incompatibility, it would
be nice to still run the test in 3.x
@jedwards4b
Copy link
Copy Markdown
Contributor

@mnlevy1981 I'm not sure what your intent is here. This test already exists in the github workflows (without using git diff).

@mnlevy1981
Copy link
Copy Markdown
Contributor Author

I'm not sure what your intent is here. This test already exists in the github workflows (without using git diff).

I didn't realize there was already a test in the workflow, but it didn't seem to catch the fact that CTSM's external (FATES) and MOM's external (MOM6) were both out of sync

@samsrabin
Copy link
Copy Markdown
Member

samsrabin commented Feb 10, 2025

git diff --exit-code doesn't really do what's intended here, does it? Wouldn't git diff always show no diffs on a fresh checkout, such as is used by Github workflows? What we need is something that actually compares the hash in each submodule to the hash or tag specified in .gitmodules.

I think that's what happens in the workflow here, which is what Jim's referring to:

$GITHUB_WORKSPACE/bin/git-fleximod test

Doing that in a mismatched CTSM checkout I set up gives error code 3.

@mnlevy1981
Copy link
Copy Markdown
Contributor Author

mnlevy1981 commented Feb 10, 2025

git diff --exit-code doesn't really do what's intended here, does it? Wouldn't git diff always show no diffs on a fresh checkout, such as is used by Github workflows?

@samsrabin In a clean checkout of cesm3_0_beta05, a top level git diff shows

$ git diff
diff --git a/components/mom b/components/mom
--- a/components/mom
+++ b/components/mom
@@ -1 +1 @@
-Subproject commit dbf39d2acaf1deebc81674e777d9af81d084b10d
+Subproject commit dbf39d2acaf1deebc81674e777d9af81d084b10d-dirty

This is due to a mismatch in components/mom/MOM6 (which is an external under components/mom:

$ cd components/mom/
$ git diff
diff --git a/MOM6 b/MOM6
index 8feb162..acadc6e 160000
--- a/MOM6
+++ b/MOM6
@@ -1 +1 @@
-Subproject commit 8feb16278d3dbeb22e4902257d2dcf6560a49e06
+Subproject commit acadc6eca5808b302c5d5dbb2bece8ea05aaf316

8feb162 is dev/ncar_250128, which is what git submodules points two. acadc6e is dev/ncar_241122b, which is what git fleximod is told to checkout.

(I'm not sure what happened to the FATES diff I was seeing in a different sandbox, I swear over the weekend I was also seeing differences in components/clm)

@samsrabin
Copy link
Copy Markdown
Member

Maybe it didn't detect those because they're both marked as only ToplevelRequired?

@mnlevy1981
Copy link
Copy Markdown
Contributor Author

Maybe it didn't detect those because they're both marked as only ToplevelRequired?

Maybe - @jedwards4b was going to look into it this afternoon... it does sound like an issue in git-fleximod test rather than something that requires a new test in CESM's Github Actions. I'm leaving this open for now, but I suspect it will get closed without merging at some point.

This should pass CI, since it will get correct MOM6 external
@jedwards4b jedwards4b merged commit 95ded9f into ESCOMP:cesm3.0-alphabranch Feb 13, 2025
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