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

Add ignore_submodules to cice #644

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Conversation

mathomp4
Copy link
Member

This PR adds ignore_submodules: true for CICE in GEOSgcm. This enables a new feature of mepo which should prevent mepo status from showing:

cice6                  | (t) geos/v0.0.2 (DH)
   | icepack: modified, not staged

as well as mepo diff from showing a difference as well. We see this because git sees it since git status and git diff know that icepack was/is a submodule of CICE but we are "ignoring" it and not using it...but git doesn't know that.

So with this change, we can register with mepo that cice should run status and diff ignoring submodule changes.

I'll keep draft for now until I can test and ideally @zhaobin74 can test (since I really don't want to mess up his code). I think it's all good in my testing.

Note if you build locally, you want to get mepo v1.51.1 (or just main) to see this working. I've updated the mepo versions at NCCS and NAS so we should be good...

@mathomp4 mathomp4 added the 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) label Aug 25, 2023
@mathomp4 mathomp4 self-assigned this Aug 25, 2023
@mathomp4 mathomp4 marked this pull request as ready for review August 28, 2023 18:20
@mathomp4 mathomp4 requested a review from a team as a code owner August 28, 2023 18:20
@zhaobin74
Copy link
Contributor

@mathomp4, this looks good. But I have a question: currently icepack is ignored by cice (our version geos/develop), see. So git status does not show | icepack: modified, not staged. If mepo uses git, why does it see icepack under cice? Thanks,

@mathomp4
Copy link
Member Author

@mathomp4, this looks good. But I have a question: currently icepack is ignored by cice (our version geos/develop), see. So git status does not show | icepack: modified, not staged. If mepo uses git, why does it see icepack under cice? Thanks,

@zhaobin74 The issue is that "internally" git knows there is or was a submodule there...somehow. Not sure how. The .gitmodules file is empty but I know that's not all that git uses for submodules. There are things baked into the .git/ folder stuff that we don't go near. So if you go to cice6 and run git status:

$ git status
HEAD detached at geos/v0.0.2
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   icepack (new commits)

Submodules changed but not updated:

* icepack 81a7628...8d80ff5 (1):
  > Merge pull request #1 from GEOS-ESM/bugfix/zhaobin74/recompute-enthalpy

no changes added to commit (use "git add" and/or "git commit -a")

Git knows there is/was a submodule there.

All this really does is when using mepo we can "hide" the fact that git is having issues from the end user. Underneath we would now add --ignore-submodules=all to the call:

$ git status --ignore-submodules=all
HEAD detached at geos/v0.0.2
nothing to commit, working tree clean

@zhaobin74
Copy link
Contributor

Hi @mathomp4, sorry my bad. I used alias git s and forgot --ignore-submodules=all was already built into the alias. Yes, git somehow still remembers icepack, I have a vague memory it happened after I merged with the consortium version.

Anyway, this should go in and make mepo status clean as it should.

Thanks.

@sdrabenh sdrabenh merged commit 8b5f4a9 into main Aug 29, 2023
4 checks passed
@sdrabenh sdrabenh deleted the feature/mathomp4/ignore-submodules-cice branch August 29, 2023 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants