Clone each repository once when it ships several addons (closes #52)

src/mygit.py

@@ -118,12 +118,12 @@ class _RemoteLsResult:
 def fetch_states(requests: list[RemoteStateRequest]) -> Iterator[RemoteState]:
     with TemporaryDirectory() as repo_dir:
         repo = pygit2.init_repository(repo_dir)
-        remote_name_to_branches: dict[str, list[str]] = {}
+        remote_name_to_branches: dict[str, dict[str, None]] = {}
         for request in requests:
             name = sha1(request.url.encode("utf-8")).hexdigest()
             if not _has_remote(repo, name):
                 repo.remotes.create(name, request.url)
-            remote_name_to_branches.setdefault(name, []).append(request.branch)
+            remote_name_to_branches.setdefault(name, {})[request.branch] = None
 
         def ls(remote: pygit2.Remote) -> _RemoteLsResult:
             try:

src/snapjaw.py

@@ -226,6 +226,42 @@ def install_addon(
             )
             config.save()
 
+        installed_addon_keys = {addon_key(addon.name) for addon in addons_by_dir.values()}
+        ghost_candidates = [
+            (key, addon)
+            for key, addon in config.addons_by_key.items()
+            if (addon.url, addon.branch) == (repo_url, repo.branch)
+            and addon_key(addon.name) not in installed_addon_keys
+        ]
+        if ghost_candidates:
+            repository_addon_keys = {addon_key(addon.name) for addon in toc.find_addons(repo.workdir, None)}
+        else:
+            repository_addon_keys = set()
+
+        removed_ghost = False
+        for key, configured_addon in ghost_candidates:
+            if addon_key(configured_addon.name) in repository_addon_keys:
+                continue
+
+            del config.addons_by_key[key]
+            removed_ghost = True
+            configured_addon_dir = os.path.join(addons_dir, configured_addon.name)
+            if os.path.isdir(configured_addon_dir):
+                if configured_addon.checksum is not None and signature.validate(
+                    configured_addon_dir, configured_addon.checksum
+                ):
+                    remove_addon_dir(configured_addon_dir)
+                    logging.info(f'Removed stale addon "{configured_addon.name}"')
+                else:
+                    logging.warning(
+                        f'Stale addon folder "{configured_addon.name}" was left in place because it was modified'
+                    )
+            else:
+                logging.info(f'Removed stale addon "{configured_addon.name}" from config')
+
+        if removed_ghost:
+            config.save()
+
         logging.info("Done")
 
 
@@ -261,18 +297,27 @@ def cmd_update(config: Config, args):
         addons = [get_addon_from_config(config, name) for name in args.names]
     else:
         addons = []
+        errors_by_repo: dict[tuple[str, str], tuple[list[str], str]] = {}
         for state in get_addon_states(config, args.addons_dir):
             if state.error is not None:
-                print(f"Error: {state.addon}: {state.error}")
+                addon = config.addons_by_key.get(addon_key(state.addon))
+                if addon is None:
+                    print(f"Error: {state.addon}: {state.error}")
+                else:
+                    names, _ = errors_by_repo.setdefault((addon.url, addon.branch), ([], state.error))
+                    names.append(state.addon)
             elif state.status == AddonStatus.Outdated:
                 addons.append(config.addons_by_key[addon_key(state.addon)])
+        for names, error in errors_by_repo.values():
+            print(f"Error: {', '.join(names)}: {error}")
 
     if not addons:
         print("No addons to update found")
         return
 
-    for addon in addons:
-        install_addon(config, addon.url, addon.branch, args.addons_dir, args.expansion)
+    repos = {(addon.url, addon.branch): None for addon in addons}
+    for url, branch in repos:
+        install_addon(config, url, branch, args.addons_dir, args.expansion)
 
 
 def get_addon_from_config(config: Config, addon_name: str) -> ConfigAddon:
@@ -362,7 +407,11 @@ def get_addon_states(config: Config, addons_dir: str) -> list[AddonState]:
     addon_key_to_state = {}
     total_addons = len(config.addons_by_key)
     processed = 0
-    requests = [mygit.RemoteStateRequest(addon.url, addon.branch) for addon in config.addons_by_key.values()]
+    requests = [
+        mygit.RemoteStateRequest(url, branch)
+        for url, branch_to_addons in url_to_branch_to_addons.items()
+        for branch in branch_to_addons
+    ]
     for state in mygit.fetch_states(requests):
         for addon in url_to_branch_to_addons[state.url][state.branch]:
             processed += 1

src/toc.py

@@ -18,22 +18,31 @@ class _TocFile:
     game_version: int
 
 
-def find_addons(dir_path: str, expansion: Expansion) -> Generator[Addon, None, None]:
+def find_addons(dir_path: str, expansion: Expansion | None) -> Generator[Addon, None, None]:
     def sort_key(toc: _TocFile) -> int:
         return len(toc.path.parents)
 
     addon_paths = []
     for toc_file in sorted(_find_toc_files(dir_path, expansion), key=sort_key):
         if all(parent not in addon_paths for parent in toc_file.path.parents):
-            yield Addon(name=toc_file.path.stem, path=str(toc_file.path.parent))
+            addon_dir = toc_file.path.parent
+            name = toc_file.path.stem
+            if addon_dir != Path(dir_path) and any(
+                path.is_file()
+                and path.suffix.casefold() == ".toc"
+                and path.stem.casefold() == addon_dir.name.casefold()
+                for path in addon_dir.iterdir()
+            ):
+                name = addon_dir.name
+            yield Addon(name=name, path=str(toc_file.path.parent))
             addon_paths.append(toc_file.path.parent)
 
 
-def _find_toc_files(root_dir: str, expansion: Expansion) -> Generator[_TocFile, None, None]:
+def _find_toc_files(root_dir: str, expansion: Expansion | None) -> Generator[_TocFile, None, None]:
     for path in Path(root_dir).rglob("*"):
         if path.is_file() and path.suffix.lower() == ".toc":
             game_version = _get_game_version(path)
-            if game_version is not None and _interface_matches(game_version, expansion):
+            if game_version is not None and (expansion is None or _interface_matches(game_version, expansion)):
                 yield _TocFile(path, game_version)
 
 

tests/test_mygit.py

@@ -147,6 +147,46 @@ def test_branch_not_found_in_refs(self, mock_remote, mock_pygit2_repo, fetch_sta
 
         assert len(states) == 0
 
+    def test_duplicate_requests_yield_one_state(self, mock_remote, mock_pygit2_repo, fetch_states_patches):
+        """Repeated requests for one URL and branch are deduplicated."""
+        url = "https://github.com/test/repo.git"
+        remote = mock_remote(
+            "abc123",
+            url,
+            refs=[SimpleNamespace(name="refs/heads/main", symref_target="", oid="deadbeef")],
+        )
+        mock_pygit2_repo.remotes.__iter__ = MagicMock(return_value=iter([remote]))
+        requests = [RemoteStateRequest(url, "main"), RemoteStateRequest(url, "main")]
+
+        with fetch_states_patches():
+            states = list(fetch_states(requests))
+
+        assert [(state.url, state.branch) for state in states] == [(url, "main")]
+        remote.list_heads.assert_called_once_with()
+
+    def test_two_branches_share_one_list_heads_call(self, mock_remote, mock_pygit2_repo, fetch_states_patches):
+        """Distinct ordered branches on one URL use one remote lookup."""
+        url = "https://github.com/test/repo.git"
+        remote = mock_remote(
+            "abc123",
+            url,
+            refs=[
+                SimpleNamespace(name="refs/heads/main", symref_target="", oid="maincommit"),
+                SimpleNamespace(name="refs/heads/release", symref_target="", oid="releasecommit"),
+            ],
+        )
+        mock_pygit2_repo.remotes.__iter__ = MagicMock(return_value=iter([remote]))
+        requests = [RemoteStateRequest(url, "release"), RemoteStateRequest(url, "main")]
+
+        with fetch_states_patches():
+            states = list(fetch_states(requests))
+
+        assert [(state.branch, state.head_commit_hex) for state in states] == [
+            ("release", "releasecommit"),
+            ("main", "maincommit"),
+        ]
+        remote.list_heads.assert_called_once_with()
+
 
 class TestCloneInternalProcess:
     """Tests for _clone() - the subprocess worker function.