[Backport main] Improved large addon installation

Addon.py

@@ -179,6 +179,16 @@ def __init__(
         self.display_name = self.name
         self.url = url.strip()
         self.relative_cache_path = ""
+
+        # A remote location for a zip of this Addon's contents. This is used for Addons that are
+        # not cached in their entirety (typically due to their size). The canonical example here is
+        # the Parts Library.
+        self.zip_url = ""
+
+        # True for Addons that are large enough that downloading all of them for every update is
+        # expensive, so git is used for them whenever it is available. Set by the Addon Index.
+        self.prefer_git = False
+
         self.branch = branch.strip()
         self.branch_display_name = branch.strip()
         self.repo_type = Addon.Kind.WORKBENCH
@@ -329,7 +339,7 @@ def load_metadata_file(self, file: str) -> None:
                 fci.Console.PrintWarning(
                     "An invalid or corrupted package.xml file was found in the cache for"
                 )
-                fci.Console.PrintWarning(f" {self.name}... ignoring the bad data.\n")
+                fci.Console.PrintWarning(f" {self.name}… ignoring the bad data.\n")
                 return
             self.set_metadata(metadata)
             self._clean_url()
@@ -348,7 +358,7 @@ def _load_installed_metadata(self) -> None:
                 fci.Console.PrintWarning(
                     "An invalid or corrupted package.xml file was found in installation of"
                 )
-                fci.Console.PrintWarning(f" {self.name}... ignoring the bad data.\n")
+                fci.Console.PrintWarning(f" {self.name}… ignoring the bad data.\n")
                 return
 
     def set_metadata(self, metadata: Metadata) -> None:
@@ -688,7 +698,9 @@ def _find_classname_in_file(current_file) -> str:
         return ""
 
     def get_zip_url(self) -> str:
-        if self.url.endswith(".zip"):
+        if self.zip_url:
+            zip_url = self.zip_url
+        elif self.url.endswith(".zip"):
             zip_url = self.url
         else:
             # The ZIP url is based on the location of the main cache file:
@@ -835,7 +847,7 @@ def package_is_installed(package_name: str) -> bool:
         # can do the check by PyPI package name:
         if importlib_metadata is None:
             fci.Console.PrintMessage(
-                f"Cannot check for installation of `{package_name}`... marking it for "
+                f"Cannot check for installation of `{package_name}`… marking it for "
                 "reinstallation to be safe\n"
             )
             return False

AddonCatalog.py

@@ -81,7 +81,7 @@ class AddonCatalogEntry:
     metadata: Optional[CatalogEntryMetadata] = None  # Generated by the cache system
     last_update_time: str = ""  # Generated by the cache system
     curated: bool = True  # Generated by the cache system
-    sparse_cache: bool = False  # Generated by the cache system
+    sparse_cache: bool = False  # Set by the catalog for Addons too large to cache in full
     relative_cache_path: str = ""  # Generated by the cache system
     git_hash: Optional[str] = None  # Generated by the cache system
     git_tag: Optional[str] = None  # Generated by the cache system
@@ -138,21 +138,22 @@ def instantiate_addon(self, addon_id: str) -> Addon:
             state = Addon.Status.UNCHECKED
         else:
             state = Addon.Status.NOT_INSTALLED
-        if self.sparse_cache:
-            if self.zip_url:
-                url = self.zip_url
-            else:
-                # Technically, this should never happen, but just in case...
-                raise RuntimeError(f"Sparse cache entry {addon_id} has no zip_url")
-        elif self.repository:
-            url = self.repository
-        else:
-            url = self.zip_url
+        if self.sparse_cache and not self.zip_url:
+            # Technically, this should never happen, but just in case...
+            raise RuntimeError(f"Sparse cache entry {addon_id} has no zip_url")
+        url = self.repository or self.zip_url or ""
         if self.git_ref:
             addon = Addon(addon_id, url, state, branch=self.git_ref)
         else:
             addon = Addon(addon_id, url, state)
         addon.relative_cache_path = self.relative_cache_path
+        if self.sparse_cache or not self.repository:
+            # If the cache is sparse, we need a "real" location to get the thing from when installing
+            addon.zip_url = self.zip_url or ""
+        # If it's too big to cache, it's probably too big to want to update by re-downloading the
+        # whole thing. So if the user's machine has git on it, and we know its git repo, then tell
+        # the Addon Manager to try to use git when installing/updating.
+        addon.prefer_git = bool(self.sparse_cache and self.repository)
 
         if self.metadata:
             try:

AddonCatalog.schema.json

@@ -56,6 +56,9 @@
           },
           "curated": {
             "type": "boolean"
+          },
+          "sparse_cache": {
+            "type": "boolean"
           }
         },
         "anyOf": [

AddonCatalogCacheCreator.py

@@ -55,9 +55,6 @@
     300  # Seconds: repos that take longer than this are assumed to be too large to index
 )
 
