commit: 09559c09f2389246ea98261832e281a9baaedbdf Author: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org> AuthorDate: Sat Nov 26 17:06:06 2022 +0000 Commit: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org> CommitDate: Fri Jan 20 20:36:42 2023 +0000 URL: https://gitweb.gentoo.org/proj/pkgcore/pkgcheck.git/commit/?id=09559c09
GitPkgCommitsCheck: catch SRC_URI mistakes Resolves: https://github.com/pkgcore/pkgcheck/issues/493 Signed-off-by: Arthur Zamarin <arthurzam <AT> gentoo.org> src/pkgcheck/checks/git.py | 67 +++++++++++++++++++++++++++++++++++++++++++++- tests/checks/test_git.py | 55 ++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 2 deletions(-) diff --git a/src/pkgcheck/checks/git.py b/src/pkgcheck/checks/git.py index 6e48d47f..764dfc5d 100644 --- a/src/pkgcheck/checks/git.py +++ b/src/pkgcheck/checks/git.py @@ -13,9 +13,11 @@ from urllib.parse import urlparse from pkgcore.ebuild.misc import sort_keywords from pkgcore.ebuild.repository import UnconfiguredTree +from pkgcore.fetch import fetchable from snakeoil import klass from snakeoil.mappings import ImmutableDict from snakeoil.osutils import pjoin +from snakeoil.sequences import iflatten_instance from snakeoil.strings import pluralism from .. import base, results, sources @@ -169,6 +171,38 @@ class MissingMove(results.PackageResult, results.Error): return f"renamed package: {self.old} -> {self.new}" +class SrcUriChecksumChange(results.PackageResult, results.Error): + """SRC_URI changing checksum without distfile rename.""" + + def __init__(self, filename, **kwargs): + super().__init__(**kwargs) + self.filename = filename + + @property + def desc(self): + return f"{self.filename!r} has different checksums across commits" + + +class SuspiciousSrcUriChange(results.PackageResult, results.Warning): + """Suspicious SRC_URI changing URI without distfile rename.""" + + def __init__(self, old_uri, new_uri, filename, **kwargs): + super().__init__(**kwargs) + if isinstance(old_uri, tuple): + self.old_uri = f"mirror://{old_uri[0].mirror_name}/{old_uri[1]}" + else: + self.old_uri = str(old_uri) + if isinstance(new_uri, tuple): + self.new_uri = f"mirror://{new_uri[0].mirror_name}/{new_uri[1]}" + else: + self.new_uri = str(new_uri) + self.filename = filename + + @property + def desc(self): + return f"{self.filename!r} has changed SRC_URI from {self.old_uri!r} to {self.new_uri!r}" + + class _RemovalRepo(UnconfiguredTree): """Repository of removed packages stored in a temporary directory.""" @@ -235,6 +269,8 @@ class GitPkgCommitsCheck(GentooRepoCheck, GitCommitsCheck): DroppedUnstableKeywords, MissingSlotmove, MissingMove, + SrcUriChecksumChange, + SuspiciousSrcUriChange, ] ) @@ -345,7 +381,34 @@ class GitPkgCommitsCheck(GentooRepoCheck, GitCommitsCheck): else: yield MissingSlotmove(old_slot, new_slot, pkg=new_pkg) - def feed(self, pkgset): + def src_uri_changes(self, pkgset): + pkg = pkgset[0].unversioned_atom + + try: + new_checksums = { + fetch.filename: (fetch.chksums, tuple(fetch.uri._uri_source)) + for pkg in self.repo.match(pkg) + for fetch in iflatten_instance(pkg.fetchables, fetchable) + } + + old_checksums = { + fetch.filename: (fetch.chksums, tuple(fetch.uri._uri_source)) + for pkg in self.modified_repo(pkgset).match(pkg) + for fetch in iflatten_instance(pkg.fetchables, fetchable) + } + except (IndexError, FileNotFoundError, tarfile.ReadError): + # ignore broken ebuild + return + + for filename in old_checksums.keys() & new_checksums.keys(): + old_checksum, old_uri = old_checksums[filename] + new_checksum, new_uri = new_checksums[filename] + if old_checksum != new_checksum: + yield SrcUriChecksumChange(filename, pkg=pkg) + elif old_uri != new_uri: + yield SuspiciousSrcUriChange(old_uri[0], new_uri[0], filename, pkg=pkg) + + def feed(self, pkgset: list[git.GitPkgChange]): # Mapping of commit types to pkgs, available commit types can be seen # under the --diff-filter option in git log parsing support and are # disambiguated as follows: @@ -407,6 +470,8 @@ class GitPkgCommitsCheck(GentooRepoCheck, GitCommitsCheck): if not pkg.maintainers and newly_added: yield DirectNoMaintainer(pkg=pkg) + yield from self.src_uri_changes(pkgset) + class MissingSignOff(results.CommitResult, results.Error): """Local commit with missing sign offs. diff --git a/tests/checks/test_git.py b/tests/checks/test_git.py index 0294f0b3..b69893d8 100644 --- a/tests/checks/test_git.py +++ b/tests/checks/test_git.py @@ -7,7 +7,7 @@ import pytest from pkgcheck.base import PkgcheckUserException from pkgcheck.checks import git as git_mod from pkgcheck.addons.git import GitCommit -from pkgcore.ebuild.cpv import VersionedCPV as CPV +from pkgcore.ebuild.cpv import VersionedCPV as CPV, UnversionedCPV as CP from pkgcore.test.misc import FakeRepo from snakeoil.cli import arghparse from snakeoil.fileutils import touch @@ -650,6 +650,59 @@ class TestGitPkgCommitsCheck(ReportTestCase): self.init_check() self.assertNoReport(self.check, self.source) + def test_checksum_change(self): + distfile = [ + "DIST", + "pkgcheck-1.tar.gz", + "549746", + "BLAKE2B", + "72ed97d93674ffd311978d03ad3738494a752bf1b02bea5eaaaf1b066c48e8c9ec5f82b79baeeabf3e56e618c76614ee6179b7115d1d875364ac6e3fbc3c6028", + "SHA512", + "6a8c135ca44ccbfe15548bd396aba9448c29f60147920b18b8be5aa5fcd1200e0b75bc5de50fc7892ad5460ddad1e7d28a7e44025bdc581a518d136eda8b0df2", + ] + with open(pjoin(self.parent_repo.path, "profiles/thirdpartymirrors"), "a") as f: + f.write("gentoo https://gentoo.org/distfiles\n";) + self.parent_repo.create_ebuild("cat/pkg-1", src_uri=f"mirror://gentoo/{distfile[1]}") + with open(pjoin(self.parent_repo.path, "cat/pkg/Manifest"), "w") as f: + f.write(" ".join(distfile) + "\n") + self.parent_git_repo.add_all("cat/pkg: add 1", signoff=True) + # pull changes and change checksum in child repo + self.child_git_repo.run(["git", "pull", "origin", "main"]) + self.child_repo.create_ebuild("cat/pkg-1-r1", src_uri=f"mirror://gentoo/{distfile[1]}") + distfile[-1] = distfile[-1][:-1] + "0" + with open(pjoin(self.child_repo.path, "cat/pkg/Manifest"), "w") as f: + f.write(" ".join(distfile) + "\n") + self.child_git_repo.add_all("cat/pkg: revbump", signoff=True) + self.init_check() + r = self.assertReport(self.check, self.source) + assert r == git_mod.SrcUriChecksumChange(distfile[1], pkg=CP("cat/pkg")) + + def test_src_uri_change(self): + distfile = [ + "DIST", + "pkgcheck-1.tar.gz", + "549746", + "BLAKE2B", + "72ed97d93674ffd311978d03ad3738494a752bf1b02bea5eaaaf1b066c48e8c9ec5f82b79baeeabf3e56e618c76614ee6179b7115d1d875364ac6e3fbc3c6028", + "SHA512", + "6a8c135ca44ccbfe15548bd396aba9448c29f60147920b18b8be5aa5fcd1200e0b75bc5de50fc7892ad5460ddad1e7d28a7e44025bdc581a518d136eda8b0df2", + ] + old_url = f"mirror://gentoo/{distfile[1]}" + new_url = f"https://pkgcore.github.io/pkgcheck/{distfile[1]}"; + with open(pjoin(self.parent_repo.path, "profiles/thirdpartymirrors"), "a") as f: + f.write("gentoo https://gentoo.org/distfiles\n";) + self.parent_repo.create_ebuild("cat/pkg-1", src_uri=old_url) + with open(pjoin(self.parent_repo.path, "cat/pkg/Manifest"), "w") as f: + f.write(" ".join(distfile) + "\n") + self.parent_git_repo.add_all("cat/pkg: add 1", signoff=True) + # pull changes and change checksum in child repo + self.child_git_repo.run(["git", "pull", "origin", "main"]) + self.child_repo.create_ebuild("cat/pkg-1", src_uri=new_url) + self.child_git_repo.add_all("cat/pkg: change SRC_URI", signoff=True) + self.init_check() + r = self.assertReport(self.check, self.source) + assert r == git_mod.SuspiciousSrcUriChange(old_url, new_url, distfile[1], pkg=CP("cat/pkg")) + class TestGitEclassCommitsCheck(ReportTestCase):
