From ef120d756c8f404a93e9266dbfa44ec8f39314ee Mon Sep 17 00:00:00 2001 From: Keerthana KT Date: Fri, 30 Oct 2026 00:44:01 +0530 Subject: [PATCH] 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. --- doc/source/changes.rst | 4 ++++ git/index/base.py | 18 ++++++++++++++---- git/objects/submodule/base.py | 5 ++--- test/test_index.py | 18 ++++++++++++++++++ test/test_submodule.py | 15 +++++++++++++++ tests/fixtures/index.sh | 6 ++++++ tests/fixtures/submodule-special.sh | 10 ++++++++++ 7 files changed, 69 insertions(+), 7 deletions(-) diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 0da1a81c4..5962ccf56 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -172,6 +172,10 @@ API changes happy paths, Python glue failures and representative compatibility regressions. Historical timing-only tests are removed; the maintained backend benchmark checks result parity and CLI budgets. +* ``IndexFile.remove()`` matches the paths of index entries, blobs, and submodules + literally, so an entry named ``*.txt`` no longer removes every ``.txt`` file. + Path strings keep their pathspec semantics. ``Submodule.move()`` and + ``Submodule.remove()`` remove the submodule's own index entry the same way. * Commit/tag metadata uses structured Gix decoders when the Gix backend supports the repository. The CLI backend retains raw decoding because Git has no faithful formatted query for all commit headers or arbitrary tag objects. ``gpgsig`` diff --git a/git/index/base.py b/git/index/base.py index 61316df45..ce7417546 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -1066,11 +1066,12 @@ def remove( - :class:`~git.objects.blob.Blob` object - Only the path portion is used in this case. + Only the path portion is used in this case. It is matched literally. - :class:`~git.index.typ.BaseIndexEntry` or compatible type - The only relevant information here is the path. The stage is ignored. + The only relevant information here is the path, which is matched + literally. The stage is ignored. :param working_tree: If ``True``, the entry will also be removed from the working tree, @@ -1107,8 +1108,17 @@ def remove( args.append("--cached") args.append("--") - # Preprocess paths. - paths = list(map(os.fspath, self._items_to_rela_paths(items))) # type: ignore[arg-type] + # Preprocess paths. Entries, blobs and submodules name exact paths, which Git + # must not expand as patterns. Path strings keep their pathspec semantics. + if isinstance(items, (str, os.PathLike)): + items = [items] + item_list = list(items) + paths = [ + ":(literal)" + os.fspath(rela_path) + if isinstance(item, (BaseIndexEntry, Blob, Submodule)) + else os.fspath(rela_path) + for item, rela_path in zip(item_list, self._items_to_rela_paths(item_list)) + ] with self._materialized_index() as path: removed_paths = self.repo.git._call_process_safe( "rm", args, paths, env={"GIT_INDEX_FILE": path}, **kwargs diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 5991d2a15..408ed71d0 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -1361,7 +1361,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = if configuration: try: entry = index.entry(self.path) - index.remove(self.path, write=False, force=True) + index.remove([entry], write=False, force=True) nentry = git.IndexEntry(entry[:3] + (module_checkout_path,) + entry[4:]) index.add([nentry], write=False) except KeyError as e: @@ -1572,8 +1572,7 @@ def remove( # First the index-entry. parent_index = self.repo.index try: - parent_index.entry(self.path) - parent_index.remove(self.path, write=False, force=True) + parent_index.remove([parent_index.entry(self.path)], write=False, force=True) except KeyError: pass # END delete entry diff --git a/test/test_index.py b/test/test_index.py index f1f875443..63d33fbd1 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -1391,6 +1391,24 @@ def test_checkout_sends_each_path_as_one_record(self): # Neither "nested/first" nor "outside" was requested. assert (root / "outside").read_bytes() == b"local" + @pytest.mark.skipif(os.name == "nt", reason="'*' and ':' are not valid Windows filenames") + def test_remove_matches_entry_paths_literally(self): + with fixture_repo("index.sh", "remove-literal", write=True) as repo: + root = Path(repo.working_tree_dir) + index = repo.index + entries = [index.entry("*.txt"), index.entry(":(glob)**")] + assert index.remove(entries, write=False) == ["*.txt", ":(glob)**"] + assert {entry.path for entry in index.iter_entries()} == {"a.txt", "b.txt", "src/app.py"} + + entry = entries[0] + blob = Blob(repo, entry.binsha, entry.mode, entry.path) + assert repo.index.remove([blob], working_tree=True) == ["*.txt"] + assert not (root / "*.txt").exists() + assert (root / "a.txt").exists() and (root / "b.txt").exists() + + # Path strings keep their pathspec semantics. + assert repo.index.remove(["*.txt"], write=False) == ["a.txt", "b.txt"] + class TestIndexUtils: @pytest.mark.parametrize("file_path_type", [str, Path]) diff --git a/test/test_submodule.py b/test/test_submodule.py index af7e1fdd3..0ed276e4a 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -326,6 +326,21 @@ def test_submodule_cli_lifecycle(monkeypatch, object_format, ref_format): assert not parent.submodules +@pytest.mark.skipif(os.name == "nt", reason="'*' is not a valid Windows filename") +@pytest.mark.parametrize("operation", ["remove", "move"]) +def test_submodule_index_update_matches_its_path_literally(operation): + with fixture_repo("submodule-special.sh", "glob-path", write=True, subdir="parent") as repo: + submodule = repo.submodules[0] + assert submodule.path == "*" + if operation == "remove": + submodule.remove() + expected = {".gitmodules", "README"} + else: + submodule.move("moved", module=False) + expected = {".gitmodules", "README", "moved"} + assert {entry.path for entry in repo.index.iter_entries()} == expected + + @pytest.mark.skipif(sys.platform != "win32", reason="Windows locks files against directory renames") @pytest.mark.parametrize("use_short_path", [False, True]) def test_submodule_reconnect_with_open_metadata_file(use_short_path): diff --git a/tests/fixtures/index.sh b/tests/fixtures/index.sh index d51acd1c0..829645e6e 100755 --- a/tests/fixtures/index.sh +++ b/tests/fixtures/index.sh @@ -119,6 +119,12 @@ case "$scenario" in rm repo/nested/$'first\noutside' 'repo/"quoted"' printf local >repo/outside ;; + remove-literal) + mkdir repo/src + for name in a.txt b.txt '*.txt' ':(glob)**' src/app.py; do printf %s "$name" >"repo/$name"; done + git -C repo add -A + git -C repo commit -qm 'Add files' + ;; *) printf 'Unknown index fixture: %s\n' "$scenario" >&2; exit 1 ;; esac for hook in "$hooks/pre-commit" "$hooks/commit-msg"; do diff --git a/tests/fixtures/submodule-special.sh b/tests/fixtures/submodule-special.sh index ef44767f8..4ef1196bc 100644 --- a/tests/fixtures/submodule-special.sh +++ b/tests/fixtures/submodule-special.sh @@ -34,4 +34,14 @@ case "$1" in git init -q -b master --separate-git-dir="$PWD/metadata" checkout touch metadata/objects/held-open ;; + glob-path) + git init -q -b master parent + git -C parent commit --allow-empty -qm 'Initial commit' + printf content > parent/README + git -C parent config -f .gitmodules submodule.module.path '*' + git -C parent config -f .gitmodules submodule.module.url https://example.invalid/module.git + git -C parent add README .gitmodules + git -C parent update-index --add --cacheinfo "160000,$(git -C parent rev-parse HEAD),*" + git -C parent commit -qm 'Add submodule' + ;; esac