Improve GitFileSystem to support symlinks and source files located in submodules

CHANGES.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+* Improve `GitFileSystem` to support symbolic links by using `git cat-file
+  --batch --follow-symlinks`.
+* Improve `GitFileSystem` to support source files located in submodules.
+* Use a slightly darker yellow in the HTML report for the numbers of
+  partially covered lines, for readability.
+
 * Add support for Python 3.12 and 3.13.
 
 ## 4.0.0 (2025-04-13)

pycobertura/filesystem.py

@@ -3,7 +3,6 @@
 import io
 import zipfile
 import subprocess
-import shlex
 
 from contextlib import contextmanager
 
@@ -81,56 +80,172 @@ def __init__(self, repo_folder, ref):
         # the report may have been collected in a subfolder of the repository
         # root. Each file path shall thus be completed by the prefix.
         self.prefix = self.repository.replace(self.repository_root, "").lstrip("/")
+        # Cache submodule paths and commit SHAs for the provided ref
+        self._submodules = self._discover_submodules()
+
+    def _git_cat_file_check(self, repo_root, spec):
+        """
+        Call `git cat-file --batch-check --follow-symlinks`
+        and return existence as bool.
+        """
+        args = ["git", "cat-file", "--batch-check", "--follow-symlinks"]
+        input_data = f"{spec}\n".encode()
+        try:
+            process = subprocess.Popen(
+                args,
+                cwd=repo_root,
+                stdin=subprocess.PIPE,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+            )
+            output, _ = process.communicate(input=input_data)
+            return_code = process.wait()
+        except (OSError, subprocess.CalledProcessError):
+            return False
+        return return_code == 0 and not output.endswith(b"missing\n")
+
+    def _git_cat_file_read(self, repo_root, spec):
+        """
+        Call `git cat-file --batch --follow-symlinks` and return blob content as bytes.
+        Raises FileNotFound if the object is missing or on error.
+        """
+        args = ["git", "cat-file", "--batch", "--follow-symlinks"]
+        input_data = f"{spec}\n".encode()
+        try:
+            process = subprocess.Popen(
+                args,
+                cwd=repo_root,
+                stdin=subprocess.PIPE,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+            )
+            output, _ = process.communicate(input=input_data)
+            return_code = process.wait()
+        except (OSError, subprocess.CalledProcessError):
+            raise self.FileNotFound(spec)
+
+        if return_code != 0 or output.endswith(b"missing\n"):
+            raise self.FileNotFound(spec)
+        lines = output.split(b"\n", 1)
+        if len(lines) < 2:
+            raise self.FileNotFound(spec)
+        return lines[1]
 
     def real_filename(self, filename):
-        prefix = "{}/".format(self.prefix) if self.prefix else ""
-        return "{ref}:{prefix}{filename}".format(
-            prefix=prefix, ref=self.ref, filename=filename
-        )
+        """
+        Constructs the Git path for a given filename.
+        This method should NOT resolve symlinks on the local disk.
+        """
+        prefix = f"{self.prefix}/" if self.prefix else ""
+        return f"{self.ref}:{prefix}{filename}"
 
     def has_file(self, filename):
-        command = "git --no-pager show {}".format(self.real_filename(filename))
-        return_code = subprocess.call(
-            command,
-            cwd=self.repository,
-            shell=True,
-            stdout=subprocess.DEVNULL,
-            stderr=subprocess.DEVNULL,
-        )
-        return not bool(return_code)
+        """
+        Check for a file's existence in the specified commit's tree.
+        """
+        # If the file is within a submodule, query the submodule repository
+        submodule_ctx = self._resolve_submodule_ctx(filename)
+        if submodule_ctx is not None:
+            submodule_root, sub_commit, rel_path = submodule_ctx
+            return self._git_cat_file_check(submodule_root, f"{sub_commit}:{rel_path}")
+
+        real_filename = self.real_filename(filename)
+        return self._git_cat_file_check(self.repository_root, real_filename)
 
     def _get_root_path(self, repository_folder):
