Skip to content

Commit ef120d7

Browse files
committed
fix(index): remove entries, blobs and submodules by literal path
`IndexFile.remove()` passed every item to `git rm` as a pathspec. Path strings are documented to accept globs, but index entries, blobs and submodules name one exact path, and Git still expanded glob characters and pathspec magic in those names. The names are chosen by whoever authored the repository: - Removing the entry for a file named `*.txt` also removed `a.txt` and `b.txt`. - Removing the entry for `:(glob)**` removed every tracked path, and with `working_tree=True` deleted those files from disk. `Submodule.remove()` (for a submodule that is not checked out, or with `module=False`) and `Submodule.move(module=False)` passed the `.gitmodules` path string to the same call with `force=True`. A superproject declaring a submodule at path `*` therefore had every tracked file staged for deletion and the index written. The neighboring `git rm` and `git mv` calls in `Submodule` already run with `GIT_LITERAL_PATHSPECS=1`. Prefix the paths of entry, blob and submodule items with Git's `:(literal)` pathspec magic and leave path strings unchanged, so a mixed call keeps the documented glob behavior for its strings. The submodule callers now pass their index entry. `git mv` reads its sources literally, so `IndexFile.move()` needs no change. `git rm` has no native Gix handler and runs through the CLI on both backends, so `doc/gix-backend.md` is unchanged. Limitation: when a caller exports `GIT_LITERAL_PATHSPECS=1`, Git does not parse pathspec magic. Removing an entry, blob or submodule then fails with "did not match any files" and removes nothing; path strings stay literal in that environment as before. Add a `remove-literal` scenario to `tests/fixtures/index.sh`, a `glob-path` scenario to `tests/fixtures/submodule-special.sh`, and regression tests in `test/test_index.py` and `test/test_submodule.py`. The three new test cases fail before the change and pass after it with both `GIT_PYTHON_BACKEND=cli` and `GIT_PYTHON_BACKEND=gix` (Git 2.52.0, GixPython 0.3.0, Python 3.11.17, macOS). Full suites with the change: CLI 1,884 passed and Gix 1,987 passed. Each run also had 108 failures, and the same 108 tests fail on the unpatched tree. They need history and tags such as `0.1.6` and `0.3.2.1`, which the shallow local clone lacks. `ruff check`, `ruff format --check`, `mypy --python-version=3.11`, `basedpyright --warnings`, ShellCheck and `git diff --check` pass.
1 parent 50d88aa commit ef120d7

7 files changed

Lines changed: 69 additions & 7 deletions

File tree

‎doc/source/changes.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ API changes
172172
happy paths, Python glue failures and representative compatibility regressions.
173173
Historical timing-only tests are removed; the maintained backend benchmark
174174
checks result parity and CLI budgets.
175+
* ``IndexFile.remove()`` matches the paths of index entries, blobs, and submodules
176+
literally, so an entry named ``*.txt`` no longer removes every ``.txt`` file.
177+
Path strings keep their pathspec semantics. ``Submodule.move()`` and
178+
``Submodule.remove()`` remove the submodule's own index entry the same way.
175179
* Commit/tag metadata uses structured Gix decoders when the Gix backend supports
176180
the repository. The CLI backend retains raw decoding because Git has no faithful
177181
formatted query for all commit headers or arbitrary tag objects. ``gpgsig``

