Skip to content

Commit 32baeab

Browse files
codexByron
authored andcommitted
Address review feedback about repository discovery
Review feedback: relative GIT_COMMON_DIR left Git subprocesses resolving GIT_DIR and GIT_COMMON_DIR from a different working directory; malformed commondir data, empty GIT_OBJECT_DIRECTORY, and the linked-worktree signature were also handled inconsistently. Pin the repository environment to resolved paths, reject invalid metadata without consulting the process working directory, and restore HEAD-based linked-worktree detection. Keep the loose HEAD and dangling .git behavior because both match Git setup.c at baseline 15c6308cf7ad276b306aa5b3ababfbdebfb1a917. Validation: 3 focused tests and 9 subtests; Ruff check and format; mypy; compileall; git diff --check.
1 parent f2d1c4c commit 32baeab

3 files changed

Lines changed: 40 additions & 13 deletions

File tree

git/repo/base.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,9 @@ def __init__(
356356
raise InvalidGitRepositoryError(epath)
357357
self.git_dir = git_dir
358358

359-
common_dir = os.getenv("GIT_COMMON_DIR")
360-
if common_dir is not None:
361-
self._common_dir = osp.abspath(common_dir)
359+
common_dir_env = os.getenv("GIT_COMMON_DIR")
360+
if common_dir_env is not None:
361+
self._common_dir = osp.abspath(common_dir_env)
362362
else:
363363
try:
364364
common_dir = (Path(self.git_dir) / "commondir").read_text().splitlines()[0].strip()
@@ -381,6 +381,8 @@ def __init__(
381381

382382
self.working_dir: PathLike = self._working_tree_dir or self.common_dir
383383
self.git = self.GitCommandWrapperType(self.working_dir)
384+
if common_dir_env is not None:
385+
self.git.update_environment(GIT_DIR=os.fspath(self.git_dir), GIT_COMMON_DIR=os.fspath(self.common_dir))
384386

385387
# Special handling, in special times.
386388
rootpath = osp.join(self.common_dir, "objects")

git/repo/fun.py

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,24 +84,29 @@ def is_git_dir(d: PathLike) -> bool:
8484
)
8585

8686
common_dir = os.getenv("GIT_COMMON_DIR")
87+
if common_dir == "":
88+
return False
8789
if common_dir is None:
8890
try:
89-
common_dir = (Path(d) / "commondir").read_text().rstrip("\r\n")
91+
common_dir = os.fsdecode((Path(d) / "commondir").read_bytes()).rstrip("\r\n")
9092
except FileNotFoundError:
9193
common_dir = os.fspath(d)
9294
except OSError:
93-
common_dir = ""
95+
return False
9496
else:
95-
common_dir = osp.realpath(osp.join(d, common_dir)) if common_dir else ""
96-
97-
object_dir = os.getenv("GIT_OBJECT_DIRECTORY") or osp.join(common_dir, "objects")
97+
if not common_dir:
98+
return False
99+
try:
100+
common_dir = osp.realpath(osp.join(d, common_dir))
101+
except (OSError, ValueError):
102+
return False
103+
104+
object_dir = os.getenv("GIT_OBJECT_DIRECTORY")
105+
if object_dir is None:
106+
object_dir = osp.join(common_dir, "objects")
98107
if valid_head and osp.isdir(object_dir) and osp.isdir(osp.join(common_dir, "refs")):
99108
return True
100-
if (
101-
osp.isfile(osp.join(d, "gitdir"))
102-
and osp.isfile(osp.join(d, "commondir"))
103-
and osp.isfile(osp.join(d, "gitfile"))
104-
):
109+
if osp.isfile(osp.join(d, "gitdir")) and osp.isfile(osp.join(d, "commondir")) and osp.isfile(headref):
105110
raise WorkTreeRepositoryUnsupported(d)
106111
return False
107112

test/test_repo.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
from git.exc import UnsafeOptionError
4141
from git.exc import UnsafeProtocolError
4242
from git.exc import BadObject
43+
from git.exc import WorkTreeRepositoryUnsupported
4344
from git.repo.fun import touch
4445
from git.util import bin_to_hex, cwd, cygpath, join_path_native, rmfile, rmtree
4546

@@ -158,6 +159,23 @@ def test_repo_discovery_rejects_invalid_metadata(self):
158159
self.assertRaises(InvalidGitRepositoryError, Repo, path)
159160

160161
(path / "HEAD").write_text("ref: refs/heads/main\n")
162+
163+
for contents in (b"", b"\xff"):
164+
(path / "commondir").write_bytes(contents)
165+
with cwd(path), self.subTest(metadata="commondir", contents=contents):
166+
self.assertRaises(InvalidGitRepositoryError, Repo, path)
167+
168+
(path / "gitdir").write_text("../worktree/.git\n")
169+
(path / "commondir").write_text("missing\n")
170+
with self.subTest(metadata="linked-worktree"):
171+
self.assertRaises(WorkTreeRepositoryUnsupported, Repo, path)
172+
173+
(path / "gitdir").unlink()
174+
(path / "commondir").unlink()
175+
for variable in ("GIT_COMMON_DIR", "GIT_OBJECT_DIRECTORY"):
176+
with mock.patch.dict(os.environ, {variable: ""}), self.subTest(metadata=variable):
177+
self.assertRaises(InvalidGitRepositoryError, Repo, path)
178+
161179
(path / ".git").write_text("not a gitfile")
162180
with self.subTest(metadata=".git"):
163181
self.assertRaises(InvalidGitRepositoryError, Repo, path)
@@ -180,6 +198,8 @@ def test_repo_discovery_uses_git_common_dir(self):
180198
assert osp.samefile(repo.common_dir, common_dir)
181199
assert osp.samefile(repo.odb.root_path(), common_dir / "objects")
182200
assert repo.bare
201+
assert osp.samefile(repo.git.rev_parse("--absolute-git-dir"), git_dir)
202+
assert osp.samefile(repo.git.rev_parse("--git-common-dir"), common_dir)
183203

184204
@with_rw_repo("0.3.2.1")
185205
def test_repo_creation_from_different_paths(self, rw_repo):

0 commit comments

Comments
 (0)