From a7d8b4e3e7e0661b98ba03dfc3808b84c6ec6ff1 Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Mon, 1 Jul 2024 17:06:50 -0600
Subject: [PATCH 1/9] all pytests passing

---
 git_fleximod/git_fleximod.py   | 109 +++++++++++++++++++++------------
 git_fleximod/gitmodules.py     |   2 +-
 tests/conftest.py              |  16 ++++-
 tests/test_e_complex_update.py |  68 ++++++++++++++++++++
 4 files changed, 152 insertions(+), 43 deletions(-)
 create mode 100644 tests/test_e_complex_update.py

diff --git a/git_fleximod/git_fleximod.py b/git_fleximod/git_fleximod.py
index ed24c4e75..69da054d4 100755
--- a/git_fleximod/git_fleximod.py
+++ b/git_fleximod/git_fleximod.py
@@ -20,7 +20,7 @@
 
 
 def fxrequired_allowed_values():
-    return ["ToplevelRequired", "ToplevelOptional", "AlwaysRequired", "AlwaysOptional"]
+    return ["ToplevelRequired", "ToplevelOptional", "AlwaysRequired", "AlwaysOptional", "TopLevelRequired", "TopLevelOptional"]
 
 
 def commandline_arguments(args=None):
@@ -33,14 +33,9 @@ def commandline_arguments(args=None):
 
     # explicitly listing a component overrides the optional flag
     if options.optional or options.components:
-        fxrequired = [
-            "ToplevelRequired",
-            "ToplevelOptional",
-            "AlwaysRequired",
-            "AlwaysOptional",
-        ]
+        fxrequired = fxrequired_allowed_values()
     else:
-        fxrequired = ["ToplevelRequired", "AlwaysRequired"]
+        fxrequired = ["ToplevelRequired", "AlwaysRequired", "TopLevelRequired"]
 
     action = options.action
     if not action:
