Address review feedback about repository setup

Review feedback identified four setup mismatches: non-missing .git stat failures could fall through to another repository, explicit GIT_DIR could be redirected through a nested .git entry, Gitfile reads were not bounded to the stat-reported size, and alternates ignored GIT_OBJECT_DIRECTORY.

Match Git setup.c by bypassing discovery for an environment-selected GIT_DIR, stopping discovery when Gitfile stat fails for reasons other than a missing path, and reading exactly the previously observed Gitfile size. Resolve alternates below the active ODB root so custom object stores remain internally consistent.

The regressions cover all four cases. This follows Git baseline 15c6308cf7ad276b306aa5b3ababfbdebfb1a917 in setup_git_directory_gently_1() and read_gitfile_gently().

Validation: 11 focused tests plus 12 subtests; Ruff check and format; mypy; compileall; git diff --check.

Author: codex

git/repo/base.py

@@ -295,23 +295,28 @@ def __init__(
         # Walk up the path to find the `.git` dir.
         curpath = os.fspath(epath) if epath is not None else ""
         git_dir: Optional[str] = None
+        explicit_git_dir = path is None and git_dir_env is not None
         while curpath:
             # ABOUT osp.NORMPATH
             # It's important to normalize the paths, as submodules will otherwise
             # initialize their repo instances with paths that depend on path-portions
             # that will not exist after being removed. It's just cleaner.
-            dotgit = osp.join(curpath, ".git")
-            sm_gitpath = find_submodule_git_dir(dotgit)
-            if sm_gitpath is not None:
-                # Worktrees can use relative paths as of Git 2.48, so join to curpath.
-                git_dir = osp.normpath(osp.join(curpath, os.fspath(sm_gitpath)))
-                self._working_tree_dir = curpath
-                break
+            if not explicit_git_dir:
+                dotgit = osp.join(curpath, ".git")
+                try:
+                    sm_gitpath = find_submodule_git_dir(dotgit)
+                except OSError:
+                    break
+                if sm_gitpath is not None:
+                    # Worktrees can use relative paths as of Git 2.48, so join to curpath.
+                    git_dir = osp.normpath(osp.join(curpath, os.fspath(sm_gitpath)))
+                    self._working_tree_dir = curpath
+                    break
 
-            # Like Git, do not fall back to a bare repository or parent directory when
-            # a non-directory .git entry exists but is not a valid gitfile.
-            if osp.exists(dotgit) and not osp.isdir(dotgit):
-                break
+                # Like Git, do not fall back to a bare repository or parent directory when
+                # a non-directory .git entry exists but is not a valid gitfile.
+                if osp.exists(dotgit) and not osp.isdir(dotgit):
+                    break
 
             if is_git_dir(curpath):
                 git_dir = curpath
@@ -349,7 +354,7 @@ def __init__(
                     self._working_tree_dir = os.getenv("GIT_WORK_TREE")
                 break
 
-            if not search_parent_directories:
+            if explicit_git_dir or not search_parent_directories:
                 break
             curpath, tail = osp.split(curpath)
             if not tail:
@@ -994,7 +999,7 @@ def _get_alternates(self) -> List[str]:
         :return:
             List of strings being pathnames of alternates
         """
-        alternates_path = osp.join(self.common_dir, "objects", "info", "alternates")
+        alternates_path = osp.join(self.odb.root_path(), "info", "alternates")
 
         if osp.exists(alternates_path):
             with open(alternates_path, "rb") as f:
@@ -1015,7 +1020,7 @@ def _set_alternates(self, alts: List[str]) -> None:
             The method does not check for the existence of the paths in `alts`, as the
             caller is responsible.
         """
-        alternates_path = osp.join(self.common_dir, "objects", "info", "alternates")
+        alternates_path = osp.join(self.odb.root_path(), "info", "alternates")
         if not alts:
             if osp.isfile(alternates_path):
                 os.remove(alternates_path)

git/repo/fun.py

@@ -118,13 +118,17 @@ def find_worktree_git_dir(dotgit: PathLike) -> Optional[str]:
     """Search for a gitdir for this worktree."""
     try:
         statbuf = os.stat(dotgit)
-    except OSError:
+    except (FileNotFoundError, NotADirectoryError):
         return None
     if not stat.S_ISREG(statbuf.st_mode) or statbuf.st_size > (1 << 20):
         return None
 
     try:
-        content = os.fsdecode(Path(dotgit).read_bytes()).rstrip("\r\n")
+        with open(dotgit, "rb") as fp:
+            content_bytes = fp.read(statbuf.st_size)
+        if len(content_bytes) != statbuf.st_size:
+            return None
+        content = os.fsdecode(content_bytes).rstrip("\r\n")
     except (OSError, UnicodeError):
         return None
     return content[8:] if len(content) >= 9 and content.startswith("gitdir: ") else None

test/test_repo.py

@@ -41,7 +41,7 @@
 from git.exc import UnsafeProtocolError
 from git.exc import BadObject
 from git.exc import WorkTreeRepositoryUnsupported
-from git.repo.fun import touch
+from git.repo.fun import find_worktree_git_dir, touch
 from git.util import bin_to_hex, cwd, cygpath, join_path_native, rmfile, rmtree
 
 from test.lib import TestBase, fixture, requires_symlinks, with_rw_directory, with_rw_repo, PathLikeMock
@@ -148,6 +148,16 @@ def test_repo_discovery_prefers_dotgit(self):
                     expected_git_dir = Git(path).rev_parse("--absolute-git-dir")
                     assert osp.samefile(Repo(path).git_dir, expected_git_dir)
 
+    def test_repo_discovery_honors_explicit_git_dir(self):
+        with tempfile.TemporaryDirectory() as tdir:
+            git_dir = Path(tdir) / "repo.git"
+            Repo.init(git_dir, bare=True).close()
+            Repo.init(git_dir / ".git", bare=True).close()
+
+            with mock.patch.dict(os.environ, {"GIT_DIR": os.fspath(git_dir)}):
+                with Repo() as repo:
+                    assert osp.samefile(repo.git_dir, git_dir)
+
     def test_repo_discovery_rejects_invalid_metadata(self):
         with tempfile.TemporaryDirectory() as tdir:
             path = Path(tdir)
@@ -192,6 +202,29 @@ def test_repo_discovery_rejects_dangling_commondir(self):
 
             self.assertRaises(InvalidGitRepositoryError, Repo, path)
 
+    @requires_symlinks
+    def test_repo_discovery_rejects_dotgit_stat_errors(self):
+        with tempfile.TemporaryDirectory() as tdir:
+            path = Path(tdir)
+            Repo.init(path).close()
+            child = path / "child"
+            child.mkdir()
+            (child / ".git").symlink_to(".git")
+
+            self.assertRaises(InvalidGitRepositoryError, Repo, child, search_parent_directories=True)
+
+    def test_gitfile_read_is_bounded(self):
+        with tempfile.TemporaryDirectory() as tdir:
+            dotgit = Path(tdir) / ".git"
+            content = b"gitdir: target\n"
+            dotgit.write_bytes(content)
+            reader = mock.mock_open(read_data=b"")
+
+            with mock.patch("builtins.open", reader):
+                assert find_worktree_git_dir(dotgit) is None
+
+            reader().read.assert_called_once_with(len(content))
+
     def test_repo_discovery_uses_storage_environment(self):
         with tempfile.TemporaryDirectory() as tdir:
             git_dir = Path(tdir) / "git"
@@ -251,6 +284,9 @@ def test_repo_discovery_preserves_object_directory(self):
                 assert osp.samefile(repo.odb.root_path(), object_dir)
                 assert repo.odb.info(bytes.fromhex(blob_hexsha)).size == len(payload)
                 assert repo.git.cat_file("blob", blob_hexsha) == payload.decode()
+                repo.alternates = ["other/location"]
+                assert repo.alternates == ["other/location"]
+                assert (object_dir / "info" / "alternates").is_file()
 
     @with_rw_repo("0.3.2.1")
     def test_repo_creation_from_different_paths(self, rw_repo):
@@ -519,6 +555,7 @@ def test_alternates_use_common_dir(self, rw_dir):
         os.makedirs(osp.join(common_dir, "objects", "info"))
         os.makedirs(osp.join(git_dir, "objects", "info"))
         repo = mock.Mock(common_dir=common_dir, git_dir=git_dir)
+        repo.odb.root_path.return_value = osp.join(common_dir, "objects")
 
         alts = ["other/location", "this/location"]
         Repo._set_alternates(repo, alts)