[Backport main] Don't let sparse clones find files in subdirectories

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,20 +611,27 @@ 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,
         addon_id: str,
         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]:

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",
+                ]
+            ),
+        )