@@ -154,6 +149,8 @@ def submodule_sparse_checkout(root_dir, name, url, path, sparsefile, tag="master
 
     if os.path.isdir(os.path.join(root_dir, path, ".git")):
         with utils.pushd(sprep_repo):
+            if os.path.isdir(os.path.join(topgit,".git")):
+                shutil.rmtree(os.path.join(topgit,".git"))
             shutil.move(".git", topgit)
             with open(".git", "w") as f:
                 f.write("gitdir: " + os.path.relpath(topgit))
@@ -166,7 +163,9 @@ def submodule_sparse_checkout(root_dir, name, url, path, sparsefile, tag="master
             return
 
         with utils.pushd(sprep_repo):
-            shutil.copy(sparsefile, gitsparse)
+            if os.path.isfile(sparsefile):
+                shutil.copy(sparsefile, gitsparse)
+                
 
     # Finally checkout the repo
     sprepo_git.git_operation("fetch", "origin", "--tags")
@@ -202,9 +201,9 @@ def single_submodule_checkout(
     # if url is provided update to the new url
     tmpurl = None
     repo_exists = False
-    if os.path.exists(os.path.join(repodir, ".git")):
-        logger.info("Submodule {} already checked out".format(name))
-        repo_exists = True
+#    if os.path.exists(os.path.join(repodir, ".git")):
+#        logger.info("Submodule {} already checked out".format(name))
+#        repo_exists = True
     # Look for a .gitmodules file in the newly checkedout repo
     if not repo_exists and url:
         # ssh urls cause problems for those who dont have git accounts with ssh keys defined
@@ -244,10 +243,13 @@ def single_submodule_checkout(
 
     if not repo_exists or not tmpurl:
         git.git_operation("submodule", "update", "--init", "--", path)
-            
+    if tag and not optional:        
+        smgit = GitInterface(repodir, logger)
+        smgit.git_operation("checkout", tag)
+
     if os.path.exists(os.path.join(repodir, ".gitmodules")):
         # recursively handle this checkout
-        print(f"Recursively checking out submodules of {name}")
+        print(f"Recursively checking out submodules of {name} {optional}")
         gitmodules = GitModules(logger, confpath=repodir)
         requiredlist = ["AlwaysRequired"]
         if optional:
@@ -288,7 +290,7 @@ def submodules_status(gitmodules, root_dir, toplevel=False):
         tag = gitmodules.get(name, "fxtag")
         url = gitmodules.get(name, "url")
         required = gitmodules.get(name, "fxrequired")
-        level = required and "Toplevel" in required
+        level = required and required.startswith("Top")
         if not path:
             utils.fatal_error("No path found in .gitmodules for {}".format(name))
         newpath = os.path.join(root_dir, path)
@@ -299,6 +301,7 @@ def submodules_status(gitmodules, root_dir, toplevel=False):
             url = url.replace("git@github.com:", "https://github.com/")
             tags = rootgit.git_operation("ls-remote", "--tags", url)
             result = rootgit.git_operation("submodule","status",newpath).split()
+#            print(f"newpath {newpath} result {result}")
             ahash = None
             if result:
                 ahash = result[0][1:]
@@ -318,8 +321,12 @@ def submodules_status(gitmodules, root_dir, toplevel=False):
                 if hhash and atag:
                     break
             optional = " (optional)" if required and "Optional" in required else ""
+#            print(f"tag is {tag} ahash is {ahash} {hhash} {atag} {newpath} {required} {level}")
+            if required and not required.startswith("Always") and not level:
+                continue
+
             if tag and (ahash == hhash or atag == tag):
-                print(f"e {name:>20} not checked out, aligned at tag {tag}{optional}")
+                print(f"e {name:>20} not checked out, aligned at tag {tag}{optional} {level} {required}")
             elif tag:
                 ahash = rootgit.git_operation(
                     "submodule", "status", "{}".format(path)
@@ -341,15 +348,19 @@ def submodules_status(gitmodules, root_dir, toplevel=False):
                 atag = git.git_operation("describe", "--tags", "--always").rstrip()
                 ahash =  git.git_operation("rev-list", "HEAD").partition("\n")[0]
                 rurl = git.git_operation("ls-remote","--get-url").rstrip()
+                recurse = False
                 if rurl != url:
                     remote = add_remote(git, url)
                     git.git_operation("fetch", remote)
                 if tag and atag == tag:
                     print(f"  {name:>20} at tag {tag}")
+                    recurse = True
                 elif tag and ahash[: len(tag)] == tag:
                     print(f"  {name:>20} at hash {ahash}")
+                    recurse = True
                 elif atag == ahash:
                     print(f"  {name:>20} at hash {ahash}")
+                    recurse = True
                 elif tag:
                     print(
                         f"s {name:>20} {atag} {ahash} is out of sync with .gitmodules {tag}"
@@ -361,18 +372,33 @@ def submodules_status(gitmodules, root_dir, toplevel=False):
                         f"e {name:>20} has no fxtag defined in .gitmodules, module at {atag}"
                     )
                     testfails += 1
-
+                    
                 status = git.git_operation("status", "--ignore-submodules", "-uno")
                 if "nothing to commit" not in status:
                     localmods = localmods + 1
                     print("M" + textwrap.indent(status, "                      "))
-
+                if recurse and os.path.exists(os.path.join(newpath, ".gitmodules")):
+                    submodules = GitModules(
+                        logger,
+                        confpath=newpath,
+                    )
+                    nf, lm, nu = submodules_status(submodules, newpath)
+                    testfails += nf
+                    localmods += lm
+                    needsupdate += nu
+                
+                    
     return testfails, localmods, needsupdate
 
 
+def git_toplevelroot(root_dir, logger):
+    rgit = GitInterface(root_dir, logger)
+    superroot = rgit.git_operation("rev-parse", "--show-superproject-working-tree")
+    return superroot
+
+
 def submodules_update(gitmodules, root_dir, requiredlist, force):
     _, localmods, needsupdate = submodules_status(gitmodules, root_dir)
-
     if localmods and not force:
         local_mods_output()
         return
@@ -383,27 +409,26 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
         fxtag = gitmodules.get(name, "fxtag")
         path = gitmodules.get(name, "path")
         url = gitmodules.get(name, "url")
-        logger.info(
-            "name={} path={} url={} fxtag={} requiredlist={} ".format(
-                name, os.path.join(root_dir, path), url, fxtag, requiredlist
-            )
-        )
 
         fxrequired = gitmodules.get(name, "fxrequired")
-        assert fxrequired in fxrequired_allowed_values()
-        rgit = GitInterface(root_dir, logger)
-        superroot = rgit.git_operation("rev-parse", "--show-superproject-working-tree")
+        if fxrequired:
+            allowedvalues = fxrequired_allowed_values()
+            assert fxrequired in allowedvalues
 
-        fxsparse = gitmodules.get(name, "fxsparse")
 
+        fxsparse = gitmodules.get(name, "fxsparse")
+        superroot = git_toplevelroot(root_dir, logger)
+                                     
         if (
             fxrequired
-            and (superroot and "Toplevel" in fxrequired)
-            or fxrequired not in requiredlist
+            and ((superroot and "Toplevel" in fxrequired)
+            or fxrequired not in requiredlist)
         ):
-            if "ToplevelOptional" == fxrequired:
-                print("Skipping optional component {}".format(name))
-            continue
+            if "Optional" in fxrequired and "Optional" not in requiredlist:
+                if fxrequired.startswith("Always"):
+                    print(f"Skipping optional component {name:>20}")
+                continue
+        
         if fxsparse:
             logger.debug(
                 "Callng submodule_sparse_checkout({}, {}, {}, {}, {}, {}".format(
@@ -417,7 +442,7 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
                     root_dir, name, path, url
                 )
             )
-
+            print(f"fxrequired {fxrequired} requiredlist {requiredlist}")
             single_submodule_checkout(
                 root_dir,
                 name,
@@ -425,7 +450,7 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
                 url=url,
                 tag=fxtag,
                 force=force,
-                optional=("AlwaysOptional" in requiredlist),
+                optional="AlwaysOptional" in requiredlist 
             )
 
         if os.path.exists(os.path.join(path, ".git")):
@@ -448,6 +473,8 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
                         print(f"{name:>20} updated to {fxtag}")
                     except Exception as error:
                         print(error)
+                    
+
                 elif not fxtag:
                     print(f"No fxtag found for submodule {name:>20}")
                 else:
@@ -455,7 +482,6 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
 
 
 
-
 def local_mods_output():
     text = """\
     The submodules labeled with 'M' above are not in a clean state.
@@ -469,7 +495,6 @@ def local_mods_output():
 """
     print(text)
 
-
 # checkout is done by update if required so this function may be depricated
 def submodules_checkout(gitmodules, root_dir, requiredlist, force=False):
     """
@@ -498,10 +523,12 @@ def submodules_checkout(gitmodules, root_dir, requiredlist, force=False):
         fxtag = gitmodules.get(name, "fxtag")
         path = gitmodules.get(name, "path")
         url = gitmodules.get(name, "url")
+        superroot = git_toplevelroot(root_dir, logger)
         if fxrequired and fxrequired not in requiredlist:
-            if "Optional" in fxrequired:
-                print("Skipping optional component {}".format(name))
+            if fxrequired == "AlwaysOptional" or (superroot == root_dir):
+                print(f"Skipping optional component {name:>20} {requiredlist}")
             continue
+            
 
         if fxsparse:
             logger.debug(
@@ -521,7 +548,9 @@ def submodules_checkout(gitmodules, root_dir, requiredlist, force=False):
                 url=url,
                 tag=fxtag,
                 force=force,
-                optional="AlwaysOptional" in requiredlist,
+                optional="AlwaysOptional" == fxrequired \
+                or "ToplevelOptional" == fxrequired \
+                or "TopLevelOptional" == fxrequired
             )
 
 
diff --git a/git_fleximod/gitmodules.py b/git_fleximod/gitmodules.py
index 7e4e05394..cf8b350dd 100644
--- a/git_fleximod/gitmodules.py
+++ b/git_fleximod/gitmodules.py
@@ -1,4 +1,4 @@
-import shutil
+import shutil, os
 from pathlib import Path
 from configparser import RawConfigParser, ConfigParser
 from .lstripreader import LstripReader
diff --git a/tests/conftest.py b/tests/conftest.py
index 65ee85d23..81edbe713 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -119,8 +119,20 @@ def complex_repo(tmp_path, logger):
     str_path = str(test_dir)
     gitp = GitInterface(str_path, logger)
     gitp.git_operation("remote", "add", "origin", "https://github.com/jedwards4b/fleximod-test2")
-    gitp.git_operation("fetch", "origin", "main")
-    gitp.git_operation("checkout", "main")
+    gitp.git_operation("fetch", "origin")
+    gitp.git_operation("checkout", "v0.0.1")
+    return test_dir
+
+@pytest.fixture
+def complex_update(tmp_path, logger):
+    test_dir = tmp_path / "testcomplex"
+    test_dir.mkdir()
+    str_path = str(test_dir)
+    gitp = GitInterface(str_path, logger)
+    gitp.git_operation("remote", "add", "origin", "https://github.com/jedwards4b/fleximod-test2")
+    gitp.git_operation("fetch", "origin")
+    gitp.git_operation("checkout", "v0.0.2")
+    
     return test_dir
     
 @pytest.fixture
diff --git a/tests/test_e_complex_update.py b/tests/test_e_complex_update.py
new file mode 100644
index 000000000..44998f186
--- /dev/null
+++ b/tests/test_e_complex_update.py
@@ -0,0 +1,68 @@
+import pytest
+from pathlib import Path
+from git_fleximod.gitinterface import GitInterface
+
+def test_complex_update(git_fleximod, complex_update, logger):
+    status = git_fleximod(complex_update, "status")
+    assert("ToplevelOptional not checked out, aligned at tag v5.3.2" in status.stdout)
+    assert("ToplevelRequired not checked out, aligned at tag MPIserial_2.5.0" in status.stdout)
+    assert("AlwaysRequired not checked out, aligned at tag MPIserial_2.4.0" in status.stdout)
+    assert("Complex not checked out, out of sync at tag testtag02, expected tag is testtag3" in status.stdout)
+    assert("AlwaysOptional not checked out, out of sync at tag None, expected tag is MPIserial_2.3.0" in status.stdout)
+    print(f"status before is {status.stdout}")
+    
+    # This should checkout and update test_submodule and complex_sub
+    result = git_fleximod(complex_update, "update")    
+    assert result.returncode == 0
+
+    status = git_fleximod(complex_update, "status")
+    assert("ToplevelOptional not checked out, aligned at tag v5.3.2" in status.stdout)
+    assert("ToplevelRequired at tag MPIserial_2.5.0" in status.stdout)
+    assert("AlwaysRequired at tag MPIserial_2.4.0" in status.stdout)
+    assert("Complex at tag testtag3" in status.stdout)
+    print(f"status after is {status.stdout}")
+    # now check the complex_sub
+    root = (complex_update / "modules" / "complex")
+    assert(not (root / "libraries" / "gptl" / ".git").exists())
+    assert(not (root / "libraries" / "mpi-serial" / ".git").exists())
+    assert((root / "modules" / "mpi-serialAR" / ".git").exists())
+    assert((root / "modules" / "mpi-serialSAR" / ".git").exists())
+    assert(not (root / "modules" / "mpi-serial2" / ".git").exists())
+    assert((root / "modules" / "mpi-sparse" / ".git").exists())
+    assert((root / "modules" / "mpi-sparse" / "m4").exists())
+    assert(not (root / "modules" / "mpi-sparse" / "README").exists())
+
+    # update a single optional submodule
+
+    result = git_fleximod(complex_update, "update ToplevelOptional")
+    assert result.returncode == 0
+
+    status = git_fleximod(complex_update, "status")
+    assert("ToplevelOptional at tag v5.3.2" in status.stdout)
+    assert("ToplevelRequired at tag MPIserial_2.5.0" in status.stdout)
+    assert("AlwaysRequired at tag MPIserial_2.4.0" in status.stdout)
+    assert("Complex at tag testtag3" in status.stdout)
+    assert("AlwaysOptional not checked out, out of sync at tag None, expected tag is MPIserial_2.3.0" in status.stdout)
+
+    # Finally update optional
+    result = git_fleximod(complex_update, "update --optional")
+    assert result.returncode == 0
+
+    status = git_fleximod(complex_update, "status")
+    assert("ToplevelOptional at tag v5.3.2" in status.stdout)
+    assert("ToplevelRequired at tag MPIserial_2.5.0" in status.stdout)
+    assert("AlwaysRequired at tag MPIserial_2.4.0" in status.stdout)
+    assert("Complex at tag testtag3" in status.stdout)
+    assert("AlwaysOptional at tag MPIserial_2.3.0" in status.stdout)
+    
+    # now check the complex_sub
+    root = (complex_update / "modules" / "complex" )
+    assert(not (root / "libraries" / "gptl" / ".git").exists())
+    assert(not (root / "libraries" / "mpi-serial" / ".git").exists())
+    assert((root / "modules" / "mpi-serial" / ".git").exists())
+    assert((root / "modules" / "mpi-serial2" / ".git").exists())
+    assert((root / "modules" / "mpi-sparse" / ".git").exists())
+    assert((root / "modules" / "mpi-sparse" / "m4").exists())
+    assert(not (root / "modules" / "mpi-sparse" / "README").exists())
+
+

From 82dd7a41d1b01092a968e2126f6f300f872d2812 Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Thu, 4 Jul 2024 07:59:52 -0600
Subject: [PATCH 2/9] test_b_update now working

---
 git_fleximod/git_fleximod.py | 250 +++++++++--------------------------
 git_fleximod/gitinterface.py |   6 +-
 2 files changed, 70 insertions(+), 186 deletions(-)

diff --git a/git_fleximod/git_fleximod.py b/git_fleximod/git_fleximod.py
index 69da054d4..505d7222f 100755
--- a/git_fleximod/git_fleximod.py
+++ b/git_fleximod/git_fleximod.py
@@ -13,7 +13,7 @@
 from git_fleximod import cli
 from git_fleximod.gitinterface import GitInterface
 from git_fleximod.gitmodules import GitModules
-from configparser import NoOptionError
+from git_fleximod.submodule import Submodule
 
 # logger variable is global
 logger = None
@@ -265,132 +265,42 @@ def single_submodule_checkout(
 
     return
 
-def add_remote(git, url):
-    remotes = git.git_operation("remote", "-v")
-    newremote = "newremote.00"
-    if url in remotes:
-        for line in remotes:
-            if url in line and "fetch" in line:
-                newremote = line.split()[0]
-                break
-    else:
-        i = 0
-        while "newremote" in remotes:
-            i = i + 1
-            newremote = f"newremote.{i:02d}"
-        git.git_operation("remote", "add", newremote, url)
-    return newremote
-
-def submodules_status(gitmodules, root_dir, toplevel=False):
+
+def init_submodule_from_gitmodules(gitmodules, name, root_dir, logger):
+    path = gitmodules.get(name, "path")
+    url = gitmodules.get(name, "url")
+    tag = gitmodules.get(name, "fxtag")
+    fxurl = gitmodules.get(name, "fxDONOTUSEurl")
+    fxsparse = gitmodules.get(name, "fxsparse")
+    fxrequired = gitmodules.get(name, "fxrequired")
+    return Submodule(root_dir, name, path, url, fxtag=tag, fxurl=fxurl, fxsparse=fxsparse, fxrequired=fxrequired, logger=logger)
+
+def submodules_status(gitmodules, root_dir, toplevel=False, depth=0):
     testfails = 0
     localmods = 0
     needsupdate = 0
+    submodules = {}
+    wrapper = textwrap.TextWrapper(initial_indent=' '*depth, width=120,subsequent_indent=' '*(depth+20))
     for name in gitmodules.sections():
-        path = gitmodules.get(name, "path")
-        tag = gitmodules.get(name, "fxtag")
-        url = gitmodules.get(name, "url")
-        required = gitmodules.get(name, "fxrequired")
-        level = required and required.startswith("Top")
-        if not path:
-            utils.fatal_error("No path found in .gitmodules for {}".format(name))
-        newpath = os.path.join(root_dir, path)
-        logger.debug("newpath is {}".format(newpath))
-        if not os.path.exists(os.path.join(newpath, ".git")):
-            rootgit = GitInterface(root_dir, logger)
-            # submodule commands use path, not name
-            url = url.replace("git@github.com:", "https://github.com/")
-            tags = rootgit.git_operation("ls-remote", "--tags", url)
-            result = rootgit.git_operation("submodule","status",newpath).split()
-#            print(f"newpath {newpath} result {result}")
-            ahash = None
-            if result:
-                ahash = result[0][1:]
-            hhash = None
-            atag = None
+        if not submodules or name not in submodules:
+            submodules[name] = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
+            
+        result,n,l,t = submodules[name].status()
+        testfails += t
+        localmods += l
+        needsupdate += n
+        if toplevel or not submodules[name].toplevel():
+            print(wrapper.fill(result))
+        subdir = os.path.join(root_dir, submodules[name].path)
+        if os.path.exists(os.path.join(subdir, ".gitmodules")):
+            submod = GitModules(logger, confpath=subdir)
+            t,l,n = submodules_status(submod, subdir, depth=depth+1)
+            testfails += t
+            localmods += l
+            needsupdate += n
             
-            needsupdate += 1
-            if not toplevel and level:
-                continue
-            for htag in tags.split("\n"):
-                if htag.endswith('^{}'):
-                    htag = htag[:-3]
-                if ahash and not atag and ahash in htag:
-                    atag = (htag.split()[1])[10:]
-                if tag and not hhash and htag.endswith(tag):
-                    hhash = htag.split()[0]
-                if hhash and atag:
-                    break
-            optional = " (optional)" if required and "Optional" in required else ""
-#            print(f"tag is {tag} ahash is {ahash} {hhash} {atag} {newpath} {required} {level}")
-            if required and not required.startswith("Always") and not level:
-                continue
-
-            if tag and (ahash == hhash or atag == tag):
-                print(f"e {name:>20} not checked out, aligned at tag {tag}{optional} {level} {required}")
-            elif tag:
-                ahash = rootgit.git_operation(
-                    "submodule", "status", "{}".format(path)
-                ).rstrip()
-                ahash = ahash[1 : len(tag) + 1]
-                if tag == ahash:
-                    print(f"e {name:>20} not checked out, aligned at hash {ahash}{optional}")
-                else:
-                    print(
-                        f"e {name:>20} not checked out, out of sync at tag {atag}, expected tag is {tag}{optional}"
-                    )
-                    testfails += 1
-            else:
-                print(f"e {name:>20} has no fxtag defined in .gitmodules{optional}")
-                testfails += 1
-        else:
-            with utils.pushd(newpath):
-                git = GitInterface(newpath, logger)
-                atag = git.git_operation("describe", "--tags", "--always").rstrip()
-                ahash =  git.git_operation("rev-list", "HEAD").partition("\n")[0]
-                rurl = git.git_operation("ls-remote","--get-url").rstrip()
-                recurse = False
-                if rurl != url:
-                    remote = add_remote(git, url)
-                    git.git_operation("fetch", remote)
-                if tag and atag == tag:
-                    print(f"  {name:>20} at tag {tag}")
-                    recurse = True
-                elif tag and ahash[: len(tag)] == tag:
-                    print(f"  {name:>20} at hash {ahash}")
-                    recurse = True
-                elif atag == ahash:
-                    print(f"  {name:>20} at hash {ahash}")
-                    recurse = True
-                elif tag:
-                    print(
-                        f"s {name:>20} {atag} {ahash} is out of sync with .gitmodules {tag}"
-                    )
-                    testfails += 1
-                    needsupdate += 1
-                else:
-                    print(
-                        f"e {name:>20} has no fxtag defined in .gitmodules, module at {atag}"
-                    )
-                    testfails += 1
-                    
-                status = git.git_operation("status", "--ignore-submodules", "-uno")
-                if "nothing to commit" not in status:
-                    localmods = localmods + 1
-                    print("M" + textwrap.indent(status, "                      "))
-                if recurse and os.path.exists(os.path.join(newpath, ".gitmodules")):
-                    submodules = GitModules(
-                        logger,
-                        confpath=newpath,
-                    )
-                    nf, lm, nu = submodules_status(submodules, newpath)
-                    testfails += nf
-                    localmods += lm
-                    needsupdate += nu
-                
-                    
     return testfails, localmods, needsupdate
 
-
 def git_toplevelroot(root_dir, logger):
     rgit = GitInterface(root_dir, logger)
     superroot = rgit.git_operation("rev-parse", "--show-superproject-working-tree")
@@ -398,25 +308,24 @@ def git_toplevelroot(root_dir, logger):
 
 
 def submodules_update(gitmodules, root_dir, requiredlist, force):
-    _, localmods, needsupdate = submodules_status(gitmodules, root_dir)
-    if localmods and not force:
-        local_mods_output()
-        return
-    if needsupdate == 0:
-        return
-
+#    _, localmods, needsupdate = submodules_status(gitmodules, root_dir)
+#    if localmods and not force:
+#        local_mods_output()
+#        return
+#    if needsupdate == 0:
+#        return
+    submodules = {}
     for name in gitmodules.sections():
-        fxtag = gitmodules.get(name, "fxtag")
-        path = gitmodules.get(name, "path")
-        url = gitmodules.get(name, "url")
-
-        fxrequired = gitmodules.get(name, "fxrequired")
-        if fxrequired:
-            allowedvalues = fxrequired_allowed_values()
-            assert fxrequired in allowedvalues
+        if not submodules or name not in submodules:
+            submodules[name] = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
+        _, needsupdate, localmods, testfails = submodules[name].status()
+        fxrequired = submodules[name].fxrequired
+        if not fxrequired:
+            fxrequired = "AlwaysRequired"
 
+        allowedvalues = fxrequired_allowed_values()
+        assert fxrequired in allowedvalues
 
-        fxsparse = gitmodules.get(name, "fxsparse")
         superroot = git_toplevelroot(root_dir, logger)
                                      
         if (
@@ -429,56 +338,27 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
                     print(f"Skipping optional component {name:>20}")
                 continue
         
-        if fxsparse:
-            logger.debug(
-                "Callng submodule_sparse_checkout({}, {}, {}, {}, {}, {}".format(
-                    root_dir, name, url, path, fxsparse, fxtag
-                )
-            )
-            submodule_sparse_checkout(root_dir, name, url, path, fxsparse, tag=fxtag)
-        else:
-            logger.info(
-                "Calling submodule_checkout({},{},{},{})".format(
-                    root_dir, name, path, url
-                )
-            )
-            print(f"fxrequired {fxrequired} requiredlist {requiredlist}")
-            single_submodule_checkout(
-                root_dir,
-                name,
-                path,
-                url=url,
-                tag=fxtag,
-                force=force,
-                optional="AlwaysOptional" in requiredlist 
-            )
+#        if fxsparse:
+#            logger.debug(
+#                "Callng submodule_sparse_checkout({}, {}, {}, {}, {}, {}".format(
+#                    root_dir, name, url, path, fxsparse, fxtag
+#                )
+#            )
+#            submodule_sparse_checkout(root_dir, name, url, path, fxsparse, tag=fxtag)
+#        else:
+        print(f"fxrequired {fxrequired} requiredlist {requiredlist}")
+        submodules[name].update(optional="AlwaysOptional")
+
+#            single_submodule_checkout(
+#                root_dir,
+#                name,
+#                path,
+#                url=url,
+#                tag=fxtag,
+#                force=force,
+#                optional="AlwaysOptional" in requiredlist 
+#            )
 
-        if os.path.exists(os.path.join(path, ".git")):
-            submoddir = os.path.join(root_dir, path)
-            with utils.pushd(submoddir):
-                git = GitInterface(submoddir, logger)
-                # first make sure the url is correct
-                upstream = git.git_operation("ls-remote", "--get-url").rstrip()
-                newremote = "origin"
-                if upstream != url:
-                    add_remote(git, url)
-
-                tags = git.git_operation("tag", "-l")
-                if fxtag and fxtag not in tags:
-                    git.git_operation("fetch", newremote, "--tags")
-                atag = git.git_operation("describe", "--tags", "--always").rstrip()
-                if fxtag and fxtag != atag:
-                    try:
-                        git.git_operation("checkout", fxtag)
-                        print(f"{name:>20} updated to {fxtag}")
-                    except Exception as error:
-                        print(error)
-                    
-
-                elif not fxtag:
-                    print(f"No fxtag found for submodule {name:>20}")
-                else:
-                    print(f"{name:>20} up to date.")
 
 
 
diff --git a/git_fleximod/gitinterface.py b/git_fleximod/gitinterface.py
index 93ae38ecd..c7462b3a1 100644
--- a/git_fleximod/gitinterface.py
+++ b/git_fleximod/gitinterface.py
@@ -62,7 +62,11 @@ def git_operation(self, operation, *args, **kwargs):
     def config_get_value(self, section, name):
         if self._use_module:
             config = self.repo.config_reader()
-            return config.get_value(section, name)
+            try:
+                val = config.get_value(section, name)
+            except:
+                val = None
+            return val
         else:
             cmd = ("git", "-C", str(self.repo_path), "config", "--get", f"{section}.{name}")
             output = utils.execute_subprocess(cmd, output_to_caller=True)

From 72211d20b74babf0f9996391a1f7f7f4091f17c2 Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Thu, 4 Jul 2024 08:44:48 -0600
Subject: [PATCH 3/9] add the new submodule class

---
 git_fleximod/submodule.py | 329 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 329 insertions(+)
 create mode 100644 git_fleximod/submodule.py

diff --git a/git_fleximod/submodule.py b/git_fleximod/submodule.py
new file mode 100644
index 000000000..f1c1b6298
--- /dev/null
+++ b/git_fleximod/submodule.py
@@ -0,0 +1,329 @@
+import os
+import textwrap
+import shutil
+from configparser import NoOptionError
+from git_fleximod import utils
+from git_fleximod.gitinterface import GitInterface
+
+class Submodule():
+    def __init__(self, root_dir, name, path, url, fxtag=None, fxurl=None, fxsparse=None, fxrequired=None, logger=None):
+        self.name = name
+        self.root_dir = root_dir
+        self.path = path
+        url = url.replace("git@github.com:", "https://github.com/")
+        self.url = url
+        self.fxurl = fxurl
+        self.fxtag = fxtag
+        self.fxsparse = fxsparse
+        self.fxrequired = fxrequired
+        self.logger = logger
+       
+    def status(self):
+        smpath = os.path.join(self.root_dir, self.path)
+        testfails = 0
+        localmods = 0
+        needsupdate = 0
+        ahash = None
+        optional = " (optional)" if "Optional" in self.fxrequired else None
+        required = None
+        level = None
+        if not os.path.exists(os.path.join(smpath, ".git")):
+            rootgit = GitInterface(self.root_dir, self.logger)
+            # submodule commands use path, not name
+            tags = rootgit.git_operation("ls-remote", "--tags", self.url)
+            result = rootgit.git_operation("submodule","status",smpath).split()
+            
+            if result:
+                ahash = result[0][1:]
+            hhash = None
+            atag = None
+            for htag in tags.split("\n"):
+                if htag.endswith('^{}'):
+                    htag = htag[:-3]
+                if ahash and not atag and ahash in htag:
+                    atag = (htag.split()[1])[10:]
+                if self.fxtag and not hhash and htag.endswith(self.fxtag):
+                    hhash = htag.split()[0]
+                if hhash and atag:
+                    break
+            if self.fxtag and (ahash == hhash or atag == self.fxtag):
+                result = f"e {self.name:>20} not checked out, aligned at tag {self.fxtag}{optional} {level} {required}"
+            elif self.fxtag:
+                ahash = rootgit.git_operation(
+                    "submodule", "status", "{}".format(self.path)
+                ).rstrip()
+                ahash = ahash[1 : len(self.fxtag) + 1]
+                if self.fxtag == ahash:
+                    result = f"e {self.name:>20} not checked out, aligned at hash {ahash}{optional}"
+                else:
+                    result = f"e {self.name:>20} not checked out, out of sync at tag {atag}, expected tag is {self.fxtag}{optional}"
+                    testfails += 1
+            else:
+                result = f"e {self.name:>20} has no fxtag defined in .gitmodules{optional}"
+                testfails += 1
+        else:
+            with utils.pushd(smpath):
+                git = GitInterface(smpath, self.logger)
+                remote = git.git_operation("remote").rstrip()
+                if remote == '':
+                    result = f"e {self.name:>20} has no associated remote"
+                    testfails += 1
+                    needsupdate += 1
+                    return result, needsupdate, localmods, testfails                    
+                rurl = git.git_operation("ls-remote","--get-url").rstrip()
+                atag = git.git_operation("describe", "--tags", "--always").rstrip()
+                ahash =  git.git_operation("rev-list", "HEAD").partition("\n")[0]
+                    
+                recurse = False
+                if rurl != self.url:
+                    remote = self._add_remote(git)
+                    git.git_operation("fetch", remote)
+                if self.fxtag and atag == self.fxtag:
+                    result = f"  {self.name:>20} at tag {self.fxtag}"
+                    recurse = True
+                elif self.fxtag and ahash[: len(self.fxtag)] == self.fxtag:
+                    result = f"  {self.name:>20} at hash {ahash}"
+                    recurse = True
+                elif atag == ahash:
+                    result = f"  {self.name:>20} at hash {ahash}"
+                    recurse = True
+                elif self.fxtag:
+                    result = f"s {self.name:>20} {atag} {ahash} is out of sync with .gitmodules {self.fxtag}"
+                    testfails += 1
+                    needsupdate += 1
+                else:
+                    result = f"e {self.name:>20} has no fxtag defined in .gitmodules, module at {atag}"
+                    testfails += 1
+                    
+                status = git.git_operation("status", "--ignore-submodules", "-uno")
+                if "nothing to commit" not in status:
+                    localmods = localmods + 1
+                    result = "M" + textwrap.indent(status, "                      ")
+        
+        return result, needsupdate, localmods, testfails
+
+    
+    def _add_remote(self, git):
+        remotes = git.git_operation("remote", "-v").splitlines()
+        upstream = None
+        if remotes:
+            upstream = git.git_operation("ls-remote", "--get-url").rstrip()
+            newremote = "newremote.00"
+
+            line = next((s for s in remotes if self.url in s), None)
+            if line:
+                newremote = line.split()[0]
+                return newremote
+            else:
+                i = 0
+                while "newremote" in remotes:
+                    i = i + 1
+                    newremote = f"newremote.{i:02d}"
+        else:
+            newremote = "origin"
+        git.git_operation("remote", "add", newremote, self.url)
+        return newremote
+
+    def toplevel(self):
+        if self.fxrequired:
+            return True if self.fxrequired.startswith("Top") else False
+
+    def sparse_checkout(self):
+        """
+            This function performs a sparse checkout of a git submodule.  It does so by first creating the .git/info/sparse-checkout fileq
+            in the submodule and then checking out the desired tag.  If the submodule is already checked out, it will not be checked out again.
+            Creating the sparse-checkout file first prevents the entire submodule from being checked out and then removed.  This is important
+            because the submodule may have a large number of files and checking out the entire submodule and then removing it would be time
+            and disk space consuming.
+
+            Returns:
+            None
+        """
+        self.logger.info("Called sparse_checkout for {}".format(self.name))
+        rgit = GitInterface(self.root_dir, self.logger)
+        superroot = rgit.git_operation("rev-parse", "--show-superproject-working-tree")
+        if superroot:
+            gitroot = superroot.strip()
+        else:
+            gitroot = self.root_dir.strip()
+        assert os.path.isdir(os.path.join(gitroot, ".git"))
+        # first create the module directory
+        if not os.path.isdir(os.path.join(self.root_dir, self.path)):
+            os.makedirs(os.path.join(self.root_dir, self.path))
+
+        # initialize a new git repo and set the sparse checkout flag
+        sprep_repo = os.path.join(self.root_dir, self.path)
+        sprepo_git = GitInterface(sprep_repo, self.logger)
+        if os.path.exists(os.path.join(sprep_repo, ".git")):
+            try:
+                self.logger.info("Submodule {} found".format(self.name))
+                chk = sprepo_git.config_get_value("core", "sparseCheckout")
+                if chk == "true":
+                    self.logger.info("Sparse submodule {} already checked out".format(self.name))
+                    return
+            except (NoOptionError):
+                self.logger.debug("Sparse submodule {} not present".format(self.name))
+            except Exception as e:
+                utils.fatal_error("Unexpected error {} occured.".format(e))
+
+        sprepo_git.config_set_value("core", "sparseCheckout", "true")
+
+        # set the repository remote
+        
+        self.logger.info("Setting remote origin in {}/{}".format(self.root_dir, self.path))
+        status = sprepo_git.git_operation("remote", "-v")
+        if self.url not in status:
+            sprepo_git.git_operation("remote", "add", "origin", self.url)
+
+        topgit = os.path.join(gitroot, ".git")
+
+        if gitroot != self.root_dir and os.path.isfile(os.path.join(self.root_dir, ".git")):
+            with open(os.path.join(self.root_dir, ".git")) as f:
+                gitpath = os.path.relpath(
+                    os.path.join(self.root_dir, f.read().split()[1]),
+                    start=os.path.join(self.root_dir, self.path),
+                )
+                topgit = os.path.join(gitpath, "modules")
+        else:
+            topgit = os.path.relpath(
+                os.path.join(self.root_dir, ".git", "modules"),
+                start=os.path.join(self.root_dir, self.path),
+            )
+
+        with utils.pushd(sprep_repo):
+            if not os.path.isdir(topgit):
+                os.makedirs(topgit)
+        topgit += os.sep + self.name
+
+        if os.path.isdir(os.path.join(self.root_dir, self.path, ".git")):
+            with utils.pushd(sprep_repo):
+                if os.path.isdir(os.path.join(topgit,".git")):
+                    shutil.rmtree(os.path.join(topgit,".git"))
+                shutil.move(".git", topgit)
+                with open(".git", "w") as f:
+                    f.write("gitdir: " + os.path.relpath(topgit))
+                #    assert(os.path.isdir(os.path.relpath(topgit, start=sprep_repo)))
+                gitsparse = os.path.abspath(os.path.join(topgit, "info", "sparse-checkout"))
+            if os.path.isfile(gitsparse):
+                self.logger.warning(
+                    "submodule {} is already initialized {}".format(self.name, topgit)
+                )
+                return
+
+            with utils.pushd(sprep_repo):
+                if os.path.isfile(self.fxsparse):
+                    shutil.copy(self.fxsparse, gitsparse)
+                
+
+        # Finally checkout the repo
+        sprepo_git.git_operation("fetch", "origin", "--tags")
+        sprepo_git.git_operation("checkout", self.fxtag)
+
+        print(f"Successfully checked out {self.name:>20} at {self.fxtag}")
+        rgit.config_set_value(f'submodule "{self.name}"', "active", "true")
+        rgit.config_set_value(f'submodule "{self.name}"', "url", self.url)
+
+    def update(self, optional=None):
+            # function implementation...
+        git = GitInterface(self.root_dir, self.logger)
+        repodir = os.path.join(self.root_dir, self.path)
+        self.logger.info("Checkout {} into {}/{}".format(self.name, self.root_dir, self.path))
+        # if url is provided update to the new url
+        tmpurl = None
+        repo_exists = False
+        #    if os.path.exists(os.path.join(repodir, ".git")):
+        #        self.logger.info("Submodule {} already checked out".format(self.name))
+        #        repo_exists = True
+        # Look for a .gitmodules file in the newly checkedout repo
+        if self.fxsparse:
+            print(f"Sparse checkout {self.name} fxsparse {self.fxsparse}")
+            self.sparse_checkout()
+        else:
+            if not repo_exists and self.url:
+                # ssh urls cause problems for those who dont have git accounts with ssh keys defined
+                # but cime has one since e3sm prefers ssh to https, because the .gitmodules file was
+                # opened with a GitModules object we don't need to worry about restoring the file here
+                # it will be done by the GitModules class
+                if self.url.startswith("git@"):
+                    tmpurl = self.url
+                    url = self.url.replace("git@github.com:", "https://github.com/")
+                    git.git_operation("clone", url, self.path)
+                    smgit = GitInterface(repodir, self.logger)
+                    if not tag:
+                        tag = smgit.git_operation("describe", "--tags", "--always").rstrip()
+                    smgit.git_operation("checkout", tag)
+                    # Now need to move the .git dir to the submodule location
+                    rootdotgit = os.path.join(self.root_dir, ".git")
+                    if os.path.isfile(rootdotgit):
+                        with open(rootdotgit) as f:
+                            line = f.readline()
+                            if line.startswith("gitdir: "):
+                                rootdotgit = line[8:].rstrip()
+
+                    newpath = os.path.abspath(os.path.join(self.root_dir, rootdotgit, "modules", self.name))
+                    if os.path.exists(newpath):
+                        shutil.rmtree(os.path.join(repodir, ".git"))
+                    else:
+                        shutil.move(os.path.join(repodir, ".git"), newpath)
+                    
+                    with open(os.path.join(repodir, ".git"), "w") as f:
+                        f.write("gitdir: " + os.path.relpath(newpath, start=repodir))
+                
+            if not os.path.exists(repodir):
+                parent = os.path.dirname(repodir)
+                if not os.path.isdir(parent):
+                    os.makedirs(parent)
+                git.git_operation("submodule", "add", "--name", self.name, "--", url, self.path) 
+
+            if not repo_exists or not tmpurl:
+                git.git_operation("submodule", "update", "--init", "--", self.path)
+
+            if self.fxtag and not optional:        
+                smgit = GitInterface(repodir, self.logger)
+                smgit.git_operation("checkout", self.fxtag)
+
+            if os.path.exists(os.path.join(repodir, ".gitmodules")):
+                # recursively handle this checkout
+                print(f"Recursively checking out submodules of {self.name} {optional}")
+                gitmodules = GitModules(self.logger, confpath=repodir)
+                requiredlist = ["AlwaysRequired"]
+                if optional:
+                    requiredlist.append("AlwaysOptional")
+                submodules_checkout(gitmodules, repodir, requiredlist, force=force)
+            if not os.path.exists(os.path.join(repodir, ".git")):
+                utils.fatal_error(
+                    f"Failed to checkout {self.name} {repo_exists} {tmpurl} {repodir} {self.path}"
+                )
+                
+            if tmpurl:
+                print(git.git_operation("restore", ".gitmodules"))
+        if os.path.exists(os.path.join(self.path, ".git")):
+            submoddir = os.path.join(self.root_dir, self.path)
+            with utils.pushd(submoddir):
+                git = GitInterface(submoddir, self.logger)
+                # first make sure the url is correct
+                print("3calling ls-remote")
+
+                newremote = self._add_remote(git)
+
+                tags = git.git_operation("tag", "-l")
+                fxtag = self.fxtag
+                if fxtag and fxtag not in tags:
+                    git.git_operation("fetch", newremote, "--tags")
+                atag = git.git_operation("describe", "--tags", "--always").rstrip()
+                if fxtag and fxtag != atag:
+                    try:
+                        git.git_operation("checkout", fxtag)
+                        print(f"{self.name:>20} updated to {fxtag}")
+                    except Exception as error:
+                        print(error)
+                    
+
+                elif not fxtag:
+                    print(f"No fxtag found for submodule {self.name:>20}")
+                else:
+                    print(f"{self.name:>20} up to date.")
+
+
+                
+        return

From 2b021fdd76e31b4c7addaae97db0241d557183a2 Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Thu, 4 Jul 2024 09:08:45 -0600
Subject: [PATCH 4/9] add documentation to submodule.py

---
 git_fleximod/submodule.py | 82 +++++++++++++++++++++++++++++++++++----
 1 file changed, 74 insertions(+), 8 deletions(-)

diff --git a/git_fleximod/submodule.py b/git_fleximod/submodule.py
index f1c1b6298..dc27c3d00 100644
--- a/git_fleximod/submodule.py
+++ b/git_fleximod/submodule.py
@@ -6,7 +6,24 @@
 from git_fleximod.gitinterface import GitInterface
 
 class Submodule():
+    """
+    Represents a Git submodule with enhanced features for flexible management.
+
+    Attributes:
+        name (str): The name of the submodule.
+        root_dir (str): The root directory of the main project.
+        path (str): The relative path from the root directory to the submodule.
+        url (str): The URL of the submodule repository.
+        fxurl (str): The URL for flexible submodule management (optional).
+        fxtag (str): The tag for flexible submodule management (optional).
+        fxsparse (str): Path to the sparse checkout file relative to the submodule path, see git-sparse-checkout for details (optional).
+        fxrequired (str): Indicates if the submodule is optional or required (optional).
+        logger (logging.Logger): Logger instance for logging (optional).
+    """
     def __init__(self, root_dir, name, path, url, fxtag=None, fxurl=None, fxsparse=None, fxrequired=None, logger=None):
+        """
+        Initializes a new Submodule instance with the provided attributes.
+        """
         self.name = name
         self.root_dir = root_dir
         self.path = path
@@ -19,6 +36,13 @@ def __init__(self, root_dir, name, path, url, fxtag=None, fxurl=None, fxsparse=N
         self.logger = logger
        
     def status(self):
+        """
+        Checks the status of the submodule and returns 4 parameters:
+        - result (str): The status of the submodule.
+        - needsupdate (int): An indicator if the submodule needs to be updated.
+        - localmods (int): An indicator if the submodule has local modifications.
+        - testfails (int): An indicator if the submodule has failed a test, this is used for testing purposes.        
+        """
         smpath = os.path.join(self.root_dir, self.path)
         testfails = 0
         localmods = 0
@@ -104,6 +128,19 @@ def status(self):
 
     
     def _add_remote(self, git):
+        """
+        Adds a new remote to the submodule if it does not already exist.
+
+        This method checks the existing remotes of the submodule. If the submodule's URL is not already listed as a remote,
+        it attempts to add a new remote. The name for the new remote is generated dynamically to avoid conflicts. If no
+        remotes exist, it defaults to naming the new remote 'origin'.
+
+        Args:
+            git (GitInterface): An instance of GitInterface to perform git operations.
+
+        Returns:
+            str: The name of the new remote if added, or the name of the existing remote that matches the submodule's URL.
+        """ 
         remotes = git.git_operation("remote", "-v").splitlines()
         upstream = None
         if remotes:
@@ -125,20 +162,27 @@ def _add_remote(self, git):
         return newremote
 
     def toplevel(self):
+        """
+          Checks if the submodule is a top-level submodule (ie not a submodule of a submodule).
+        """
         if self.fxrequired:
             return True if self.fxrequired.startswith("Top") else False
 
     def sparse_checkout(self):
         """
-            This function performs a sparse checkout of a git submodule.  It does so by first creating the .git/info/sparse-checkout fileq
-            in the submodule and then checking out the desired tag.  If the submodule is already checked out, it will not be checked out again.
-            Creating the sparse-checkout file first prevents the entire submodule from being checked out and then removed.  This is important
-            because the submodule may have a large number of files and checking out the entire submodule and then removing it would be time
-            and disk space consuming.
+        Performs a sparse checkout of the submodule.
 
-            Returns:
+        This method optimizes the checkout process by only checking out files specified in the submodule's sparse-checkout configuration,
+        rather than the entire submodule content. It achieves this by first ensuring the `.git/info/sparse-checkout` file is created and
+        configured in the submodule's directory. Then, it proceeds to checkout the desired tag. If the submodule has already been checked out,
+        this method will not perform the checkout again.
+
+        This approach is particularly beneficial for submodules with a large number of files, as it significantly reduces the time and disk space
+        required for the checkout process by avoiding the unnecessary checkout and subsequent removal of unneeded files.
+
+        Returns:
             None
-        """
+        """ 
         self.logger.info("Called sparse_checkout for {}".format(self.name))
         rgit = GitInterface(self.root_dir, self.logger)
         superroot = rgit.git_operation("rev-parse", "--show-superproject-working-tree")
@@ -224,7 +268,29 @@ def sparse_checkout(self):
         rgit.config_set_value(f'submodule "{self.name}"', "url", self.url)
 
     def update(self, optional=None):
-            # function implementation...
+        """
+        Updates the submodule to the latest or specified version.
+
+        This method handles the update process of the submodule, including checking out the submodule into the specified path,
+        handling sparse checkouts if configured, and updating the submodule's URL if necessary. It supports both SSH and HTTPS URLs,
+        automatically converting SSH URLs to HTTPS to avoid issues for users without SSH keys.
+
+        The update process involves the following steps:
+        1. If the submodule is configured for sparse checkout, it performs a sparse checkout.
+        2. If the submodule is not already checked out, it clones the submodule using the provided URL.
+        3. If a specific tag or hash is provided, it checks out that tag; otherwise, it checks out the latest version.
+        4. If the root `.git` is a file (indicating a submodule or a worktree), additional steps are taken to integrate the submodule properly.
+
+        Args:
+            optional (bool): Indicates if the submodule is optional. This parameter is currently unused in the function but can be implemented for conditional updates based on the submodule's importance.
+
+        Note:
+            - The method currently does not use the `optional` parameter, but it is designed for future use where updates can be conditional based on the submodule's importance.
+            - SSH URLs are automatically converted to HTTPS to accommodate users without SSH keys.
+
+        Returns:
+            None
+        """
         git = GitInterface(self.root_dir, self.logger)
         repodir = os.path.join(self.root_dir, self.path)
         self.logger.info("Checkout {} into {}/{}".format(self.name, self.root_dir, self.path))

From 143abac699cd344b978b3fcd4fc76b9e21c409a5 Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Thu, 4 Jul 2024 10:19:56 -0600
Subject: [PATCH 5/9] tests a-d all pass

---
 git_fleximod/git_fleximod.py | 204 ++++-------------------------------
 git_fleximod/submodule.py    |  69 +++++-------
 2 files changed, 47 insertions(+), 226 deletions(-)

diff --git a/git_fleximod/git_fleximod.py b/git_fleximod/git_fleximod.py
index 505d7222f..e3adddb85 100755
--- a/git_fleximod/git_fleximod.py
+++ b/git_fleximod/git_fleximod.py
@@ -175,97 +175,6 @@ def submodule_sparse_checkout(root_dir, name, url, path, sparsefile, tag="master
     rgit.config_set_value(f'submodule "{name}"', "active", "true")
     rgit.config_set_value(f'submodule "{name}"', "url", url)
 
-
-def single_submodule_checkout(
-    root, name, path, url=None, tag=None, force=False, optional=False
-):
-    """
-    This function checks out a single git submodule.
-
-    Parameters:
-    root (str): The root directory for the git operation.
-    name (str): The name of the submodule.
-    path (str): The path to the submodule.
-    url (str, optional): The URL of the submodule. Defaults to None.
-    tag (str, optional): The tag to checkout. Defaults to None.
-    force (bool, optional): If set to True, forces the checkout operation. Defaults to False.
-    optional (bool, optional): If set to True, the submodule is considered optional. Defaults to False.
-
-    Returns:
-    None
-    """
-    # function implementation...
-    git = GitInterface(root, logger)
-    repodir = os.path.join(root, path)
-    logger.info("Checkout {} into {}/{}".format(name, root, path))
-    # if url is provided update to the new url
-    tmpurl = None
-    repo_exists = False
-#    if os.path.exists(os.path.join(repodir, ".git")):
-#        logger.info("Submodule {} already checked out".format(name))
-#        repo_exists = True
-    # Look for a .gitmodules file in the newly checkedout repo
-    if not repo_exists and url:
-        # ssh urls cause problems for those who dont have git accounts with ssh keys defined
-        # but cime has one since e3sm prefers ssh to https, because the .gitmodules file was
-        # opened with a GitModules object we don't need to worry about restoring the file here
-        # it will be done by the GitModules class
-        if url.startswith("git@"):
-            tmpurl = url
-            url = url.replace("git@github.com:", "https://github.com/")
-            git.git_operation("clone", url, path)
-            smgit = GitInterface(repodir, logger)
-            if not tag:
-                tag = smgit.git_operation("describe", "--tags", "--always").rstrip()
-            smgit.git_operation("checkout", tag)
-            # Now need to move the .git dir to the submodule location
-            rootdotgit = os.path.join(root, ".git")
-            if os.path.isfile(rootdotgit):
-                with open(rootdotgit) as f:
-                    line = f.readline()
-                    if line.startswith("gitdir: "):
-                        rootdotgit = line[8:].rstrip()
-
-            newpath = os.path.abspath(os.path.join(root, rootdotgit, "modules", name))
-            if os.path.exists(newpath):
-                shutil.rmtree(os.path.join(repodir, ".git"))
-            else:
-                shutil.move(os.path.join(repodir, ".git"), newpath)
-
-            with open(os.path.join(repodir, ".git"), "w") as f:
-                f.write("gitdir: " + os.path.relpath(newpath, start=repodir))
-                
-    if not os.path.exists(repodir):
-        parent = os.path.dirname(repodir)
-        if not os.path.isdir(parent):
-            os.makedirs(parent)
-        git.git_operation("submodule", "add", "--name", name, "--", url, path) 
-
-    if not repo_exists or not tmpurl:
-        git.git_operation("submodule", "update", "--init", "--", path)
-    if tag and not optional:        
-        smgit = GitInterface(repodir, logger)
-        smgit.git_operation("checkout", tag)
-
-    if os.path.exists(os.path.join(repodir, ".gitmodules")):
-        # recursively handle this checkout
-        print(f"Recursively checking out submodules of {name} {optional}")
-        gitmodules = GitModules(logger, confpath=repodir)
-        requiredlist = ["AlwaysRequired"]
-        if optional:
-            requiredlist.append("AlwaysOptional")
-        submodules_checkout(gitmodules, repodir, requiredlist, force=force)
-    if not os.path.exists(os.path.join(repodir, ".git")):
-        utils.fatal_error(
-            f"Failed to checkout {name} {repo_exists} {tmpurl} {repodir} {path}"
-        )
-
-    if tmpurl:
-        print(git.git_operation("restore", ".gitmodules"))
-
-    return
-
-
 def init_submodule_from_gitmodules(gitmodules, name, root_dir, logger):
     path = gitmodules.get(name, "path")
     url = gitmodules.get(name, "url")
@@ -280,7 +189,7 @@ def submodules_status(gitmodules, root_dir, toplevel=False, depth=0):
     localmods = 0
     needsupdate = 0
     submodules = {}
-    wrapper = textwrap.TextWrapper(initial_indent=' '*depth, width=120,subsequent_indent=' '*(depth+20))
+    wrapper = textwrap.TextWrapper(initial_indent=' '*(depth*10), width=120,subsequent_indent=' '*(depth*20))
     for name in gitmodules.sections():
         if not submodules or name not in submodules:
             submodules[name] = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
@@ -306,23 +215,15 @@ def git_toplevelroot(root_dir, logger):
     superroot = rgit.git_operation("rev-parse", "--show-superproject-working-tree")
     return superroot
 
-
 def submodules_update(gitmodules, root_dir, requiredlist, force):
-#    _, localmods, needsupdate = submodules_status(gitmodules, root_dir)
-#    if localmods and not force:
-#        local_mods_output()
-#        return
-#    if needsupdate == 0:
-#        return
     submodules = {}
     for name in gitmodules.sections():
         if not submodules or name not in submodules:
             submodules[name] = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
         _, needsupdate, localmods, testfails = submodules[name].status()
-        fxrequired = submodules[name].fxrequired
-        if not fxrequired:
-            fxrequired = "AlwaysRequired"
-
+        if not submodules[name].fxrequired:
+            submodules[name].fxrequired = "AlwaysRequired"
+        fxrequired = submodules[name].fxrequired    
         allowedvalues = fxrequired_allowed_values()
         assert fxrequired in allowedvalues
 
@@ -334,32 +235,24 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
             or fxrequired not in requiredlist)
         ):
             if "Optional" in fxrequired and "Optional" not in requiredlist:
+                print(f"Skipping optional component {name:>20}")
                 if fxrequired.startswith("Always"):
                     print(f"Skipping optional component {name:>20}")
                 continue
-        
-#        if fxsparse:
-#            logger.debug(
-#                "Callng submodule_sparse_checkout({}, {}, {}, {}, {}, {}".format(
-#                    root_dir, name, url, path, fxsparse, fxtag
-#                )
-#            )
-#            submodule_sparse_checkout(root_dir, name, url, path, fxsparse, tag=fxtag)
-#        else:
-        print(f"fxrequired {fxrequired} requiredlist {requiredlist}")
-        submodules[name].update(optional="AlwaysOptional")
-
-#            single_submodule_checkout(
-#                root_dir,
-#                name,
-#                path,
-#                url=url,
-#                tag=fxtag,
-#                force=force,
-#                optional="AlwaysOptional" in requiredlist 
-#            )
-
-
+        optional = "AlwaysOptional" in requiredlist
+        print(f"1 Required list is {requiredlist} optional is {optional}")
+        if fxrequired in requiredlist:
+            submodules[name].update()
+            repodir = os.path.join(root_dir, submodules[name].path)
+            if os.path.exists(os.path.join(repodir, ".gitmodules")):
+                # recursively handle this checkout
+                print(f"Recursively checking out submodules of {name}")
+                gitmodules = GitModules(submodules[name].logger, confpath=repodir)
+                requiredlist = ["AlwaysRequired"]
+                if optional:
+                    requiredlist.append("AlwaysOptional")
+                print(f"2 Required list is {requiredlist}")
+                submodules_update(gitmodules, repodir, requiredlist, force=force)
 
 
 def local_mods_output():
@@ -375,65 +268,6 @@ def local_mods_output():
 """
     print(text)
 
-# checkout is done by update if required so this function may be depricated
-def submodules_checkout(gitmodules, root_dir, requiredlist, force=False):
-    """
-    This function checks out all git submodules based on the provided parameters.
-
-    Parameters:
-    gitmodules (ConfigParser): The gitmodules configuration.
-    root_dir (str): The root directory for the git operation.
-    requiredlist (list): The list of required modules.
-    force (bool, optional): If set to True, forces the checkout operation. Defaults to False.
-
-    Returns:
-    None
-    """
-    # function implementation...
-    print("")
-    _, localmods, needsupdate = submodules_status(gitmodules, root_dir)
-    if localmods and not force:
-        local_mods_output()
-        return
-    if not needsupdate:
-        return
-    for name in gitmodules.sections():
-        fxrequired = gitmodules.get(name, "fxrequired")
-        fxsparse = gitmodules.get(name, "fxsparse")
-        fxtag = gitmodules.get(name, "fxtag")
-        path = gitmodules.get(name, "path")
-        url = gitmodules.get(name, "url")
-        superroot = git_toplevelroot(root_dir, logger)
-        if fxrequired and fxrequired not in requiredlist:
-            if fxrequired == "AlwaysOptional" or (superroot == root_dir):
-                print(f"Skipping optional component {name:>20} {requiredlist}")
-            continue
-            
-
-        if fxsparse:
-            logger.debug(
-                "Callng submodule_sparse_checkout({}, {}, {}, {}, {}, {}".format(
-                    root_dir, name, url, path, fxsparse, fxtag
-                )
-            )
-            submodule_sparse_checkout(root_dir, name, url, path, fxsparse, tag=fxtag)
-        else:
-            logger.debug(
-                "Calling submodule_checkout({},{},{})".format(root_dir, name, path)
-            )
-            single_submodule_checkout(
-                root_dir,
-                name,
-                path,
-                url=url,
-                tag=fxtag,
-                force=force,
-                optional="AlwaysOptional" == fxrequired \
-                or "ToplevelOptional" == fxrequired \
-                or "TopLevelOptional" == fxrequired
-            )
-
-
 def submodules_test(gitmodules, root_dir):
     """
     This function tests the git submodules based on the provided parameters.
diff --git a/git_fleximod/submodule.py b/git_fleximod/submodule.py
index dc27c3d00..a3e5624e5 100644
--- a/git_fleximod/submodule.py
+++ b/git_fleximod/submodule.py
@@ -32,23 +32,28 @@ def __init__(self, root_dir, name, path, url, fxtag=None, fxurl=None, fxsparse=N
         self.fxurl = fxurl
         self.fxtag = fxtag
         self.fxsparse = fxsparse
-        self.fxrequired = fxrequired
+        if fxrequired:
+            self.fxrequired = fxrequired
+        else:
+            self.fxrequired = "AlwaysRequired"
         self.logger = logger
        
     def status(self):
         """
         Checks the status of the submodule and returns 4 parameters:
         - result (str): The status of the submodule.
-        - needsupdate (int): An indicator if the submodule needs to be updated.
-        - localmods (int): An indicator if the submodule has local modifications.
-        - testfails (int): An indicator if the submodule has failed a test, this is used for testing purposes.        
+        - needsupdate (bool): An indicator if the submodule needs to be updated.
+        - localmods (bool): An indicator if the submodule has local modifications.
+        - testfails (bool): An indicator if the submodule has failed a test, this is used for testing purposes.        
         """
         smpath = os.path.join(self.root_dir, self.path)
-        testfails = 0
-        localmods = 0
-        needsupdate = 0
+        testfails = False
+        localmods = False
+        needsupdate = False
         ahash = None
-        optional = " (optional)" if "Optional" in self.fxrequired else None
+        optional = ""
+        if "Optional" in self.fxrequired:
+            optional = " (optional)" 
         required = None
         level = None
         if not os.path.exists(os.path.join(smpath, ".git")):
@@ -71,7 +76,8 @@ def status(self):
                 if hhash and atag:
                     break
             if self.fxtag and (ahash == hhash or atag == self.fxtag):
-                result = f"e {self.name:>20} not checked out, aligned at tag {self.fxtag}{optional} {level} {required}"
+                result = f"e {self.name:>20} not checked out, aligned at tag {self.fxtag}{optional}"
+                needsupdate = True
             elif self.fxtag:
                 ahash = rootgit.git_operation(
                     "submodule", "status", "{}".format(self.path)
@@ -81,18 +87,19 @@ def status(self):
                     result = f"e {self.name:>20} not checked out, aligned at hash {ahash}{optional}"
                 else:
                     result = f"e {self.name:>20} not checked out, out of sync at tag {atag}, expected tag is {self.fxtag}{optional}"
-                    testfails += 1
+                    testfails = True
+                needsupdate = True
             else:
                 result = f"e {self.name:>20} has no fxtag defined in .gitmodules{optional}"
-                testfails += 1
+                testfails = True
         else:
             with utils.pushd(smpath):
                 git = GitInterface(smpath, self.logger)
                 remote = git.git_operation("remote").rstrip()
                 if remote == '':
                     result = f"e {self.name:>20} has no associated remote"
-                    testfails += 1
-                    needsupdate += 1
+                    testfails = True
+                    needsupdate = True
                     return result, needsupdate, localmods, testfails                    
                 rurl = git.git_operation("ls-remote","--get-url").rstrip()
                 atag = git.git_operation("describe", "--tags", "--always").rstrip()
@@ -113,15 +120,15 @@ def status(self):
                     recurse = True
                 elif self.fxtag:
                     result = f"s {self.name:>20} {atag} {ahash} is out of sync with .gitmodules {self.fxtag}"
-                    testfails += 1
-                    needsupdate += 1
+                    testfails = True
+                    needsupdate = True
                 else:
                     result = f"e {self.name:>20} has no fxtag defined in .gitmodules, module at {atag}"
-                    testfails += 1
+                    testfails = True
                     
                 status = git.git_operation("status", "--ignore-submodules", "-uno")
                 if "nothing to commit" not in status:
-                    localmods = localmods + 1
+                    localmods = True
                     result = "M" + textwrap.indent(status, "                      ")
         
         return result, needsupdate, localmods, testfails
@@ -161,13 +168,6 @@ def _add_remote(self, git):
         git.git_operation("remote", "add", newremote, self.url)
         return newremote
 
-    def toplevel(self):
-        """
-          Checks if the submodule is a top-level submodule (ie not a submodule of a submodule).
-        """
-        if self.fxrequired:
-            return True if self.fxrequired.startswith("Top") else False
-
     def sparse_checkout(self):
         """
         Performs a sparse checkout of the submodule.
@@ -267,7 +267,7 @@ def sparse_checkout(self):
         rgit.config_set_value(f'submodule "{self.name}"', "active", "true")
         rgit.config_set_value(f'submodule "{self.name}"', "url", self.url)
 
-    def update(self, optional=None):
+    def update(self):
         """
         Updates the submodule to the latest or specified version.
 
@@ -282,10 +282,8 @@ def update(self, optional=None):
         4. If the root `.git` is a file (indicating a submodule or a worktree), additional steps are taken to integrate the submodule properly.
 
         Args:
-            optional (bool): Indicates if the submodule is optional. This parameter is currently unused in the function but can be implemented for conditional updates based on the submodule's importance.
-
+           None
         Note:
-            - The method currently does not use the `optional` parameter, but it is designed for future use where updates can be conditional based on the submodule's importance.
             - SSH URLs are automatically converted to HTTPS to accommodate users without SSH keys.
 
         Returns:
@@ -339,23 +337,15 @@ def update(self, optional=None):
                 parent = os.path.dirname(repodir)
                 if not os.path.isdir(parent):
                     os.makedirs(parent)
-                git.git_operation("submodule", "add", "--name", self.name, "--", url, self.path) 
+                git.git_operation("submodule", "add", "--name", self.name, "--", self.url, self.path) 
 
             if not repo_exists or not tmpurl:
                 git.git_operation("submodule", "update", "--init", "--", self.path)
 
-            if self.fxtag and not optional:        
+            if self.fxtag:        
                 smgit = GitInterface(repodir, self.logger)
                 smgit.git_operation("checkout", self.fxtag)
 
-            if os.path.exists(os.path.join(repodir, ".gitmodules")):
-                # recursively handle this checkout
-                print(f"Recursively checking out submodules of {self.name} {optional}")
-                gitmodules = GitModules(self.logger, confpath=repodir)
-                requiredlist = ["AlwaysRequired"]
-                if optional:
-                    requiredlist.append("AlwaysOptional")
-                submodules_checkout(gitmodules, repodir, requiredlist, force=force)
             if not os.path.exists(os.path.join(repodir, ".git")):
                 utils.fatal_error(
                     f"Failed to checkout {self.name} {repo_exists} {tmpurl} {repodir} {self.path}"
@@ -368,10 +358,7 @@ def update(self, optional=None):
             with utils.pushd(submoddir):
                 git = GitInterface(submoddir, self.logger)
                 # first make sure the url is correct
-                print("3calling ls-remote")
-
                 newremote = self._add_remote(git)
-
                 tags = git.git_operation("tag", "-l")
                 fxtag = self.fxtag
                 if fxtag and fxtag not in tags:

From 62a338b531bbf858b5d941d4f8f7af5166328dcf Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Thu, 4 Jul 2024 10:33:31 -0600
Subject: [PATCH 6/9] all pytests now passing

---
 git_fleximod/submodule.py      | 6 ++++++
 tests/test_e_complex_update.py | 9 +++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/git_fleximod/submodule.py b/git_fleximod/submodule.py
index a3e5624e5..68fb08d6d 100644
--- a/git_fleximod/submodule.py
+++ b/git_fleximod/submodule.py
@@ -168,6 +168,12 @@ def _add_remote(self, git):
         git.git_operation("remote", "add", newremote, self.url)
         return newremote
 
+    def toplevel(self):
+        """
+        Returns True if the submodule is Toplevel (either Required or Optional)
+        """
+        return True if "Top" in self.fxrequired else False
+
     def sparse_checkout(self):
         """
         Performs a sparse checkout of the submodule.
diff --git a/tests/test_e_complex_update.py b/tests/test_e_complex_update.py
index 44998f186..0c3ab4c6a 100644
--- a/tests/test_e_complex_update.py
+++ b/tests/test_e_complex_update.py
@@ -9,7 +9,6 @@ def test_complex_update(git_fleximod, complex_update, logger):
     assert("AlwaysRequired not checked out, aligned at tag MPIserial_2.4.0" in status.stdout)
     assert("Complex not checked out, out of sync at tag testtag02, expected tag is testtag3" in status.stdout)
     assert("AlwaysOptional not checked out, out of sync at tag None, expected tag is MPIserial_2.3.0" in status.stdout)
-    print(f"status before is {status.stdout}")
     
     # This should checkout and update test_submodule and complex_sub
     result = git_fleximod(complex_update, "update")    
@@ -20,7 +19,7 @@ def test_complex_update(git_fleximod, complex_update, logger):
     assert("ToplevelRequired at tag MPIserial_2.5.0" in status.stdout)
     assert("AlwaysRequired at tag MPIserial_2.4.0" in status.stdout)
     assert("Complex at tag testtag3" in status.stdout)
-    print(f"status after is {status.stdout}")
+
     # now check the complex_sub
     root = (complex_update / "modules" / "complex")
     assert(not (root / "libraries" / "gptl" / ".git").exists())
@@ -59,9 +58,11 @@ def test_complex_update(git_fleximod, complex_update, logger):
     root = (complex_update / "modules" / "complex" )
     assert(not (root / "libraries" / "gptl" / ".git").exists())
     assert(not (root / "libraries" / "mpi-serial" / ".git").exists())
-    assert((root / "modules" / "mpi-serial" / ".git").exists())
-    assert((root / "modules" / "mpi-serial2" / ".git").exists())
+    assert(not (root / "modules" / "mpi-serial" / ".git").exists())
+    assert((root / "modules" / "mpi-serialAR" / ".git").exists())
+    assert((root / "modules" / "mpi-serialSAR" / ".git").exists())
     assert((root / "modules" / "mpi-sparse" / ".git").exists())
+    assert((root / "modules" / "mpi-serial2" / ".git").exists())
     assert((root / "modules" / "mpi-sparse" / "m4").exists())
     assert(not (root / "modules" / "mpi-sparse" / "README").exists())
 

From ded91fda0c896e5967b6df8dd2d18895f9dae096 Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Thu, 4 Jul 2024 12:15:29 -0600
Subject: [PATCH 7/9] all tests passing, issue #50 fixed

---
 git_fleximod/git_fleximod.py | 15 ++++++++-------
 git_fleximod/submodule.py    | 13 +++++++++++--
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/git_fleximod/git_fleximod.py b/git_fleximod/git_fleximod.py
index e3adddb85..c4e595d85 100755
--- a/git_fleximod/git_fleximod.py
+++ b/git_fleximod/git_fleximod.py
@@ -215,11 +215,13 @@ def git_toplevelroot(root_dir, logger):
     superroot = rgit.git_operation("rev-parse", "--show-superproject-working-tree")
     return superroot
 
-def submodules_update(gitmodules, root_dir, requiredlist, force):
-    submodules = {}
+def submodules_update(gitmodules, root_dir, requiredlist, force, submodules=None):
+    if not submodules:
+        submodules = {}
     for name in gitmodules.sections():
         if not submodules or name not in submodules:
             submodules[name] = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
+    
         _, needsupdate, localmods, testfails = submodules[name].status()
         if not submodules[name].fxrequired:
             submodules[name].fxrequired = "AlwaysRequired"
@@ -235,24 +237,23 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
             or fxrequired not in requiredlist)
         ):
             if "Optional" in fxrequired and "Optional" not in requiredlist:
-                print(f"Skipping optional component {name:>20}")
                 if fxrequired.startswith("Always"):
                     print(f"Skipping optional component {name:>20}")
                 continue
         optional = "AlwaysOptional" in requiredlist
-        print(f"1 Required list is {requiredlist} optional is {optional}")
+
         if fxrequired in requiredlist:
             submodules[name].update()
             repodir = os.path.join(root_dir, submodules[name].path)
             if os.path.exists(os.path.join(repodir, ".gitmodules")):
                 # recursively handle this checkout
                 print(f"Recursively checking out submodules of {name}")
-                gitmodules = GitModules(submodules[name].logger, confpath=repodir)
+                gitsubmodules = GitModules(submodules[name].logger, confpath=repodir)
                 requiredlist = ["AlwaysRequired"]
                 if optional:
                     requiredlist.append("AlwaysOptional")
-                print(f"2 Required list is {requiredlist}")
-                submodules_update(gitmodules, repodir, requiredlist, force=force)
+
+                submodules_update(gitsubmodules, repodir, requiredlist, force=force, submodules=submodules)
 
 
 def local_mods_output():
diff --git a/git_fleximod/submodule.py b/git_fleximod/submodule.py
index 68fb08d6d..daa0fef09 100644
--- a/git_fleximod/submodule.py
+++ b/git_fleximod/submodule.py
@@ -102,8 +102,17 @@ def status(self):
                     needsupdate = True
                     return result, needsupdate, localmods, testfails                    
                 rurl = git.git_operation("ls-remote","--get-url").rstrip()
-                atag = git.git_operation("describe", "--tags", "--always").rstrip()
-                ahash =  git.git_operation("rev-list", "HEAD").partition("\n")[0]
+                line = git.git_operation("log", "--pretty=format:\"%h %d").partition('\n')[0]
+                parts = line.split()
+                ahash = parts[0][1:]
+                if len(parts) > 3:
+                    atag = parts[3][:-1]
+                else:
+                    atag = None
+                
+                #print(f"line is {line} ahash is {ahash} atag is {atag}")
+                #                atag = git.git_operation("describe", "--tags", "--always").rstrip()
+                # ahash =  git.git_operation("rev-list", "HEAD").partition("\n")[0]
                     
                 recurse = False
                 if rurl != self.url:

From dae1c82884feb69249dd0616c2a18131252ef4d2 Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Thu, 4 Jul 2024 12:45:32 -0600
Subject: [PATCH 8/9] now working with issue #50

---
 git_fleximod/git_fleximod.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/git_fleximod/git_fleximod.py b/git_fleximod/git_fleximod.py
index c4e595d85..2e282af43 100755
--- a/git_fleximod/git_fleximod.py
+++ b/git_fleximod/git_fleximod.py
@@ -215,9 +215,8 @@ def git_toplevelroot(root_dir, logger):
     superroot = rgit.git_operation("rev-parse", "--show-superproject-working-tree")
     return superroot
 
-def submodules_update(gitmodules, root_dir, requiredlist, force, submodules=None):
-    if not submodules:
-        submodules = {}
+def submodules_update(gitmodules, root_dir, requiredlist, force):
+    submodules = {}
     for name in gitmodules.sections():
         if not submodules or name not in submodules:
             submodules[name] = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
@@ -249,12 +248,11 @@ def submodules_update(gitmodules, root_dir, requiredlist, force, submodules=None
                 # recursively handle this checkout
                 print(f"Recursively checking out submodules of {name}")
                 gitsubmodules = GitModules(submodules[name].logger, confpath=repodir)
-                requiredlist = ["AlwaysRequired"]
+                newrequiredlist = ["AlwaysRequired"]
                 if optional:
-                    requiredlist.append("AlwaysOptional")
-
-                submodules_update(gitsubmodules, repodir, requiredlist, force=force, submodules=submodules)
+                    newrequiredlist.append("AlwaysOptional")
 
+                submodules_update(gitsubmodules, repodir, newrequiredlist, force=force)
 
 def local_mods_output():
     text = """\

From 53fce573bcad0ca1ae5c78013df8a42ad3414e5b Mon Sep 17 00:00:00 2001
From: Jim Edwards <jedwards@ucar.edu>
Date: Thu, 4 Jul 2024 16:10:22 -0600
Subject: [PATCH 9/9] no need to make submodule objects persist

---
 git_fleximod/git_fleximod.py | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/git_fleximod/git_fleximod.py b/git_fleximod/git_fleximod.py
index 2e282af43..e28499de8 100755
--- a/git_fleximod/git_fleximod.py
+++ b/git_fleximod/git_fleximod.py
@@ -188,19 +188,17 @@ def submodules_status(gitmodules, root_dir, toplevel=False, depth=0):
     testfails = 0
     localmods = 0
     needsupdate = 0
-    submodules = {}
     wrapper = textwrap.TextWrapper(initial_indent=' '*(depth*10), width=120,subsequent_indent=' '*(depth*20))
     for name in gitmodules.sections():
-        if not submodules or name not in submodules:
-            submodules[name] = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
+        submod = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
             
-        result,n,l,t = submodules[name].status()
+        result,n,l,t = submod.status()
         testfails += t
         localmods += l
         needsupdate += n
-        if toplevel or not submodules[name].toplevel():
+        if toplevel or not submod.toplevel():
             print(wrapper.fill(result))
-        subdir = os.path.join(root_dir, submodules[name].path)
+        subdir = os.path.join(root_dir, submod.path)
         if os.path.exists(os.path.join(subdir, ".gitmodules")):
             submod = GitModules(logger, confpath=subdir)
             t,l,n = submodules_status(submod, subdir, depth=depth+1)
@@ -216,15 +214,13 @@ def git_toplevelroot(root_dir, logger):
     return superroot
 
 def submodules_update(gitmodules, root_dir, requiredlist, force):
-    submodules = {}
     for name in gitmodules.sections():
-        if not submodules or name not in submodules:
-            submodules[name] = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
+        submod = init_submodule_from_gitmodules(gitmodules, name, root_dir, logger)
     
-        _, needsupdate, localmods, testfails = submodules[name].status()
-        if not submodules[name].fxrequired:
-            submodules[name].fxrequired = "AlwaysRequired"
-        fxrequired = submodules[name].fxrequired    
+        _, needsupdate, localmods, testfails = submod.status()
+        if not submod.fxrequired:
+            submod.fxrequired = "AlwaysRequired"
+        fxrequired = submod.fxrequired    
         allowedvalues = fxrequired_allowed_values()
         assert fxrequired in allowedvalues
 
@@ -242,12 +238,12 @@ def submodules_update(gitmodules, root_dir, requiredlist, force):
         optional = "AlwaysOptional" in requiredlist
 
         if fxrequired in requiredlist:
-            submodules[name].update()
-            repodir = os.path.join(root_dir, submodules[name].path)
+            submod.update()
+            repodir = os.path.join(root_dir, submod.path)
             if os.path.exists(os.path.join(repodir, ".gitmodules")):
                 # recursively handle this checkout
                 print(f"Recursively checking out submodules of {name}")
-                gitsubmodules = GitModules(submodules[name].logger, confpath=repodir)
+                gitsubmodules = GitModules(submod.logger, confpath=repodir)
                 newrequiredlist = ["AlwaysRequired"]
                 if optional:
                     newrequiredlist.append("AlwaysOptional")