‎git/index/base.py‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,11 +1066,12 @@ def remove(
10661066
10671067
- :class:`~git.objects.blob.Blob` object
10681068
1069-
Only the path portion is used in this case.
1069+
Only the path portion is used in this case. It is matched literally.
10701070
10711071
- :class:`~git.index.typ.BaseIndexEntry` or compatible type
10721072
1073-
The only relevant information here is the path. The stage is ignored.
1073+
The only relevant information here is the path, which is matched
1074+
literally. The stage is ignored.
10741075
10751076
:param working_tree:
10761077
If ``True``, the entry will also be removed from the working tree,
@@ -1107,8 +1108,17 @@ def remove(
11071108
args.append("--cached")
11081109
args.append("--")
11091110

1110-
# Preprocess paths.
1111-
paths = list(map(os.fspath, self._items_to_rela_paths(items))) # type: ignore[arg-type]
1111+
# Preprocess paths. Entries, blobs and submodules name exact paths, which Git
1112+
# must not expand as patterns. Path strings keep their pathspec semantics.
1113+
if isinstance(items, (str, os.PathLike)):
1114+
items = [items]
1115+
item_list = list(items)
1116+
paths = [
1117+
":(literal)" + os.fspath(rela_path)
1118+
if isinstance(item, (BaseIndexEntry, Blob, Submodule))
1119+
else os.fspath(rela_path)
1120+
for item, rela_path in zip(item_list, self._items_to_rela_paths(item_list))
1121+
]
11121122
with self._materialized_index() as path:
11131123
removed_paths = self.repo.git._call_process_safe(
11141124
"rm", args, paths, env={"GIT_INDEX_FILE": path}, **kwargs

‎git/objects/submodule/base.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1361,7 +1361,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool =
13611361
if configuration:
13621362
try:
13631363
entry = index.entry(self.path)
1364-
index.remove(self.path, write=False, force=True)
1364+
index.remove([entry], write=False, force=True)
13651365
nentry = git.IndexEntry(entry[:3] + (module_checkout_path,) + entry[4:])
13661366
index.add([nentry], write=False)
13671367
except KeyError as e:
@@ -1572,8 +1572,7 @@ def remove(
15721572
# First the index-entry.
15731573
parent_index = self.repo.index
15741574
try:
1575-
parent_index.entry(self.path)
1576-
parent_index.remove(self.path, write=False, force=True)
1575+
parent_index.remove([parent_index.entry(self.path)], write=False, force=True)
15771576
except KeyError:
15781577
pass
15791578
# END delete entry

‎test/test_index.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,6 +1391,24 @@ def test_checkout_sends_each_path_as_one_record(self):
13911391
# Neither "nested/first" nor "outside" was requested.
13921392
assert (root / "outside").read_bytes() == b"local"
13931393

1394+
@pytest.mark.skipif(os.name == "nt", reason="'*' and ':' are not valid Windows filenames")
1395+
def test_remove_matches_entry_paths_literally(self):
1396+
with fixture_repo("index.sh", "remove-literal", write=True) as repo:
1397+
root = Path(repo.working_tree_dir)
1398+
index = repo.index
1399+
entries = [index.entry("*.txt"), index.entry(":(glob)**")]
1400+
assert index.remove(entries, write=False) == ["*.txt", ":(glob)**"]
1401+
assert {entry.path for entry in index.iter_entries()} == {"a.txt", "b.txt", "src/app.py"}
1402+
1403+
entry = entries[0]
1404+
blob = Blob(repo, entry.binsha, entry.mode, entry.path)
1405+
assert repo.index.remove([blob], working_tree=True) == ["*.txt"]
1406+
assert not (root / "*.txt").exists()
1407+
assert (root / "a.txt").exists() and (root / "b.txt").exists()
1408+
1409+
# Path strings keep their pathspec semantics.
1410+
assert repo.index.remove(["*.txt"], write=False) == ["a.txt", "b.txt"]
1411+
13941412

13951413
class TestIndexUtils:
13961414
@pytest.mark.parametrize("file_path_type", [str, Path])

‎test/test_submodule.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,21 @@ def test_submodule_cli_lifecycle(monkeypatch, object_format, ref_format):
326326
assert not parent.submodules
327327

328328

329+
@pytest.mark.skipif(os.name == "nt", reason="'*' is not a valid Windows filename")
330+
@pytest.mark.parametrize("operation", ["remove", "move"])
331+
def test_submodule_index_update_matches_its_path_literally(operation):
332+
with fixture_repo("submodule-special.sh", "glob-path", write=True, subdir="parent") as repo:
333+
submodule = repo.submodules[0]
334+
assert submodule.path == "*"
335+
if operation == "remove":
336+
submodule.remove()
337+
expected = {".gitmodules", "README"}
338+
else:
339+
submodule.move("moved", module=False)
340+
expected = {".gitmodules", "README", "moved"}
341+
assert {entry.path for entry in repo.index.iter_entries()} == expected
342+
343+
329344
@pytest.mark.skipif(sys.platform != "win32", reason="Windows locks files against directory renames")
330345
@pytest.mark.parametrize("use_short_path", [False, True])
331346
def test_submodule_reconnect_with_open_metadata_file(use_short_path):

‎tests/fixtures/index.sh‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,12 @@ case "$scenario" in
119119
rm repo/nested/$'first\noutside' 'repo/"quoted"'
120120
printf local >repo/outside
121121
;;
122+
remove-literal)
123+
mkdir repo/src
124+
for name in a.txt b.txt '*.txt' ':(glob)**' src/app.py; do printf %s "$name" >"repo/$name"; done
125+
git -C repo add -A
126+
git -C repo commit -qm 'Add files'
127+
;;
122128
*) printf 'Unknown index fixture: %s\n' "$scenario" >&2; exit 1 ;;
123129
esac
124130
for hook in "$hooks/pre-commit" "$hooks/commit-msg"; do

‎tests/fixtures/submodule-special.sh‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,14 @@ case "$1" in
3434
git init -q -b master --separate-git-dir="$PWD/metadata" checkout
3535
touch metadata/objects/held-open
3636
;;
37+
glob-path)
38+
git init -q -b master parent
39+
git -C parent commit --allow-empty -qm 'Initial commit'
40+
printf content > parent/README
41+
git -C parent config -f .gitmodules submodule.module.path '*'
42+
git -C parent config -f .gitmodules submodule.module.url https://example.invalid/module.git
43+
git -C parent add README .gitmodules
44+
git -C parent update-index --add --cacheinfo "160000,$(git -C parent rev-parse HEAD),*"
45+
git -C parent commit -qm 'Add submodule'
46+
;;
3747
esac

0 commit comments

Comments
 (0)