@@ -23,6 +23,13 @@ pkgdev 0.2.18 (unreleased)
   version the stablereq check considers due for stabilization if possible
   (Arthur Zamarin)
 
+**pkgdev commit:**
+
+- commit: the repository of pre-change packages is now built over an already
+  filled directory, instead of being populated underneath a live repository
+  and having its caches invalidated through a pkgcore internal meant for
+  mutable repositories (Arthur Zamarin, #172)
+
 **pkgdev showkw:**
 
 - showkw: drop the vendored copy of tabulate in favor of a dependency on
 
@@ -9,6 +9,7 @@
 import tempfile
 import textwrap
 from collections import UserDict, defaultdict, deque
+from collections.abc import Iterable
 from dataclasses import dataclass
 from enum import Enum
 from functools import partial
@@ -292,35 +293,26 @@ def __call__(self, parser, namespace, values, option_string=None):
 class HistoricalRepo(UnconfiguredTree):
     """Repository of historical packages stored in a temporary directory."""
 
-    def __init__(self, parent_repo, tmpdir, *args, **kwargs):
-        self.__parent_repo = parent_repo
+    def __init__(self, tmpdir, *args, **kwargs):
         # cache tmpdir to delay cleanup until exit
         self.__tmpdir = tmpdir
         super().__init__(*args, **kwargs)
 
-    def add_pkgs(self, pkgs):
-        """Update the repo with a given sequence of packages."""
-        self._populate(pkgs)
-        # notify the repo object that new pkgs were added
-        for pkg in pkgs:
-            self.notify_add_package(pkg)
-
-    def _populate(self, pkgs):
-        """Populate the repo with a given sequence of historical packages."""
-        paths = {pkg.key for pkg in pkgs}
-        old_files = subprocess.Popen(
-            ["git", "archive", "HEAD"] + list(paths),
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
-            cwd=self.__parent_repo.location,
-        )
-        if old_files.poll():  # pragma: no cover
-            error = old_files.stderr.read().decode().strip()
-            raise Exception(f"failed populating archive repo: {error}")
-        with tarfile.open(mode="r|", fileobj=old_files.stdout) as tar:
-            # see filter in https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extractall
-            # Whilst we trust git archive, we still leave the basic protections on.
-            tar.extractall(path=self.location, filter="data")
+
+def _extract_historical(repo, path: str, keys: Iterable[str]):
+    """Extract the packages as they are in HEAD into a directory."""
+    old_files = subprocess.Popen(
+        ["git", "archive", "HEAD", *keys],
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        cwd=repo.location,
+    )
+    if old_files.poll():  # pragma: no cover
+        error = old_files.stderr.read().decode().strip()
+        raise RuntimeError(f"failed populating archive repo: {error}")
+    with tarfile.open(mode="r|", fileobj=old_files.stdout) as tar:
+        # Whilst we trust git archive, we still leave the basic protections on.
+        tar.extractall(path=path, filter="data")
 
 
 # GLEP 81 categories, mapped to what their packages allocate
@@ -349,13 +341,14 @@ class ChangeSummary:
     # mapping of handled statuses to functions
     status_funcs = None
 
-    def __init__(self, options):
+    def __init__(self, options, changes: "Iterable[PkgChange]"):
         self.options = options
         self.repo = options.repo
+        self.changes = {x.atom: x for x in changes}
 
     @jit_attr
     def old_repo(self):
-        """Create a repository of historical packages removed from git."""
+        """Create a repository of the changed packages as they are in HEAD."""
         tmpdir = tempfile.TemporaryDirectory()
         repo_dir = tmpdir.name
 
@@ -367,8 +360,10 @@ def old_repo(self):
         with open(pjoin(repo_dir, "profiles", "repo_name"), "w") as f:
             f.write(f"{self.repo.repo_id}-old\n")
 
+        _extract_historical(self.repo, repo_dir, {atom.key for atom in self.changes})
+
         repo_config = RepoConfig(repo_dir)
-        tree_cls = partial(HistoricalRepo, self.repo, tmpdir)
+        tree_cls = partial(HistoricalRepo, tmpdir)
         return tree(self.options.config, repo_config, tree_cls=tree_cls)
 
     def __str__(self):
@@ -387,16 +382,11 @@ class MetadataSummary(ChangeSummary):
 
     status_funcs: ClassVar[dict[str, callable]] = {}
 
-    def __init__(self, options, changes):
-        super().__init__(options)
-        self.changes = {x.atom: x for x in changes}
-
     @change("M")
     def modify(self):
         """Generate summaries for modify actions."""
         atom = next(iter(self.changes))
         pkgs = self.repo.match(atom)
-        self.old_repo.add_pkgs(pkgs)
         try:
             old_pkg = self.old_repo.match(atom)[0]
             new_pkg = pkgs[0]
@@ -443,10 +433,6 @@ class PkgSummary(ChangeSummary):
 
     status_funcs: ClassVar[dict[str, callable]] = {}
 
-    def __init__(self, options, changes):
-        super().__init__(options)
-        self.changes = {x.atom: x for x in changes}
-
     @jit_attr
     def versions(self):
         """Tuple of package versions that were changed."""
@@ -535,7 +521,6 @@ def modify(self):
         summaries = set()
         for atom in self.changes:
             pkgs = self.repo.match(atom)
-            self.old_repo.add_pkgs(pkgs)
             try:
                 old_pkg = self.old_repo.match(atom)[0]
                 new_pkg = pkgs[0]
 
@@ -6,12 +6,15 @@
 from functools import partial
 from io import StringIO
 from os.path import join as pjoin
+from types import SimpleNamespace
 from unittest.mock import patch
 
 import pytest
+from pkgcore.ebuild.atom import atom as atom_cls
 from snakeoil.contexts import os_environ
 
 from pkgdev.mangle import copyright_regex, keywords_regex
+from pkgdev.scripts import pkgdev_commit as commit
 from pkgdev.scripts import run
 
 
@@ -1194,3 +1197,33 @@ def test_failed_manifest(self, capsys, repo, make_git_repo):
         out, err = capsys.readouterr()
         assert not err
         assert out == " * cat/pkg-1: invalid EAPI '-1'\n"
+
+
+class TestHistoricalRepo:
+    def mk(self, repo, git_repo, tool, pkgs):
+        for cpvstr in pkgs:
+            repo.create_ebuild(cpvstr, description="old")
+        git_repo.add_all("initial")
+        for cpvstr in pkgs:
+            repo.create_ebuild(cpvstr, description="new")
+        with chdir(repo.location):
+            options, _ = tool.parse_args(["commit", "-a"])
+        changes = [SimpleNamespace(atom=atom_cls(f"={cpvstr}"), status="M") for cpvstr in pkgs]
+        return commit.MetadataSummary(options, changes)
+
+    def test_populated_without_being_told(self, repo, make_git_repo, tool):
+        git_repo = make_git_repo(repo.location)
+        summary = self.mk(repo, git_repo, tool, ["cat/pkg-1"])
+        # no add_pkgs call: the directory is filled before the repo is built
+        old_pkg = summary.old_repo.match(atom_cls("=cat/pkg-1"))[0]
+        assert old_pkg.description == "old"
+
+    def test_covers_every_changed_package(self, repo, make_git_repo, tool):
+        git_repo = make_git_repo(repo.location)
+        summary = self.mk(repo, git_repo, tool, ["cat/a-1", "cat/b-1"])
+        # a second package must not need the repo's caches invalidated
+        for cpvstr in ("cat/a-1", "cat/b-1"):
+            assert summary.old_repo.match(atom_cls(f"={cpvstr}"))[0].description == "old"
+
+    def test_no_reliance_on_the_mutable_repo_backchannel(self):
+        assert not hasattr(commit.HistoricalRepo, "add_pkgs")