GitPkgCommitsCheck: catch SRC_URI mistakes

Resolves: #493
Signed-off-by: Arthur Zamarin <arthurzam@gentoo.org>

Author: arthurzam

src/pkgcheck/checks/git.py

@@ -13,9 +13,11 @@
 
 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 @@ def desc(self):
         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 @@ def modified_checks(self, pkgs):
             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 @@ def feed(self, pkgset):
                 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.

tests/checks/test_git.py

@@ -7,7 +7,7 @@
 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 @@ def test_revision_move(self):
         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):