FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(repo): delegate revision message searches to Git · gitpython-developers/GitPython@3ca35bf · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

Commit 3ca35bf

Browse files
codex
authored and
Byron
committed
fix(repo): delegate revision message searches to Git
<!-- Byron --> rubber-stamp, after checking diff quickly. <!-- agent --> Python evaluated user-supplied `:/pattern` and `^{/pattern}` searches with a backtracking regex engine and walked/deserialized history itself. A short expression could occupy a worker indefinitely on an ordinary repository (finding 21500). Delegate matching and traversal to `git rev-parse`, using its extended regular expressions, and delete the Python regex/history implementation. Construct fixed-prefix operands so they cannot become options, preserving support for Git versions without `--end-of-options`. Invalid expressions now raise `BadName`, like searches without a match. Resolve revisions before peeling objects in `Repo.commit()` and `Repo.tree()`; appending a peel suffix previously changed top-level search patterns. Add bounded subprocess regressions, native-Git parity cases, and checks that history is not deserialized in Python. Validation: 27 focused/related tests and Ruff checks passed. A 56-case comparison preserved previously successful ordinary revision resolutions. Native behavior was checked against Git and its local source at `d38352cd43`.
1 parent 0b64501 commit 3ca35bf

4 files changed

Lines changed: 137 additions & 116 deletions

File tree

