proj/pkgcore/pkgcheck:master commit in: src/pkgcheck/checks/, /, tests/checks/
"Arthur Zamarin" <[email protected]>
| Newsgroups | gmane.linux.gentoo.cvs |
|---|---|
| Message-ID | <1786108018.894de0c7d13234f3ce5addfc41225a81f0473067.arthurzam@gentoo> |
commit: 894de0c7d13234f3ce5addfc41225a81f0473067
Author: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org>
AuthorDate: Fri Aug 7 13:06:58 2026 +0000
Commit: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org>
CommitDate: Fri Aug 7 13:06:58 2026 +0000
URL: https://gitweb.gentoo.org/proj/pkgcore/pkgcheck.git/commit/?id=894de0c7
GitPkgCommitsCheck: fix crash on removals spanning several commits
_RemovalRepo._populate() reconstructs the pre-removal state of a package
from a single `git archive <commit>~1`, choosing the earliest removal by
commit time, and then registers every removed version with the repo
object. That one tree isn't guaranteed to hold all of them: a revbump
done as `git mv pkg-2.ebuild pkg-2-r1.ebuild` in one commit, plus the
removal of another version in a second commit, leaves pkg-2.ebuild absent
from the second commit's parent while old_pkg() still registers it as
removed. pkgcore then reads that missing file to determine the EAPI for
pkg.supported and raises FileNotFoundError.
The archive point lands on the wrong commit in two ways. The two commits
may share a commit timestamp, in which case min() ties and resolves by
set iteration order, making the crash PYTHONHASHSEED dependent and so
appear intermittent. Commit times may also genuinely run out of
topological order after a rebase or a cherry-pick.
Extract the removed ebuilds that are still missing after the initial
archive from their own commit's parent. Only removals are considered,
since an addition's ebuild is absent from its parent commit by definition
and probing for those would add a failing git call per added version on a
hot path. The archive point itself is untouched, so a normal scan issues
no extra git calls and callers relying on a tarfile.ReadError to skip
broken ebuilds keep seeing it.
Resolves: https://github.com/pkgcore/pkgcheck/issues/675
Resolves: https://github.com/pkgcore/pkgcheck/issues/756
Signed-off-by: Arthur Zamarin <arthurzam <AT> gentoo.org>
NEWS.rst | 12 ++++++++++++
src/pkgcheck/checks/git.py | 41 +++++++++++++++++++++++++++++++++++------
tests/checks/test_git.py | 23 +++++++++++++++++++++++
3 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/NEWS.rst b/NEWS.rst
index 461a9c9b..c6e4b682 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -2,6 +2,18 @@
Release Notes
=============
+-----------------------------
+pkgcheck 0.10.43 (unreleased)
+-----------------------------
+
+**Fixes:**
+
+- GitPkgCommitsCheck: fix a ``FileNotFoundError`` crash when a package's
+ removals span several commits, e.g. a revbump done as a rename followed by the
+ removal of another version. The historical repo was archived from a single
+ commit's parent, which doesn't necessarily hold every removed version, while
+ all of them were registered with it (Arthur Zamarin, #675, #756)
+
-----------------------------
pkgcheck 0.10.42 (2026-07-31)
-----------------------------
diff --git a/src/pkgcheck/checks/git.py b/src/pkgcheck/checks/git.py
index 0b2606b0..a5b140e0 100644
--- a/src/pkgcheck/checks/git.py
+++ b/src/pkgcheck/checks/git.py
@@ -288,7 +288,7 @@ class _RemovalRepo(UnconfiguredTree):
def cleanup(self):
self.__tmpdir.cleanup()
- def __call__(self, pkgs):
+ def __call__(self, pkgs: list[git.GitPkgChange]):
"""Update the repo with a given sequence of packages."""
self._populate(pkgs)
if self.__created:
@@ -298,28 +298,57 @@ class _RemovalRepo(UnconfiguredTree):
self.__created = True
return self
- def _populate(self, pkgs):
+ def _populate(self, pkgs: list[git.GitPkgChange]):
"""Populate the repo with a given sequence of historical packages."""
pkg = min(pkgs, key=attrgetter("time"))
paths = [pjoin(pkg.category, pkg.package)]
for subdir in ("eclass", "profiles"):
if os.path.exists(pjoin(self.__parent_repo.location, subdir)):
paths.append(subdir)
+ self._extract(pkg.commit, paths)
+ self._populate_missing(pkgs, pkg)
+
+ def _extract(self, commit: str, paths: list[str], required: bool = True):
+ """Extract paths from a commit's parent commit into the repo."""
old_files = subprocess.Popen(
- ["git", "archive", f"{pkg.commit}~1"] + paths,
+ ["git", "archive", f"{commit}~1"] + paths,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
cwd=self.__parent_repo.location,
)
- if old_files.poll():
+ if required and old_files.poll():
error = old_files.stderr.read().decode().strip()
raise PkgcheckUserException(f"failed populating archive repo: {error}")
# https://docs.python.org/3.12/library/tarfile.html#tarfile-extraction-filter
if hasattr(tarfile, "data_filter"):
# https://docs.python.org/3.12/library/tarfile.html#tarfile.TarFile.extraction_filter
tarfile.TarFile.extraction_filter = staticmethod(tarfile.data_filter)
- with tarfile.open(mode="r|", fileobj=old_files.stdout) as tar:
- tar.extractall(path=self.location)
+ try:
+ with tarfile.open(mode="r|", fileobj=old_files.stdout) as tar:
+ tar.extractall(path=self.location)
+ except tarfile.ReadError:
+ # git wrote no archive, the paths are absent from the tree
+ if required:
+ raise
+ finally:
+ old_files.stdout.close()
+ old_files.stderr.close()
+ old_files.wait()
+
+ def _populate_missing(self, pkgs: list[git.GitPkgChange], archived: git.GitPkgChange):
+ """Extract removed ebuilds absent from the already archived commit."""
+ missing: dict[str, list[str]] = defaultdict(list)
+ for pkg in pkgs:
+ if pkg.commit == archived.commit:
+ continue
+ if pkg.status != "D" and not (pkg.status == "R" and pkg.old is None):
+ continue
+ path = pjoin(pkg.category, pkg.package, f"{pkg.package}-{pkg.fullver}.ebuild")
+ if not os.path.exists(pjoin(self.location, path)):
+ missing[pkg.commit].append(path)
+
+ for commit, paths in missing.items():
+ self._extract(commit, paths, required=False)
class GitPkgCommitsCheck(GentooRepoCheck, GitCommitsCheck):
diff --git a/tests/checks/test_git.py b/tests/checks/test_git.py
index 8e871647..f4857965 100644
--- a/tests/checks/test_git.py
+++ b/tests/checks/test_git.py
@@ -10,6 +10,7 @@ from pkgcore.ebuild.cpv import UnversionedCPV as CP
from pkgcore.ebuild.cpv import VersionedCPV as CPV
from pkgcore.test.misc import FakeRepo
from snakeoil.cli import arghparse
+from snakeoil.contexts import os_environ
from snakeoil.fileutils import touch
from pkgcheck.addons.git import GitCommit
@@ -582,6 +583,28 @@ class TestGitPkgCommitsCheck(ReportTestCase):
expected = git_mod.DroppedUnstableKeywords(["~amd64"], commit, pkg=CPV("cat/pkg-1"))
assert r == expected
+ def test_removal_after_rename_in_earlier_dated_commit(self):
+ # keep a version of each pkg around so no keywords get dropped
+ for cpv in ("cat/aaa-1", "cat/aaa-2", "cat/pkg-1", "cat/pkg-2"):
+ self.parent_repo.create_ebuild(cpv, keywords=["~amd64"])
+ self.parent_git_repo.add_all("cat/aaa, cat/pkg: version bumps")
+ self.child_git_repo.run(["git", "pull", "origin", "main"])
+
+ # 'cat/aaa' sorts first, so its removal populates the shared removal
+ # repo before cat/pkg is checked, which is when versions get registered
+ self.child_git_repo.remove("cat/aaa/aaa-1.ebuild", msg="cat/aaa: remove 1")
+
+ # revbump cat/pkg-2 through a rename, then remove cat/pkg-1 in a later
+ # commit carrying an earlier commit time, as a rebase produces; that
+ # removal's parent tree no longer holds pkg-2.ebuild
+ with os_environ(GIT_COMMITTER_DATE="2021-02-16T00:20:00"):
+ self.child_git_repo.move("cat/pkg/pkg-2.ebuild", "cat/pkg/pkg-2-r1.ebuild")
+ with os_environ(GIT_COMMITTER_DATE="2021-02-16T00:10:00"):
+ self.child_git_repo.remove("cat/pkg/pkg-1.ebuild", msg="cat/pkg: remove 1")
+
+ self.init_check()
+ self.assertNoReport(self.check, self.source)
+
def test_dropped_keywords_inherit_eclass(self):
# add stable ebuild to parent repo
with open(pjoin(self.parent_git_repo.path, "eclass/make.eclass"), "w") as f: