From c80fa831717a69aba6878446a27de0aa1dc15456 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Mon, 14 Sep 2026 09:01:37 -0500 Subject: [PATCH] Don't let sparse clones find files in subdirectories (cherry picked from commit 52ff780a5084878dc7488f972a37559e723876da) --- AddonCatalogCacheCreator.py | 26 ++++++---- .../app/test_addon_catalog_cache_creator.py | 48 ++++++++++++++++++- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/AddonCatalogCacheCreator.py b/AddonCatalogCacheCreator.py index 36e0bed2..45103553 100644 --- a/AddonCatalogCacheCreator.py +++ b/AddonCatalogCacheCreator.py @@ -35,6 +35,7 @@ import io import json import os +import posixpath import re import requests @@ -561,7 +562,7 @@ def sparse_clone(self, name: str, url: str, branch: str, files: List[str]) -> No ["git", "config", "core.sparsecheckout", "true"], check=True ) with open(".git/info/sparse-checkout", "w") as f: - f.write("\n".join(files)) + f.write("\n".join(self.sparse_checkout_patterns(files))) f.write("\n") # So we are safe appending later subprocess.run( # nosec B603 B607 ["git", "fetch", "--depth=1", "origin", branch], @@ -601,7 +602,7 @@ def add_to_sparse_clone(self, name: str, files: List[str]) -> None: clone_path = os.path.join(cwd, name) os.chdir(clone_path) with open(".git/info/sparse-checkout", "a") as f: - f.write("\n".join(files)) + f.write("\n".join(self.sparse_checkout_patterns(files))) f.write("\n") # So we are safe appending later try: subprocess.run(["git", "read-tree", "-m", "-u", "HEAD"], check=True) # nosec B603 B607 @@ -610,6 +611,13 @@ def add_to_sparse_clone(self, name: str, files: List[str]) -> None: print(f"ERROR: {e}") os.chdir(cwd) + @staticmethod + def sparse_checkout_patterns(files: List[str]) -> List[str]: + """Convert paths relative to the top of a repository into sparse checkout patterns that + match only that exact path. Without a leading slash git matches a bare filename in every + subdirectory, pulling in unrelated files such as docs/requirements.txt. See #489.""" + return ["/" + posixpath.normpath(file.replace("\\", "/")).lstrip("/") for file in files] + def find_file( self, filename: str, @@ -617,13 +625,13 @@ def find_file( index: int, catalog_entry: AddonCatalog.AddonCatalogEntry, ) -> Optional[str]: - """Find a given file in the downloaded cache for this addon. Returns None if the file does - not exist.""" - start_dir = os.path.join(self.cwd, self.get_directory_name(addon_id, index, catalog_entry)) - for dirpath, _, filenames in os.walk(start_dir): - if filename in filenames: - return os.path.join(dirpath, filename) - return None + """Find a given file at the top level of the downloaded cache for this addon. Files of the + same name in subdirectories are not addon metadata, and are ignored. Returns None if the + file does not exist.""" + path = os.path.join( + self.cwd, self.get_directory_name(addon_id, index, catalog_entry), filename + ) + return path if os.path.isfile(path) else None @staticmethod def get_icon_from_metadata(metadata: addonmanager_metadata.Metadata) -> Optional[str]: diff --git a/AddonManagerTest/app/test_addon_catalog_cache_creator.py b/AddonManagerTest/app/test_addon_catalog_cache_creator.py index 2a1421a0..17ef6ffa 100644 --- a/AddonManagerTest/app/test_addon_catalog_cache_creator.py +++ b/AddonManagerTest/app/test_addon_catalog_cache_creator.py @@ -143,7 +143,7 @@ def test_get_directory_name_with_no_information(self): self.assertTrue(result.startswith(os.path.join("test_addon", "99"))) def test_find_file_with_existing_file(self): - """Find file locates the first occurrence of a given file""" + """Find file locates a given file at the top level of the addon""" ace = AddonCatalog.AddonCatalogEntry({"git_ref": "main"}) file_path = os.path.abspath( os.path.join("home", "cache", "TestMod", "1-main", "some_fake_file.txt") @@ -163,6 +163,18 @@ def test_find_file_with_non_existent_file(self): result = writer.find_file("some_other_fake_file.txt", "TestMod", 1, ace) self.assertIsNone(result) + def test_find_file_ignores_files_in_subdirectories(self): + """A file of the same name in a subdirectory, such as docs/requirements.txt, is not found""" + ace = AddonCatalog.AddonCatalogEntry({"git_ref": "main"}) + self.fake_fs().create_file( + os.path.join("home", "cache", "TestMod", "1-main", "docs", "requirements.txt"), + contents="sphinx", + ) + writer = accc.CacheWriter() + writer.cwd = os.path.abspath(os.path.join("home", "cache")) + result = writer.find_file("requirements.txt", "TestMod", 1, ace) + self.assertIsNone(result) + def test_generate_cache_entry_from_package_xml_bad_metadata(self): """Given an invalid metadata file, no cache entry is generated, but also no exception is raised.""" @@ -536,4 +548,36 @@ def test_add_to_sparse_clone_checks_out_without_network_access(self, mock_run): commands = self.issued_commands(mock_run) self.assertEqual([["git", "read-tree", "-m", "-u", "HEAD"]], commands) with open(sparse_file, encoding="utf-8") as f: - self.assertEqual("package.xml\nicon.svg\n", f.read()) + self.assertEqual("package.xml\n/icon.svg\n", f.read()) + + @patch("AddonCatalogCacheCreator.subprocess.run") + def test_new_sparse_clone_checks_out_only_top_level_files(self, mock_run): + """The sparse checkout patterns of a new clone are anchored to the top of the repository, + so a docs/requirements.txt file is not mistaken for the addon's requirements.txt.""" + + def fake_run(command, **_): + if command[:2] == ["git", "init"]: + os.makedirs(os.path.join(".git", "info")) + return MagicMock(returncode=0) + + mock_run.side_effect = fake_run + writer = accc.CacheWriter() + writer.sparse_clone("TestMod", "https://some.url", "main", ["package.xml", "metadata.txt"]) + sparse_file = os.path.join(os.getcwd(), "TestMod", ".git", "info", "sparse-checkout") + with open(sparse_file, encoding="utf-8") as f: + self.assertEqual("/package.xml\n/metadata.txt\n", f.read()) + self.assertEqual({}, writer.clone_errors) + + def test_sparse_checkout_patterns_are_anchored_and_normalized(self): + """Relative paths in any common spelling become a single anchored sparse checkout pattern.""" + self.assertEqual( + ["/package.xml", "/Resources/icon.svg", "/Resources/icon.svg", "/Resources/icon.svg"], + accc.CacheWriter.sparse_checkout_patterns( + [ + "package.xml", + "./Resources/icon.svg", + "Resources\\icon.svg", + "/Resources/icon.svg", + ] + ), + )