-        command = "git rev-parse --show-toplevel"
-        command_tokens = shlex.split(command)
+        command = ["git", "rev-parse", "--show-toplevel"]
         try:
-            output = subprocess.check_output(command_tokens, cwd=repository_folder)
+            output = subprocess.check_output(command, cwd=repository_folder)
         except (OSError, subprocess.CalledProcessError):
             raise ValueError(
-                "The folder {} is not "
-                "a valid git repository.".format(repository_folder)
+                f"The folder {repository_folder} is not a valid git repository."
             )
-
         return output.decode("utf-8").rstrip()
 
     @contextmanager
     def open(self, filename):
         """
-        Yield a file-like object for file `filename`.
+        Yield a file-like object for the given filename,
+        following symlinks if necessary.
 
         This function is a context manager.
         """
-        filename = self.real_filename(filename)
+        # If the file is within a submodule,
+        # read from the submodule repository at the pinned commit
+        submodule_ctx = self._resolve_submodule_ctx(filename)
+        if submodule_ctx is not None:
+            submodule_root, sub_commit, rel_path = submodule_ctx
+            content = self._git_cat_file_read(
+                submodule_root, f"{sub_commit}:{rel_path}"
+            )
+            yield io.StringIO(content.decode("utf-8"))
+            return
 
-        command = "git --no-pager show {}".format(filename)
-        command_tokens = shlex.split(command)
+        real_filename = self.real_filename(filename)
+        content = self._git_cat_file_read(self.repository_root, real_filename)
+        yield io.StringIO(content.decode("utf-8"))
 
+    def _discover_submodules(self):
+        """
+        Discover submodule paths and SHAs for the given ref by inspecting the tree.
+        Returns a mapping of submodule path -> commit SHA.
+        """
         try:
-            output = subprocess.check_output(command_tokens, cwd=self.repository)
+            output = subprocess.check_output(
+                [
+                    "git",
+                    "ls-tree",
+                    "-r",
+                    "--full-tree",
+                    self.ref,
+                ],
+                cwd=self.repository_root,
+            )
         except (OSError, subprocess.CalledProcessError):
-            raise self.FileNotFound(filename)
-
-        output = output.decode("utf-8").rstrip()
-        yield io.StringIO(output)
+            return {}
+
+        submodules = {}
+        for line in output.decode("utf-8", errors="replace").splitlines():
+            # Expected format: "160000 commit <sha>\t<path>"
+            try:
+                meta, path = line.split("\t", 1)
+            except ValueError:
+                continue
+            parts = meta.split()
+            if len(parts) < 3:
+                continue
+            mode, obj_type, sha = parts[0], parts[1], parts[2]
+            if mode == "160000" and obj_type == "commit":
+                submodules[path] = sha
+        return submodules
+
+    def _resolve_submodule_ctx(self, filename):
+        """
+        If the path points into a submodule, return a tuple of
+        (submodule_root_abs_path, submodule_commit_sha, relative_path_inside_submodule).
+        Otherwise, return None.
+        """
+        # Find the longest matching submodule path that prefixes the filename
+        matching = [
+            p
+            for p in self._submodules.keys()
+            if filename == p or filename.startswith(p + "/")
+        ]
+        if not matching:
+            return None
+        # Use the longest (deepest) match in case of nested submodules
+        sub_path = max(matching, key=len)
+        sub_sha = self._submodules.get(sub_path)
+        if not sub_sha:
+            return None
+
+        rel_path = filename.removeprefix(sub_path).lstrip("/")
+        submodule_root = os.path.join(self.repository_root, sub_path)
+        if not os.path.isdir(submodule_root):
+            # Submodule not checked out;
+            # we cannot read without local checkout of objects
+            return None
+        return (submodule_root, sub_sha, rel_path)
 
 
 def filesystem_factory(source, source_prefix=None, ref=None):

pycobertura/templates/html-delta.jinja2

