Merge pull request #4254 from jasonb5/fix_find_gitroot

Fixes _find_gitroot for E3SM provenance

The function _find_gitroot would error when trying to determine the
models .git directory when being used as a submodule of another
project.

The error would occur when trying to copy a git metadata
file for provenance whose path was relative to the git repository.

This required two fixes, the first was to resolve the relative path and
the second was handling a third scenario for determining the model's
.git directory.

Test suite: scripts_regression_tests
Test baseline: n/a
Test namelist changes: n/a
Test status: n/a

Fixes #4251
User interface changes?: n
Update gh-pages html (Y/N)?: n

CIME/provenance.py

@@ -20,6 +20,7 @@
 )
 
 import tarfile, getpass, signal, glob, shutil, sys
+from contextlib import contextmanager
 
 logger = logging.getLogger(__name__)
 
@@ -85,40 +86,113 @@ def _run_git_cmd_recursively(cmd, srcroot, output):
         fd.write((output2 if rc2 == 0 else err2) + "\n")
 
 
-def _parse_dot_git_path(srcroot):
+def _parse_dot_git_path(gitdir):
+    """Parse `.git` path.
+
+    Take a path e.g. `/storage/cime/.git/worktrees/cime` and parse `.git`
+    directory e.g. `/storage/cime/.git`.
+
+    Args:
+        gitdir (str): Path containing `.git` directory.
+
+    Returns:
+        str: Path ending with `.git`
+    """
     dot_git_pattern = r"^(.*/\.git).*"
 
-    m = re.match(dot_git_pattern, srcroot)
+    m = re.match(dot_git_pattern, gitdir)
+
+    expect(m is not None, f"Could not parse .git from path {gitdir!r}")
+
+    return m.group(1)
+
+
+def _read_gitdir(gitroot):
+    """Read `gitdir` from `.git` file.
+
+    Reads `.git` file in a worktree or submodule and parse `gitdir`.
+
+    Args:
+        gitroot (str): Path ending with `.git` file.
+
+    Returns:
+        str: Path contained in `.git` file.
+    """
+    expect(os.path.isfile(gitroot), f"Expected {gitroot!r} to be a file")
+
+    with open(gitroot) as fd:
+        line = fd.readline()
+
+    gitdir_pattern = r"^gitdir:\s*(.*)$"
+
+    m = re.match(gitdir_pattern, line)
 
-    expect(m is not None, f"Could not parse git path from {srcroot!r}")
+    expect(m is not None, f"Could parse gitdir path from file {gitroot!r}")
 
     return m.group(1)
 
 
+@contextmanager
+def _swap_cwd(new_cwd):
+    old_cwd = os.getcwd()
+
+    os.chdir(new_cwd)
+
+    try:
+        yield
+    finally:
+        os.chdir(old_cwd)
+
+
 def _find_git_root(srcroot):
+    """Finds the `.git` directory.
+
+    NOTICE: It's assumed `srcroot` is an absolute path.
+
+    There are three scenarios to locate the correct `.git` directory:
+
+    NOTICE: In the 3rd case git `.git` directory is not actually `.git`.
+
+    1. In a standard git repository it will be located at `{srcroot}/.git`.
+    2. In a git worktree the `{srcroot}/.git` is a file containing a path,
+       `{gitdir}`, which the `{gitroot}` can be parsed from.
+    3. In a git submodule the `{srcroot}/.git` is a file containing a path,
+       `{gitdir}`, where the `{gitroot}` is `{gitdir}`.
+
+    To aid in finding the correct `{gitroot}` the file `{gitroot}/config`
+    is checked, this file will always be located in the correct directory.
+
+    Args:
+        srcroot (str): Path to the source root.
+
+    Returns:
+        str: Absolute path to `.git` directory.
+    """
     gitroot = f"{srcroot}/.git"
 
     expect(
         os.path.exists(gitroot),
         f"{srcroot!r} is not a git repository, failed to collect provenance",
     )
 
-    # Handle normal git repositories
+    # Handle 1st scenario
     if os.path.isdir(gitroot):
         return gitroot
 
-    # Handle git worktrees
-    with open(gitroot) as fd:
-        line = fd.readline()
+    # ensure we're in correct directory so abspath works correctly
+    with _swap_cwd(srcroot):
+        gitdir = os.path.abspath(_read_gitdir(gitroot))
 
-    gitdir_pattern = r"^gitdir:\s?(.*)$"
+    # Handle 3rd scenario, gitdir is the `.git` directory
+    config_path = os.path.join(gitdir, "config")
 
-    m = re.match(gitdir_pattern, line)
+    if os.path.exists(config_path):
+        return gitdir
 
-    expect(m is not None, f"Could not determine git root in {srcroot!r}")
+    # Handle 2nd scenario, gitdir should already be absolute path
+    parsed_gitroot = _parse_dot_git_path(gitdir)
 
-    # First group is the actual gitroot
-    return m.group(1)
+    return parsed_gitroot
 
 
 def _record_git_provenance(srcroot, exeroot, lid):
