Skip to content

Commit b99404a

Browse files
committed
fix: preserve repository and object paths across platforms
The Windows Python 3.8 CI job reported 42 failures and 179 setup errors. Most submodule failures shared a discovery bug: Git resolves relative gitfile targets using forward slashes even when the caller supplies a native Windows path. Normalize Git-facing discovery operands with the existing platform helper, and account for native separators in Git's worktree registry output. Use matching `surrogateescape` codecs for Git protocol paths so undecodable tree names round-trip without Windows filesystem encoding changing their bytes. Verify path/stage, mode, and object ID after materializing a private index: `git update-index --index-info` can exit successfully while dropping Windows-incompatible names. Raise `ValueError` before publishing such an index and document the platform restriction in `changes.rst`. Keep unusual names in object-only tests when the host cannot represent them in a checkout. Use native-valid paths for worktree tests, assert rejection of unsupported index names and quoted file references, and keep full quoted reference coverage with reftable. Fix separator and LF assumptions in config, URL, and packed-reference fixtures. Close test-owned repositories before submodule removal and make the fake Windows Git executable discoverable without shell execution. Also restore root paths for `Repo.tree()` results resolved directly from tree IDs while preserving explicit subtree paths. Validation: 116 repository tests and 10 focused submodule tests passed; index/helper tests passed 93 with 2 platform skips; the focused format/safety run passed 72; config/reference/remote tests passed 57 with 24 subtests; 36 refresh tests and 2 revision regressions passed. The Git 2.52 targeted index matrix passed 19 tests. Ruff, mypy, basedpyright, Sphinx with warnings as errors, and `git diff --check` pass. Native Windows validation awaits CI.
1 parent b7d76fc commit b99404a

15 files changed

Lines changed: 163 additions & 43 deletions

‎doc/source/changes.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ API changes
6363
``IndexFile.from_tree()``, ``IndexFile.new()``, ``write()`` and ``write_tree()``.
6464
Git preserves untouched metadata when an existing index is edited. Temporary
6565
indexes isolate tree/merge operations from the real index and working tree.
66+
Git's platform-specific index filename restrictions apply. Unsupported entries
67+
raise ``ValueError`` before the original index changes, including names with
68+
colons or control characters on Windows. Tree objects can still contain names
69+
that the working tree or index cannot represent.
6670
``version`` is read-only. ``from_tree()`` accepts ``trivial``, ``aggressive``,
6771
and ``verbose`` options; arbitrary ``read-tree`` keyword forwarding is removed.
6872
* Standalone binary tree parsers, serializers, and multi-tree traversal helpers

‎git/index/base.py‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
from gitdb.base import IStream
2323

