Address review feedback about Gitfile handling · gitpython-developers/GitPython@b8c000e · GitHub
Skip to content

Commit b8c000e

Browse files
codexByron
authored andcommitted
Address review feedback about Gitfile handling
Review feedback identified that chained or self-referential .git pointers recurse, filesystem-encoded metadata can fail text decoding, and a relative GIT_DIR is not retained for later Git commands. Parse one regular, size-bounded Gitfile exactly once, decode Gitfile and commondir paths with the filesystem codec, and retain the resolved GIT_DIR for subprocesses. This rejects cycles like Git instead of recursing and keeps commands stable after working-directory changes. Git baseline: 15c6308cf7ad276b306aa5b3ababfbdebfb1a917, setup.c read_gitfile_gently() and get_common_dir_noenv(). Validation: 7 focused tests and 12 subtests; Ruff check and format; mypy; compileall; git diff --check.
1 parent 32baeab commit b8c000e

3 files changed

Lines changed: 45 additions & 34 deletions

File tree

git/repo/base.py

Lines changed: 5 additions & 2 deletions

git/repo/fun.py

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -117,44 +117,33 @@ def find_worktree_git_dir(dotgit: PathLike) -> Optional[str]:
117117
statbuf = os.stat(dotgit)
118118
except OSError:
119119
return None
120-
if not stat.S_ISREG(statbuf.st_mode):
120+
if not stat.S_ISREG(statbuf.st_mode) or statbuf.st_size > (1 << 20):
121121
return None
122122

123123
try:
124-
lines = Path(dotgit).read_text().splitlines()
125-
for key, value in [line.strip().split(": ") for line in lines]:
126-
if key == "gitdir":
127-
return value
128-
except ValueError:
129-
pass
130-
return None
124+
content = os.fsdecode(Path(dotgit).read_bytes()).rstrip("\r\n")
125+
except OSError:
126+
return None
127+
return content[8:] if len(content) >= 9 and content.startswith("gitdir: ") else None
131128

132129

133130
def find_submodule_git_dir(d: PathLike) -> Optional[PathLike]:
134131
"""Search for a submodule repo."""
135132
if is_git_dir(d):
136133
return d
137134

138-
try:
139-
with open(d) as fp:
140-
content = fp.read().rstrip()
141-
except IOError:
142-
# It's probably not a file.
143-
pass
144-
else:
145-
if content.startswith("gitdir: "):
146-
path = content[8:]
147-
148-
if Git.is_cygwin():
149-
# Cygwin creates submodules prefixed with `/cygdrive/...`.
150-
# Cygwin git understands Cygwin paths much better than Windows ones.
151-
# Also the Cygwin tests are assuming Cygwin paths.
152-
path = cygpath(path)
153-
if not osp.isabs(path):
154-
path = osp.normpath(osp.join(osp.dirname(d), path))
155-
return find_submodule_git_dir(path)
156-
# END handle exception
157-
return None
135+
path = find_worktree_git_dir(d)
136+
if path is None:
137+
return None
138+
139+
if Git.is_cygwin():
140+
# Cygwin creates submodules prefixed with `/cygdrive/...`.
141+
# Cygwin git understands Cygwin paths much better than Windows ones.
142+
# Also the Cygwin tests are assuming Cygwin paths.
143+
path = cygpath(path)
144+
if not osp.isabs(path):
145+
path = osp.normpath(osp.join(osp.dirname(d), path))
146+
return path if is_git_dir(path) else None
158147

159148

160149
def short_to_long(odb: "GitCmdObjectDB", hexsha: str) -> Optional[bytes]:

test/test_repo.py

Lines changed: 23 additions & 4 deletions

0 commit comments

Comments
 (0)