@@ -144,7 +218,6 @@ def _record_git_provenance(srcroot, exeroot, lid):
     _run_git_cmd_recursively("remote -v", srcroot, remote_prov)
 
     gitroot = _find_git_root(srcroot)
-    gitroot = _parse_dot_git_path(gitroot)
 
     # Git config
     config_src = os.path.join(gitroot, "config")

CIME/tests/test_unit_provenance.py

@@ -20,6 +20,43 @@ def test_parse_dot_git_path(self):
 
         assert value == "/src/CIME/.git"
 
+    def test_read_gitdir(self):
+        with tempfile.TemporaryDirectory() as tempdir:
+            dot_git_path = os.path.join(tempdir, ".git")
+
+            with open(dot_git_path, "w") as fd:
+                fd.write("gitdir:     /src/CIME/.git/worktrees/test")
+
+            value = provenance._read_gitdir(dot_git_path)
+
+            assert value == "/src/CIME/.git/worktrees/test"
+
+            with open(dot_git_path, "w") as fd:
+                fd.write("gitdir:/src/CIME/.git/worktrees/test")
+
+            value = provenance._read_gitdir(dot_git_path)
+
+            assert value == "/src/CIME/.git/worktrees/test"
+
+    def test_read_gitdir_not_file(self):
+        with tempfile.TemporaryDirectory() as tempdir:
+            dot_git_path = os.path.join(tempdir, ".git")
+
+            os.makedirs(dot_git_path)
+
+            with self.assertRaises(utils.CIMEError):
+                provenance._read_gitdir(dot_git_path)
+
+    def test_read_gitdir_bad_contents(self):
+        with tempfile.TemporaryDirectory() as tempdir:
+            dot_git_path = os.path.join(tempdir, ".git")
+
+            with open(dot_git_path, "w") as fd:
+                fd.write("some value: /src/CIME/.git/worktrees/test")
+
+            with self.assertRaises(utils.CIMEError):
+                provenance._read_gitdir(dot_git_path)
+
     def test_find_git_root(self):
         with tempfile.TemporaryDirectory() as tempdir:
             os.makedirs(os.path.join(tempdir, ".git"))
@@ -28,28 +65,62 @@ def test_find_git_root(self):
 
             assert value == f"{tempdir}/.git"
 
+    def test_find_git_root_does_not_exist(self):
+        with tempfile.TemporaryDirectory() as tempdir:
+            with self.assertRaises(utils.CIMEError):
+                provenance._find_git_root(tempdir)
+
+    def test_find_git_root_submodule(self):
+        with tempfile.TemporaryDirectory() as tempdir:
+            cime_path = os.path.join(tempdir, "cime")
+
+            os.makedirs(cime_path)
+
+            cime_git_dot_file = os.path.join(cime_path, ".git")
+
+            with open(cime_git_dot_file, "w") as fd:
+                fd.write(f"gitdir: {tempdir}/.git/modules/cime")
+
+            temp_dot_git_path = os.path.join(tempdir, ".git", "modules", "cime")
+
+            os.makedirs(temp_dot_git_path)
+
+            temp_config = os.path.join(temp_dot_git_path, "config")
+
+            with open(temp_config, "w") as fd:
+                fd.write("")
+
+            value = provenance._find_git_root(cime_path)
+
+            assert value == f"{tempdir}/.git/modules/cime"
+
+            # relative path
+            with open(cime_git_dot_file, "w") as fd:
+                fd.write(f"gitdir: ../.git/modules/cime")
+
+            value = provenance._find_git_root(cime_path)
+
+            assert value == f"{tempdir}/.git/modules/cime"
+
     def test_find_git_root_worktree(self):
         with tempfile.TemporaryDirectory() as tempdir:
-            with open(os.path.join(tempdir, ".git"), "w") as fd:
+            git_dot_file = os.path.join(tempdir, ".git")
+
+            with open(git_dot_file, "w") as fd:
                 fd.write("gitdir: /src/CIME/.git/worktrees/test")
 
             value = provenance._find_git_root(tempdir)
 
-            assert value == "/src/CIME/.git/worktrees/test"
+            assert value == "/src/CIME/.git"
 
-    def test_find_git_root_worktree_malformed(self):
+    def test_find_git_root_worktree_bad_contents(self):
         with tempfile.TemporaryDirectory() as tempdir:
             with open(os.path.join(tempdir, ".git"), "w") as fd:
                 fd.write("some value: /src/CIME/.git/worktrees/test")
 
             with self.assertRaises(utils.CIMEError):
                 provenance._find_git_root(tempdir)
 
-    def test_find_git_root_error(self):
-        with tempfile.TemporaryDirectory() as tempdir:
-            with self.assertRaises(utils.CIMEError):
-                provenance._find_git_root(tempdir)
-
     @mock.patch("CIME.provenance.run_cmd")
     def test_run_git_cmd_recursively(self, run_cmd):
         run_cmd.return_value = (0, "data", None)