24-
from git.compat import defenc, force_bytes
24+
from git.compat import defenc, force_bytes, safe_decode
2525
from git.cmd import Git
2626
import git.diff as git_diff
2727
from git.exc import CheckoutError, GitCommandError, GitError, InvalidGitRepositoryError, UnmergedEntriesError
@@ -144,7 +144,7 @@ def _read_entries(self, path: PathLike) -> Dict[Tuple[PathLike, StageType], Inde
144144
continue
145145
metadata, path_bytes = record.split(b"\t", 1)
146146
tag, mode, oid, stage = metadata.split()
147-
entry_path = os.fsdecode(path_bytes)
147+
entry_path = safe_decode(path_bytes)
148148
_validate_repo_path(entry_path)
149149
binsha = bytes.fromhex(oid.decode("ascii"))
150150
if len(binsha) != self.repo._oid_size or int(stage) not in range(4):
@@ -198,22 +198,28 @@ def _materialized_index(self) -> Generator[str, None, None]:
198198
records = []
199199
for name in sorted(changed):
200200
_validate_repo_path(name)
201-
records.append(b"0 " + self.repo._null_hexsha.encode("ascii") + b"\t" + os.fsencode(name) + b"\0")
201+
name_bytes = name.encode(defenc, "surrogateescape")
202+
records.append(b"0 " + self.repo._null_hexsha.encode("ascii") + b"\t" + name_bytes + b"\0")
202203
for stage in range(4):
203204
desired_entry = desired.get((name, stage))
204205
if desired_entry is not None:
205206
records.append(
206207
("%o %s %d\t" % (desired_entry.mode, desired_entry.hexsha, stage)).encode("ascii")
207-
+ os.fsencode(name)
208+
+ name_bytes
208209
+ b"\0"
209210
)
210211
with tempfile.TemporaryFile() as stream:
211212
stream.write(b"".join(records))
212213
stream.seek(0)
213214
self.repo.git._call_process_safe("update_index", "-z", "--index-info", istream=stream, env=env)
215+
# Git can report success while ignoring names unsupported on this
216+
# platform. Do not publish an index with silently missing entries.
217+
actual = self._read_entries(path)
218+
if actual.keys() != desired.keys() or any(actual[key][:2] != desired[key][:2] for key in actual):
219+
raise ValueError("Git did not retain the requested index entries; filenames may be unsupported")
214220
for option, mask in (("--assume-unchanged", CE_VALID), ("--skip-worktree", CE_EXT_SKIP_WORKTREE << 16)):
215221
names = [
216-
os.fsencode(name) + b"\0"
222+
name.encode(defenc, "surrogateescape") + b"\0"
217223
for name in sorted(changed)
218224
if (name, 0) in desired and desired[(name, 0)].flags & mask
219225
]
@@ -532,7 +538,7 @@ def _write_path_to_stdin(
532538

533539
if proc.stdin is not None:
534540
try:
535-
proc.stdin.write(os.fsencode(filepath) + b"\0")
541+
proc.stdin.write(os.fspath(filepath).encode(defenc, "surrogateescape") + b"\0")
536542
except OSError as e:
537543
# Pipe broke, usually because some error happened.
538544
raise fmakeexc() from e

‎git/objects/tree.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
import git.diff as git_diff
1212
from git.util import IterableList, join_path, to_bin_sha
13-
from git.compat import safe_decode
13+
from git.compat import defenc, safe_decode
1414
from git.cmd import Git
1515

1616
from . import util
@@ -416,7 +416,7 @@ def _serialize(self, stream: "BytesIO") -> "Tree":
416416
if len(oid) != self.repo._oid_size or mode >> 12 not in types:
417417
raise ValueError("Invalid tree entry object ID or mode")
418418
source.write(("%o %s %s\t" % (mode, types[mode >> 12], oid.hex())).encode("ascii"))
419-
source.write(os.fsencode(name) + b"\0")
419+
source.write(name.encode(defenc, "surrogateescape") + b"\0")
420420
source.seek(0)
421421
oid = self.repo.git._call_process_safe("mktree", "-z", "--missing", istream=source)
422422
Git._check_operand(oid, "tree object ID")

‎git/repo/base.py‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
finalize_process,
4444
hex_to_bin,
4545
remove_password_if_present,
46+
to_native_path_linux,
4647
)
4748

4849
from .fun import (
@@ -326,7 +327,9 @@ def __init__(
326327
dotgit = osp.join(curpath, ".git")
327328
candidate = curpath if explicit_git_dir or not osp.lexists(dotgit) else dotgit
328329
try:
329-
git_dir = probe._call_process_safe("rev_parse", "--resolve-git-dir", candidate)
330+
# Git resolves relative gitfile targets using the last forward
331+
# slash in this operand, including on Windows.
332+
git_dir = probe._call_process_safe("rev_parse", "--resolve-git-dir", to_native_path_linux(candidate))
330333
git_dir = osp.abspath(git_dir)
331334
if osp.isfile(candidate):
332335
# Git canonicalizes gitfile targets. Retain an equivalent
@@ -381,8 +384,14 @@ def __init__(
381384
if not fields or not fields[0].startswith("worktree ") or "bare" in fields:
382385
continue
383386
worktree = fields[0][9:]
387+
# Git only strips a forward-slash /.git suffix from its registry.
388+
# A Windows gitdir file can instead contain a native backslash.
389+
if osp.basename(worktree) == ".git" and osp.isfile(worktree):
390+
worktree = osp.dirname(worktree)
384391
try:
385-
resolved = probe._call_process_safe("rev_parse", "--resolve-git-dir", osp.join(worktree, ".git"))
392+
resolved = probe._call_process_safe(
393+
"rev_parse", "--resolve-git-dir", to_native_path_linux(osp.join(worktree, ".git"))
394+
)
386395
except GitCommandError:
387396
continue
388397
if osp.realpath(resolved) == osp.realpath(git_dir):
@@ -867,7 +876,10 @@ def tree(self, rev: Union[Tree_ish, str, None] = None) -> "Tree":
867876
if rev is None:
868877
return self.head.commit.tree
869878
obj = self.rev_parse(str(rev))
870-
return obj if obj.type == "tree" else to_commit(obj).tree
879+
if obj.type == "tree":
880+
obj.path = getattr(obj, "path", "")
881+
return obj
882+
return to_commit(obj).tree
871883

872884
def iter_commits(
873885
self,
@@ -1061,7 +1073,7 @@ def alternates(self) -> List[str]:
10611073
path = line[len(b"alternate: ") :]
10621074
if path.startswith(b'"') and path.endswith(b'"'):
10631075
path = _unquote_path(path[1:-1])
1064-
paths.append(os.fsdecode(path))
1076+
paths.append(safe_decode(path))
10651077
return paths
10661078

10671079
def is_dirty(
@@ -1132,7 +1144,7 @@ def _get_untracked_files(self, *args: Any, **kwargs: Any) -> List[str]:
11321144
paths = []
11331145
for record in records:
11341146
if record.startswith(b"?? "):
1135-
paths.append(os.fsdecode(record[3:]))
1147+
paths.append(safe_decode(record[3:]))
11361148
elif record[:1] in (b"R", b"C") or record[1:2] in (b"R", b"C"):
11371149
next(records, None) # Renames/copies carry a second path record.
11381150
return paths
@@ -1146,7 +1158,7 @@ def ignored(self, *paths: PathLike) -> List[str]:
11461158
if "\0" in os.fspath(path):
11471159
raise ValueError("Paths cannot contain NUL")
11481160
with tempfile.TemporaryFile() as stream:
1149-
stream.write(b"\0".join(os.fsencode(path) for path in paths) + b"\0")
1161+
stream.write(b"\0".join(os.fspath(path).encode(defenc, "surrogateescape") for path in paths) + b"\0")
11501162
stream.seek(0)
11511163
status, output, stderr = self.git._call_process_safe(
11521164
"check_ignore",
@@ -1161,7 +1173,7 @@ def ignored(self, *paths: PathLike) -> List[str]:
11611173
return []
11621174
if status:
11631175
raise GitCommandError("git check-ignore", status, stderr, output)
1164-
return [os.fsdecode(path) for path in output.split(b"\0") if path]
1176+
return [safe_decode(path) for path in output.split(b"\0") if path]
11651177

11661178
@property
11671179
def active_branch(self) -> Head:

‎git/repo/fun.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,13 @@
2626
from gitdb.exc import BadName, BadObject
2727

2828
from git.cmd import Git
29+
from git.compat import defenc
2930
from git.exc import GitCommandError
3031
from git.objects import Object
3132
from git.objects.base import IndexObject
3233
from git.refs import SymbolicReference
3334
from git.types import AnyGitObject, Literal, PathLike
34-
from git.util import bin_to_hex, hex_to_bin
35+
from git.util import bin_to_hex, hex_to_bin, to_native_path_linux
3536

3637
if TYPE_CHECKING:
3738
from gitdb.db import CompoundDB, LooseObjectDB
@@ -51,7 +52,7 @@ def find_submodule_git_dir(d: PathLike) -> Optional[PathLike]:
5152
if not osp.exists(path):
5253
return None
5354
try:
54-
return Git()._call_process_safe("rev_parse", "--resolve-git-dir", path)
55+
return Git()._call_process_safe("rev_parse", "--resolve-git-dir", to_native_path_linux(path))
5556
except GitCommandError:
5657
return None
5758

@@ -137,7 +138,7 @@ def rev_parse(repo: "Repo", rev: str) -> AnyGitObject:
137138
# Git resolves the mode, including index stages and executable/symlink
138139
# entries. No object storage or revision grammar is decoded in Python.
139140
with tempfile.TemporaryFile() as stream:
140-
stream.write(os.fsencode(rev) + b"\0")
141+
stream.write(rev.encode(defenc, "surrogateescape") + b"\0")
141142
stream.seek(0)
142143
mode = repo.git._call_process_safe("cat_file", "--batch-check=%(objectmode)", "-Z", istream=stream).strip(
143144
"\0"

‎test/test_cli_objects_index.py‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
"""Repository-format independent object/index operations and their safety boundary."""
22

3+
import os
4+
from io import BytesIO
35
from pathlib import Path
46

57
import pytest
8+
from gitdb.base import IStream
69

7-
from git import Actor, Commit, IndexFile, Repo
10+
from git import Actor, Commit, IndexFile, Repo, Tree
811
from git.exc import HookExecutionError, UnmergedEntriesError, UnsafeOptionError
912
from git.index.typ import BaseIndexEntry, IndexEntry
1013

@@ -17,7 +20,9 @@ def repo(request, tmp_path):
1720

1821
def test_objects_index_commit_and_merge(repo):
1922
root = Path(repo.working_tree_dir)
20-
names = ["space name", "line\nname", "tab\tname", "--option", "unicode-é", "dir/file"]
23+
names = ["space name", "--option", "unicode-é", "dir/file"]
24+
if os.name != "nt":
25+
names.extend(["line\nname", "tab\tname"])
2126
for name in names:
2227
path = root / name
2328
path.parent.mkdir(exist_ok=True)
@@ -39,9 +44,10 @@ def test_objects_index_commit_and_merge(repo):
3944
updated = commit.replace(message="updated")
4045
assert repo.commit(updated.hexsha).message == "updated"
4146
assert repo.head.commit == commit
42-
(root / "line\nname").write_text("modified")
43-
index.checkout(["line\nname"], force=True)
44-
assert (root / "line\nname").read_text() == "line\nname"
47+
checkout_name = "--option" if os.name == "nt" else "line\nname"
48+
(root / checkout_name).write_text("modified")
49+
index.checkout([checkout_name], force=True)
50+
assert (root / checkout_name).read_text() == checkout_name
4551
before = Path(index.path).read_bytes()
4652
virtual = IndexFile.from_tree(repo, tree)
4753
assert virtual.write_tree() == tree
@@ -53,6 +59,18 @@ def test_objects_index_commit_and_merge(repo):
5359
assert index.write_tree() == tree
5460

5561

62+
def test_tree_names_do_not_require_worktree_support(repo):
63+
tree = Tree(repo, repo._null_binsha, path="")
64+
tree._cache = [
65+
(b"a" * repo._oid_size, 0o100644, name) for name in ("line\nname", "tab\tname", "name:with:colons", "\udc9f")
66+
]
67+
data = BytesIO()
68+
tree._serialize(data)
69+
data.seek(0)
70+
stored = repo.odb.store(IStream("tree", len(data.getvalue()), data))
71+
assert Tree(repo, stored.binsha, path="")._cache == sorted(tree._cache, key=lambda entry: entry[2])
72+
73+
5674
def test_index_stages_missing_objects_and_atomic_failure(repo):
5775
index = repo.index
5876
missing = b"a" * repo._oid_size

‎test/test_config.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import configparser
44
from collections import UserList
55
from io import BytesIO
6+
from pathlib import Path
67
from unittest import mock
78

89
import pytest
@@ -46,7 +47,7 @@ def test_config_includes_and_repository_conditions(tmp_path):
4647
config.set_value("values", "source", "before")
4748
config.set_value("include", "path", str(included))
4849
config.add_value("values", "source", "after")
49-
config.set_value('includeIf "gitdir:' + repo.git_dir + '"', "path", str(included))
50+
config.set_value('includeIf "gitdir:' + Path(repo.git_dir).as_posix() + '"', "path", str(included))
5051
assert GitConfigParser(config_path, repo=repo).get_values("values", "source") == [
5152
"before",
5253
"after",

‎test/test_git.py‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -894,14 +894,9 @@ def test_successful_default_refresh_invalidates_cached_version_info(self):
894894
stack.enter_context(_patch_out_env("GIT_PYTHON_GIT_EXECUTABLE"))
895895

896896
if sys.platform == "win32":
897-
# On Windows, use a shell so "git" finds "git.cmd". The correct and safe
898-
# ways to do this straightforwardly are to set GIT_PYTHON_GIT_EXECUTABLE
899-
# to git.cmd in the environment, or call git.refresh with the command's
900-
# full path. See the Git.USE_SHELL docstring for deprecation details.
901-
# But this tests a "default" scenario where neither is done. The
902-
# approach used here, setting USE_SHELL to True so PATHEXT is honored,
903-
# should not be used in production code (nor even in most test cases).
904-
stack.enter_context(mock.patch.object(Git, "USE_SHELL", True))
897+
# The fake executable is a batch file. Name its extension explicitly
898+
# so PATH lookup works without a shell, including version probes.
899+
stack.enter_context(mock.patch.object(Git, "git_exec_name", "git.cmd"))
905900

906901
new_git = Git()
907902
_rename_with_stem(path2, "git") # "Install" git, "late" in the PATH.

‎test/test_index.py‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,45 @@ def test_index_reader_and_writer_reject_unsafe_paths(self, path):
211211
index.write()
212212

213213
def test_valid_unusual_index_names_round_trip(self):
214-
names = ["a b", "a\nb", "a\tb", "name:value", "dir/.gitignore", "café"]
214+
names = ["a b", "--option", "dir/.gitignore", "café"]
215+
windows_unsupported = ["a\nb", "a\tb", "name:value"]
215216
if os.name != "nt":
216-
names.append("a\\b")
217+
names.extend([*windows_unsupported, "a\\b", "\udc9f"])
217218
with tempfile.TemporaryDirectory() as directory:
218219
index = IndexFile(self.rorepo, Path(directory, "index"))
219220
index.entries = {(name, 0): IndexEntry((0o100644, b"a" * 20, 0, name)) for name in names}
220221
index.write()
221222
assert sorted(entry.path for entry in index.update().entries.values()) == sorted(names)
223+
if os.name == "nt":
224+
before = Path(index.path).read_bytes()
225+
for name in windows_unsupported:
226+
index.entries[(name, 0)] = IndexEntry((0o100644, b"a" * 20, 0, name))
227+
with pytest.raises(ValueError, match="Git did not retain"):
228+
index.write()
229+
assert Path(index.path).read_bytes() == before
230+
assert not Path(str(index.path) + ".lock").exists()
231+
del index.entries[(name, 0)]
232+
233+
@ddt.data("write", "write_tree")
234+
def test_index_rejects_silently_ignored_entries_atomically(self, operation):
235+
call = Git._call_process_safe
236+
237+
def ignore_index_updates(git, command, *args, **kwargs):
238+
if command == "update_index":
239+
return ""
240+
return call(git, command, *args, **kwargs)
241+
242+
with tempfile.TemporaryDirectory() as directory:
243+
index = IndexFile(self.rorepo, Path(directory, "index"))
244+
index.entries = {("before", 0): IndexEntry((0o100644, b"a" * 20, 0, "before"))}
245+
index.write()
246+
before = Path(index.path).read_bytes()
247+
index.entries[("after", 0)] = IndexEntry((0o100644, b"a" * 20, 0, "after"))
248+
with mock.patch.object(Git, "_call_process_safe", ignore_index_updates):
249+
with pytest.raises(ValueError, match="Git did not retain"):
250+
getattr(index, operation)()
251+
assert Path(index.path).read_bytes() == before
252+
assert not Path(str(index.path) + ".lock").exists()
222253

223254
def test_long_index_names_are_fully_validated(self):
224255
prefix = "a/" + "nested/" * 650

‎test/test_reflog.py‎

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

4+
import sys
5+
46
import pytest
57

68
from git import Actor, Repo
@@ -138,6 +140,12 @@ def test_checkout_and_reset_reject_interactive_patch_helpers(repo):
138140

139141

140142
def test_quoted_branch_configuration_survives_rename(repo):
143+
if sys.platform == "win32" and repo.ref_format == "files":
144+
# Loose references need filenames that Windows cannot represent.
145+
with pytest.raises(OSError, match="Could not create reference") as error:
146+
repo.create_head('quoted"branch')
147+
assert isinstance(error.value.__cause__, GitCommandError)
148+
return
141149
branch = repo.create_head('quoted"branch')
142150
with branch.config_writer() as config:
143151
config.set_value("description", "retained")

0 commit comments

Comments
 (0)