@@ -7,7 +7,7 @@
 {% include 'skeleton.css' %}
 .red {color: red}
 .green {color: green}
-.yellow {color: yellow}
+.yellow {color: #FFD700}
 .hit {background-color: #E6FFEC}
 .miss {background-color: #FFEBE9}
 .partial {background-color: #FFFECD}

tests/test_filesystem.py

@@ -1,5 +1,6 @@
 import subprocess
 from unittest.mock import patch, MagicMock
+import pytest
 
 
 FIRST_PYCOBERTURA_COMMIT_SHA = "d1fe88da6b18340762b24bb1f89067a3439c4041"
@@ -121,21 +122,30 @@ def test_filesystem_git():
     import pycobertura.filesystem as fsm
 
     branch, folder, filename = "master", "tests/dummy", "test-file"
+    repo_root = "/tmp/repo"
 
     with patch.object(fsm, "subprocess") as subprocess_mock:
-        subprocess_mock.check_output = MagicMock(return_value=b"<file-content>")
+        # Mock for _get_root_path
+        subprocess_mock.check_output.return_value = repo_root.encode('utf-8')
+
+        # Mock for open
+        mock_process = MagicMock()
+        mock_process.communicate.return_value = ('some_hash blob 12\n<file-content>'.encode(), b'')
+        mock_process.wait.return_value = 0
+        subprocess_mock.Popen.return_value = mock_process
 
         fs = fsm.GitFileSystem(folder, branch)
 
         with fs.open(filename) as f:
             assert hasattr(f, 'read')
 
-        expected_git_filename = "master:tests/dummy/test-file"
+        expected_git_filename = f"{branch}:{folder}/{filename}"
         git_filename = fs.real_filename(filename)
         assert git_filename == expected_git_filename
 
-        expected_command = ["git", "--no-pager", "show", git_filename]
-        subprocess_mock.check_output.assert_called_with(expected_command, cwd=folder)
+        expected_command = ["git", "cat-file", "--batch", "--follow-symlinks"]
+        subprocess_mock.Popen.assert_called_with(expected_command, cwd=repo_root, stdin=subprocess_mock.PIPE,
+                                                 stdout=subprocess_mock.PIPE, stderr=subprocess_mock.PIPE)
 
 
 def test_filesystem_git_integration():
@@ -184,22 +194,6 @@ def test_filesystem_git__git_not_found():
             assert folder in str(e)
 
 
-def test_filesystem_git_integration():
-    from pycobertura.filesystem import GitFileSystem
-
-    fs = GitFileSystem(".", FIRST_PYCOBERTURA_COMMIT_SHA)
-
-    # Files included in pycobertura's first commit.
-    source_files = [
-        "README.md",
-        ".gitignore",
-    ]
-
-    for source_file in source_files:
-        with fs.open(source_file) as f:
-            assert hasattr(f, "read")
-
-
 def test_filesystem_git_has_file_integration():
     from pycobertura.filesystem import GitFileSystem
 
@@ -222,11 +216,11 @@ def test_filesystem_git_integration__not_found():
 
     dummy_source_file = "CHANGES.md"
 
-    try:
+    with pytest.raises(GitFileSystem.FileNotFound) as excinfo:
         with fs.open(dummy_source_file) as f:
             pass
-    except GitFileSystem.FileNotFound as fnf:
-        assert fnf.path == fs.real_filename(dummy_source_file)
+
+    assert excinfo.value.path == fs.real_filename(dummy_source_file)
 
 
 def test_filesystem_git_has_file_integration__not_found():

tests/test_reporters.py

@@ -528,7 +528,7 @@ def test_html_report_delta():
     html_output = report_delta.generate()
     assert '.red {color: red}' in html_output
     assert '.green {color: green}' in html_output
-    assert '.yellow {color: yellow}' in html_output
+    assert '.yellow {color: #FFD700}' in html_output
     assert "normalize.css" in html_output
     assert "Skeleton V2.0" in html_output