-# Repos that are too large, or that should for some reason not be fully cloned here
-FORCE_SPARSE_CLONE = ["parts_library", "offline-documentation", "FreeCAD-Documentation-html"]
-
 
 def recursive_serialize(obj: Any):
     """Recursively serialize an object, supporting non-dataclasses that themselves contain
@@ -205,18 +202,8 @@ def create_local_copy_of_single_addon(
         self, addon_id: str, catalog_entries: List[AddonCatalog.AddonCatalogEntry]
     ):
         for index, catalog_entry in enumerate(catalog_entries):
-            if addon_id in FORCE_SPARSE_CLONE:
-                if catalog_entry.repository is None:
-                    print(
-                        f"ERROR: Cannot use sparse clone for {addon_id} because it has no git repo."
-                    )
-                    continue
-                if catalog_entry.zip_url is None:
-                    print(
-                        f"ERROR: Cannot use sparse clone for {addon_id} because it has no zip URL."
-                    )
-                    continue
-                catalog_entry.sparse_cache = True
+            catalog_entry.sparse_cache = self.should_use_sparse_clone(addon_id, catalog_entry)
+            if catalog_entry.sparse_cache:
                 self.create_local_copy_of_single_addon_with_git_sparse(
                     addon_id, index, catalog_entry
                 )
@@ -236,6 +223,24 @@ def create_local_copy_of_single_addon(
             self.catalog.add_git_info_to_entry(addon_id, index, git_hash, git_tag)
             self.create_zip_of_entry(addon_id, index, catalog_entry)
 
+    def should_use_sparse_clone(
+        self, addon_id: str, catalog_entry: AddonCatalog.AddonCatalogEntry
+    ) -> bool:
+        """Whether to cache only the metadata files of this Addon, leaving clients to get the rest
+        of it from its zip URL. The catalog asks for this by setting "sparse_cache" on Addons that
+        are too large to cache in full, but it takes both a repository to clone the files from and
+        a zip URL for the clients to use, so an entry without those is cached normally."""
+
+        if not catalog_entry.sparse_cache:
+            return False
+        if catalog_entry.repository is None:
+            print(f"ERROR: Cannot use sparse clone for {addon_id} because it has no git repo.")
+            return False
+        if catalog_entry.zip_url is None:
+            print(f"ERROR: Cannot use sparse clone for {addon_id} because it has no zip URL.")
+            return False
+        return True
+
     def get_git_info(
         self, addon_id: str, index: int, catalog_entry: AddonCatalog.AddonCatalogEntry
     ) -> Tuple[str | None, str | None]:

AddonManagerTest/app/mocks.py

@@ -26,9 +26,9 @@
 import os
 from typing import List
 
-
-class GitFailed(RuntimeError):
-    pass
+# The real exception types, so that code under test catches what this mock raises. Importing them
+# does not require git to be installed: only constructing a real GitManager does.
+from addonmanager_git import GitFailed, GitCancelled
 
 
 class MockConsole:
@@ -236,26 +236,25 @@ def __init__(self):
         self.get_last_authors_response = {"Jane Doe": {"email": "jdoe@freecad.org", "count": 1}}
         self.should_fail = False
         self.fail_once = False  # Switch back to success after the simulated failure
+        self.should_be_interrupted = False  # Emulate the user cancelling the operation
 
     def _check_for_failure(self):
+        if self.should_be_interrupted:
+            raise GitCancelled("Unit test forced interruption")
         if self.should_fail:
             if self.fail_once:
                 self.should_fail = False
             raise GitFailed("Unit test forced failure")
 
-    def clone(self, _remote, _local_path, _args: List[str] = None):
+    def clone(self, _remote, _local_path, _args: List[str] = None, line_callback=None):
         self.called_methods.append("clone")
         self._check_for_failure()
 
-    def async_clone(self, _remote, _local_path, _progress_monitor, _args: List[str] = None):
-        self.called_methods.append("async_clone")
-        self._check_for_failure()
-
     def checkout(self, _local_path, _spec, _args: List[str] = None):
         self.called_methods.append("checkout")
         self._check_for_failure()
 
-    def update(self, _local_path):
+    def update(self, _local_path, line_callback=None):
         self.called_methods.append("update")
         self._check_for_failure()
 
@@ -268,10 +267,6 @@ def reset(self, _local_path, _args: List[str] = None):
         self.called_methods.append("reset")
         self._check_for_failure()
 
-    def async_fetch_and_update(self, _local_path, _progress_monitor, _args=None):
-        self.called_methods.append("async_fetch_and_update")
-        self._check_for_failure()
-
     def update_available(self, _local_path) -> bool:
         self.called_methods.append("update_available")
         self._check_for_failure()

AddonManagerTest/app/test_addon_catalog_cache_creator.py

@@ -252,6 +252,89 @@ def test_generate_cache_entry_with_nothing_to_cache(self):
     def test_generate_cache_entry_with_approval(self):
         """If the addon appears in the catalog (as opposed to just the index), it gets marked as approved."""
 
+    def test_should_use_sparse_clone_when_the_catalog_asks_for_it(self):
+        """A catalog entry that asks for a sparse cache and has everything needed for one gets
+        one, even though its addon is not in the hard-coded list."""
+        ace = AddonCatalog.AddonCatalogEntry(
+            {
+                "repository": "https://some.url",
+                "git_ref": "main",
+                "zip_url": "zip",
+                "sparse_cache": True,
+            }
+        )
+        writer = accc.CacheWriter()
+        self.assertTrue(writer.should_use_sparse_clone("SomeLargeAddon", ace))
+
+    def test_should_use_sparse_clone_without_a_zip_url(self):
+        """A sparse cache leaves clients to download the addon from its zip URL, so an entry
+        without one is cached normally instead."""
+        ace = AddonCatalog.AddonCatalogEntry(
+            {"repository": "https://some.url", "git_ref": "main", "sparse_cache": True}
+        )
+        writer = accc.CacheWriter()
+        self.assertFalse(writer.should_use_sparse_clone("SomeLargeAddon", ace))
+
+    def test_should_use_sparse_clone_without_a_repository(self):
+        """The metadata files of a sparse cache come from a git clone, so an entry without a
+        repository is cached normally instead."""
+        ace = AddonCatalog.AddonCatalogEntry({"zip_url": "zip", "sparse_cache": True})
+        writer = accc.CacheWriter()
+        self.assertFalse(writer.should_use_sparse_clone("SomeLargeAddon", ace))
+
+    def test_should_use_sparse_clone_for_a_normal_addon(self):
+        """An addon that does not ask for a sparse cache is cached in full."""
+        ace = AddonCatalog.AddonCatalogEntry(
+            {"repository": "https://some.url", "git_ref": "main", "zip_url": "zip"}
+        )
+        writer = accc.CacheWriter()
+        self.assertFalse(writer.should_use_sparse_clone("SomeNormalAddon", ace))
+
+    @patch("AddonCatalogCacheCreator.CacheWriter.create_local_copy_of_single_addon_with_git_sparse")
+    def test_create_local_copy_of_single_addon_using_sparse_clone(self, mock_create_with_sparse):
+        """An entry that asks for a sparse cache is fetched with a sparse clone, and is marked so
+        that clients know that only part of it is cached."""
+        catalog_entries = [
+            AddonCatalog.AddonCatalogEntry(
+                {
+                    "repository": "https://some.url",
+                    "git_ref": "main",
+                    "zip_url": "zip",
+                    "sparse_cache": True,
+                }
+            ),
+        ]
+        writer = accc.CacheWriter()
+        writer.catalog = MagicMock()
+        writer.cwd = os.path.abspath(os.path.join("home", "cache"))
+
+        writer.create_local_copy_of_single_addon("SomeLargeAddon", catalog_entries)
+
+        self.assertEqual(1, mock_create_with_sparse.call_count)
+        self.assertTrue(catalog_entries[0].sparse_cache)
+
+    @patch("AddonCatalogCacheCreator.CacheWriter.create_local_copy_of_single_addon_with_git")
+    @patch("AddonCatalogCacheCreator.CacheWriter.create_local_copy_of_single_addon_with_git_sparse")
+    def test_create_local_copy_of_single_addon_with_impossible_sparse_clone(
+        self, mock_create_with_sparse, mock_create_with_git
+    ):
+        """An entry that asks for a sparse cache but cannot have one is cached in full, and is not
+        marked as sparse: clients must not be told to look for a zip that does not exist."""
+        catalog_entries = [
+            AddonCatalog.AddonCatalogEntry(
+                {"repository": "https://some.url", "git_ref": "main", "sparse_cache": True}
+            ),
+        ]
+        writer = accc.CacheWriter()
+        writer.catalog = MagicMock()
+        writer.cwd = os.path.abspath(os.path.join("home", "cache"))
+
+        writer.create_local_copy_of_single_addon("SomeLargeAddon", catalog_entries)
+
+        self.assertEqual(0, mock_create_with_sparse.call_count)
+        self.assertEqual(1, mock_create_with_git.call_count)
+        self.assertFalse(catalog_entries[0].sparse_cache)
+
     @patch("AddonCatalogCacheCreator.CacheWriter.create_local_copy_of_single_addon_with_git")
     def test_create_local_copy_of_single_addon_using_git(self, mock_create_with_git):
         """Given a single addon, each catalog entry is fetched with git if git info is available."""
@@ -312,9 +395,15 @@ def get_catalog(self):
                         AddonCatalog.AddonCatalogEntry({"zip_url": "zip1"}),
                         AddonCatalog.AddonCatalogEntry({"zip_url": "zip2"}),
                     ],
-                    accc.FORCE_SPARSE_CLONE[0]: [
-                        AddonCatalog.AddonCatalogEntry({"zip_url": "zip1"}),
-                        AddonCatalog.AddonCatalogEntry({"zip_url": "zip2"}),
+                    "TestMod3": [
+                        AddonCatalog.AddonCatalogEntry(
+                            {
+                                "repository": "https://some.url",
+                                "git_ref": "main",
+                                "zip_url": "zip1",
+                                "sparse_cache": True,
+                            }
+                        ),
                     ],
                 }
 

AddonManagerTest/app/test_addoncatalog.py

@@ -90,6 +90,68 @@ def test_version_match_with_min_and_max_bad_match_low(self):
             )
             self.assertFalse(ac.is_compatible())
 
+    def test_instantiate_addon_with_repository(self):
+        """An Addon that is cached by the Addon Manager keeps its repository as its URL, and has no
+        zip URL of its own: the cached copy is downloaded instead."""
+        ac = self.AddonCatalogEntry(
+            {
+                "repository": "https://github.com/FreeCAD/FreeCAD",
+                "git_ref": "main",
+                "zip_url": "https://github.com/FreeCAD/FreeCAD/archive/main.zip",
+                "relative_cache_path": "AddonManager/AnAddon.zip",
+            }
+        )
+
+        addon = ac.instantiate_addon("AnAddon")
+
+        self.assertEqual("https://github.com/FreeCAD/FreeCAD", addon.url)
+        self.assertEqual("", addon.zip_url)
+        self.assertFalse(addon.prefer_git)
+
+    def test_instantiate_addon_with_sparse_cache(self):
+        """A sparsely-cached Addon must be downloaded from the catalog's zip, because only a
+        fraction of it is in the cache, but its URL is still the repository it came from, so that
+        file locations such as its README can be constructed from it."""
+        ac = self.AddonCatalogEntry(
+            {
+                "repository": "https://github.com/FreeCAD/FreeCAD-library",
+                "git_ref": "master",
+                "zip_url": "https://github.com/FreeCAD/FreeCAD-library/archive/master.zip",
+                "sparse_cache": True,
+                "relative_cache_path": "AddonManager/parts_library.zip",
+            }
+        )
+
+        addon = ac.instantiate_addon("parts_library")
+
+        self.assertEqual("https://github.com/FreeCAD/FreeCAD-library", addon.url)
+        self.assertEqual(
+            "https://github.com/FreeCAD/FreeCAD-library/archive/master.zip", addon.get_zip_url()
+        )
+        self.assertTrue(addon.prefer_git)
+
+    def test_instantiate_addon_with_sparse_cache_and_no_zip(self):
+        """A sparsely-cached Addon with no zip URL cannot be downloaded at all."""
+        ac = self.AddonCatalogEntry(
+            {
+                "repository": "https://github.com/FreeCAD/FreeCAD-library",
+                "git_ref": "master",
+                "sparse_cache": True,
+            }
+        )
+
+        with self.assertRaises(RuntimeError):
+            ac.instantiate_addon("parts_library")
+
+    def test_instantiate_addon_with_zip_only(self):
+        """An Addon with no repository is downloaded from the catalog's zip."""
+        ac = self.AddonCatalogEntry({"zip_url": "https://example.com/an_addon.zip"})
+
+        addon = ac.instantiate_addon("AnAddon")
+
+        self.assertEqual("https://example.com/an_addon.zip", addon.url)
+        self.assertEqual("https://example.com/an_addon.zip", addon.get_zip_url())
+
 
 class TestAddonCatalog(TestCase):
     """Tests for the AddonCatalog class."""