‎.basedpyright/baseline.json‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -373,22 +373,6 @@
373373
}
374374
],
375375
"./git/repo/base.py": [
376-
{
377-
"code": "reportReturnType",
378-
"range": {
379-
"startColumn": 15,
380-
"endColumn": 46,
381-
"lineCount": 1
382-
}
383-
},
384-
{
385-
"code": "reportReturnType",
386-
"range": {
387-
"startColumn": 15,
388-
"endColumn": 51,
389-
"lineCount": 1
390-
}
391-
},
392376
{
393377
"code": "reportReturnType",
394378
"range": {

‎git/repo/base.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
find_submodule_git_dir,
4949
is_git_dir,
5050
rev_parse,
51+
to_commit,
5152
touch,
5253
)
5354

@@ -821,7 +822,7 @@ def commit(self, rev: Union[str, Commit_ish, None] = None) -> Commit:
821822
"""
822823
if rev is None:
823824
return self.head.commit
824-
return self.rev_parse(str(rev) + "^0")
825+
return to_commit(self.rev_parse(str(rev)))
825826

826827
def iter_trees(self, *args: Any, **kwargs: Any) -> Iterator["Tree"]:
827828
""":return: Iterator yielding :class:`~git.objects.tree.Tree` objects
@@ -851,7 +852,8 @@ def tree(self, rev: Union[Tree_ish, str, None] = None) -> "Tree":
851852
"""
852853
if rev is None:
853854
return self.head.commit.tree
854-
return self.rev_parse(str(rev) + "^{tree}")
855+
obj = self.rev_parse(str(rev))
856+
return obj if obj.type == "tree" else to_commit(obj).tree
855857

856858
def iter_commits(
857859
self,

‎git/repo/fun.py‎

Lines changed: 19 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@
2727
from gitdb.exc import BadName, BadObject
2828

2929
from git.cmd import Git
30-
from git.exc import WorkTreeRepositoryUnsupported
30+
from git.exc import GitCommandError, WorkTreeRepositoryUnsupported
3131
from git.objects import Object
3232
from git.objects.util import parse_date
3333
from git.refs import SymbolicReference
3434
from git.util import cygpath, bin_to_hex, hex_to_bin
3535

3636
# Typing ----------------------------------------------------------------------
3737

38-
from typing import Iterator, Optional, TYPE_CHECKING, Tuple, Union, cast, overload
38+
from typing import Optional, TYPE_CHECKING, Union, cast, overload
3939

4040
from git.types import AnyGitObject, Literal, PathLike
4141

@@ -472,100 +472,23 @@ def _find_closing_brace(rev: str, start: int) -> int:
472472
raise ValueError("Missing closing brace to define type in %s" % rev)
473473

474474

475-
def _parse_search(pattern: str) -> Tuple[str, bool]:
475+
def _find_commit_by_message(repo: "Repo", rev: Optional[AnyGitObject], pattern: str) -> AnyGitObject:
476476
if not pattern:
477477
raise ValueError("Revision search requires a pattern")
478-
# END handle empty pattern
479-
480-
if pattern.startswith("!-"):
481-
return pattern[2:], True
482-
if pattern.startswith("!!"):
483-
return pattern[1:], False
484-
if pattern.startswith("!"):
478+
if pattern.startswith("!") and not pattern.startswith(("!-", "!!")):
485479
raise ValueError("Need one character after /!, typically -")
486-
return pattern, False
487-
488-
489-
def _unescape_braced_regex(pattern: str) -> str:
490-
out = []
491-
idx = 0
492-
while idx < len(pattern):
493-
char = pattern[idx]
494-
if char == "\\" and idx + 1 < len(pattern):
495-
next_char = pattern[idx + 1]
496-
if next_char in "{}\\":
497-
out.append(next_char)
498-
else:
499-
out.append(char)
500-
out.append(next_char)
501-
# END handle escaped char
502-
idx += 2
503-
continue
504-
# END handle backslash
505-
out.append(char)
506-
idx += 1
507-
# END for each char
508-
return "".join(out)
509-
510480

511-
def _find_commit_by_message(
512-
repo: "Repo", rev: Optional[AnyGitObject], pattern: str, braced: bool = False
513-
) -> AnyGitObject:
514-
pattern, negated = _parse_search(_unescape_braced_regex(pattern) if braced else pattern)
515-
try:
516-
regex = re.compile(pattern)
517-
except re.error as e:
518-
raise ValueError("Invalid commit message regex %r" % pattern) from e
519-
# END handle invalid regex
520-
if rev is None:
521-
commits = _all_ref_commits(repo)
522-
else:
523-
commits = _reachable_commits([to_commit(rev)])
524-
# END handle starting point
525-
526-
for commit in commits:
527-
message = commit.message
528-
if isinstance(message, bytes):
529-
message = message.decode(commit.encoding, "replace")
530-
# END handle bytes message
531-
matches = regex.search(message or "") is not None
532-
if matches != negated:
533-
return commit
534-
# END found commit
535-
# END for each commit
536-
raise BadName("No commit found matching message pattern %r" % pattern)
537-
538-
539-
def _all_ref_commits(repo: "Repo") -> Iterator["Commit"]:
540-
starts = []
541-
for ref in repo.references:
542-
try:
543-
starts.append(to_commit(ref.object))
544-
except (BadName, ValueError):
545-
pass
546-
# END skip refs that do not point to commits
547-
# END for each ref
481+
# Git's native regular expressions avoid Python's exponential backtracking,
482+
# and its history walk does not deserialize every visited commit in Python.
483+
search = ":/" + pattern if rev is None else "%s^{/%s}" % (to_commit(rev).hexsha, pattern)
548484
try:
549-
starts.append(repo.head.commit)
550-
except ValueError:
551-
pass
552-
# END handle unborn head
553-
return _reachable_commits(starts)
554-
555-
556-
def _reachable_commits(starts: list["Commit"]) -> Iterator["Commit"]:
557-
seen = set()
558-
pending = starts[:]
559-
while pending:
560-
pending.sort(key=lambda commit: commit.committed_date, reverse=True)
561-
commit = pending.pop(0)
562-
if commit.binsha in seen:
563-
continue
564-
# END skip seen commit
565-
seen.add(commit.binsha)
566-
yield commit
567-
pending.extend(commit.parents)
568-
# END while commits remain
485+
# The fixed prefix prevents options without requiring Git 2.30's
486+
# --end-of-options support in rev-parse.
487+
hexsha = repo.git.rev_parse("--verify", search)
488+
except GitCommandError as e:
489+
# Git does not distinguish invalid regexes from searches with no match.
490+
raise BadName("No commit found matching message pattern %r" % pattern) from e
491+
return _object_from_hexsha(repo, hexsha)
569492

570493

571494
def _index_lookup(repo: "Repo", spec: str) -> AnyGitObject:
@@ -597,7 +520,7 @@ def _tree_lookup(obj: AnyGitObject, path: str) -> AnyGitObject:
597520

598521
def _peel(obj: AnyGitObject, output_type: str, repo: "Repo", rev: str) -> AnyGitObject:
599522
if output_type.startswith("/"):
600-
return _find_commit_by_message(repo, obj, output_type[1:], braced=True)
523+
return _find_commit_by_message(repo, obj, output_type[1:])
601524
if output_type == "":
602525
return deref_tag(obj) if obj.type == "tag" else obj
603526
if output_type == "object":
@@ -654,6 +577,10 @@ def rev_parse(repo: "Repo", rev: str) -> AnyGitObject:
654577
:manpage:`git-rev-parse(1)`-compatible revision specification as string.
655578
Please see :manpage:`git-rev-parse(1)` for details.
656579
580+
Commit message searches use Git's native extended regular expressions.
581+
Invalid search expressions and searches without a match both raise
582+
:exc:`~gitdb.exc.BadName`.
583+
657584
:raise gitdb.exc.BadObject:
658585
If the given revision could not be found.
659586

‎test/test_rev_parse.py‎

Lines changed: 114 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@
33
# This module is part of GitPython and is released under the
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

6+
import subprocess
7+
import sys
68
from pathlib import Path
79

810
import pytest
9-
10-
from git import Repo
11-
from git.refs import RemoteReference
12-
from git.refs import SymbolicReference
1311
from gitdb.exc import BadName
1412

13+
from git import Actor, Commit, Repo
14+
from git.refs import RemoteReference, SymbolicReference
15+
1516

1617
def _write(repo, path, content):
1718
full_path = Path(repo.working_tree_dir) / path
@@ -140,17 +141,124 @@ def test_rev_parse_commit_message_search(rev_parse_repo):
140141
assert repo.rev_parse("HEAD^{/!-release}") == merge
141142

142143

144+
@pytest.mark.parametrize(
145+
"revision",
146+
[
147+
":/release[[:space:]]candidate",
148+
"HEAD^{/release{1}}",
149+
"ann^{/root}",
150+
":/!-release",
151+
"HEAD^{/!-release}",
152+
"HEAD^{/release}^0",
153+
"HEAD^{/release}^{tree}",
154+
],
155+
)
156+
def test_rev_parse_commit_message_search_matches_git(rev_parse_repo, revision):
157+
repo = rev_parse_repo["repo"]
158+
expected = repo.git.rev_parse("--verify", revision)
159+
assert repo.rev_parse(revision).hexsha == expected
160+
161+
162+
@pytest.mark.parametrize("revision", [":/release", "HEAD^{/release}"])
163+
def test_commit_and_tree_message_search(rev_parse_repo, revision):
164+
repo = rev_parse_repo["repo"]
165+
release = rev_parse_repo["release"]
166+
assert repo.commit(revision) == release
167+
assert repo.tree(revision) == release.tree
168+
169+
170+
def test_rev_parse_commit_message_literal_bang_and_option(rev_parse_repo):
171+
repo = rev_parse_repo["repo"]
172+
commit = repo.index.commit("!urgent --all")
173+
assert repo.rev_parse(":/!!urgent") == commit
174+
assert repo.rev_parse("HEAD^{/!!urgent}") == commit
175+
assert repo.rev_parse(":/--all") == commit
176+
assert repo.rev_parse("HEAD^{/--all}") == commit
177+
178+
179+
def test_rev_parse_commit_message_escaped_braces(rev_parse_repo):
180+
repo = rev_parse_repo["repo"]
181+
commit = repo.index.commit("literal{3}")
182+
repo.index.commit("literalll")
183+
assert repo.rev_parse(r"HEAD^{/literal\{3\}}") == commit
184+
185+
186+
def test_revision_message_search_does_not_backtrack_in_python(tmp_path):
187+
with Repo.init(tmp_path) as repo:
188+
actor = Actor("GitPython Tests", "gitpython@example.com")
189+
repo.index.commit("hello " + "b" * 80, author=actor, committer=actor)
190+
191+
# Run the adversarial expression in a killable process: Python's regex engine
192+
# can otherwise hold the GIL indefinitely, including against a short message.
193+
subprocess.run(
194+
[
195+
sys.executable,
196+
"-c",
197+
"""
198+
import sys
199+
from git import Repo
200+
from gitdb.exc import BadName
201+
202+
with Repo(sys.argv[1]) as repo:
203+
for resolve in (repo.rev_parse, repo.commit, repo.tree):
204+
for revision in (':/(.+)+ZZZ', 'HEAD^{/(.+)+ZZZ}'):
205+
try:
206+
resolve(revision)
207+
except BadName:
208+
pass
209+
else:
210+
raise AssertionError('Unexpected match: ' + revision)
211+
""",
212+
str(tmp_path),
213+
],
214+
check=True,
215+
capture_output=True,
216+
timeout=5,
217+
)
218+
219+
220+
def test_revision_message_search_does_not_deserialize_history(tmp_path, monkeypatch):
221+
with Repo.init(tmp_path) as repo:
222+
actor = Actor("GitPython Tests", "gitpython@example.com")
223+
commit = repo.index.commit("needle", author=actor, committer=actor)
224+
225+
def reject_deserialization(*args, **kwargs):
226+
raise AssertionError("Message search must not deserialize commits in Python")
227+
228+
monkeypatch.setattr(Commit, "_deserialize", reject_deserialization)
229+
assert repo.rev_parse(":/needle") == commit
230+
assert repo.rev_parse("HEAD^{/needle}") == commit
231+
232+
233+
def test_commit_and_tree_resolve_before_peeling(rev_parse_repo):
234+
repo = rev_parse_repo["repo"]
235+
root = rev_parse_repo["root"]
236+
merge = rev_parse_repo["merge"]
237+
assert repo.commit("ann") == root
238+
assert repo.tree("ann") == root.tree
239+
assert repo.tree("HEAD:dir") == merge.tree["dir"]
240+
assert repo.tree("HEAD^{tree}") == merge.tree
241+
with pytest.raises(ValueError):
242+
repo.commit("HEAD^{tree}")
243+
with pytest.raises(ValueError):
244+
repo.tree("HEAD:README.md")
245+
246+
143247
def test_rev_parse_rejects_invalid_object_specs(rev_parse_repo):
144248
repo = rev_parse_repo["repo"]
145249

146250
with pytest.raises(ValueError):
147251
repo.rev_parse(":")
148252
with pytest.raises(ValueError):
149253
repo.rev_parse(":/")
150-
with pytest.raises(ValueError):
254+
with pytest.raises(BadName):
151255
repo.rev_parse(":/[")
152-
with pytest.raises(ValueError):
256+
with pytest.raises(BadName):
153257
repo.rev_parse("HEAD^{/[}")
258+
with pytest.raises(ValueError):
259+
repo.rev_parse(":/!reserved")
260+
with pytest.raises(ValueError):
261+
repo.rev_parse("HEAD^{/!reserved}")
154262
with pytest.raises(ValueError):
155263
repo.rev_parse("@{-0}")
156264
with pytest.raises(ValueError):

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL