commit: 7d50c226b79d158ed08e925369560727993fb0b7 Author: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org> AuthorDate: Sat Jan 21 09:44:17 2023 +0000 Commit: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org> CommitDate: Sat Jan 21 09:44:17 2023 +0000 URL: https://gitweb.gentoo.org/proj/pkgcore/pkgcheck.git/commit/?id=7d50c226
SuspiciousSrcUriChange: fix false positive with mirrors The original code for comparing for URL change was giving false positives when the URI is mirror based, since an object was used to compare. Pre-compute the URI string and use strings for comparison. Also add a test for that failure. Resolves: https://github.com/pkgcore/pkgcheck/issues/531 Signed-off-by: Arthur Zamarin <arthurzam <AT> gentoo.org> src/pkgcheck/checks/git.py | 26 ++++++++++++++------------ tests/checks/test_git.py | 6 ++++++ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/pkgcheck/checks/git.py b/src/pkgcheck/checks/git.py index 764dfc5d..adc16874 100644 --- a/src/pkgcheck/checks/git.py +++ b/src/pkgcheck/checks/git.py @@ -186,16 +186,10 @@ class SrcUriChecksumChange(results.PackageResult, results.Error): class SuspiciousSrcUriChange(results.PackageResult, results.Warning): """Suspicious SRC_URI changing URI without distfile rename.""" - def __init__(self, old_uri, new_uri, filename, **kwargs): + def __init__(self, old_uri: str, new_uri: str, filename: str, **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.old_uri = old_uri + self.new_uri = new_uri self.filename = filename @property @@ -381,18 +375,26 @@ class GitPkgCommitsCheck(GentooRepoCheck, GitCommitsCheck): else: yield MissingSlotmove(old_slot, new_slot, pkg=new_pkg) + @staticmethod + def _fetchable_str(fetch: fetchable) -> str: + uri = tuple(fetch.uri._uri_source)[0] + if isinstance(uri, tuple): + return f"mirror://{uri[0].mirror_name}/{uri[1]}" + else: + return str(uri) + def src_uri_changes(self, pkgset): pkg = pkgset[0].unversioned_atom try: new_checksums = { - fetch.filename: (fetch.chksums, tuple(fetch.uri._uri_source)) + fetch.filename: (fetch.chksums, self._fetchable_str(fetch)) 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)) + fetch.filename: (fetch.chksums, self._fetchable_str(fetch)) for pkg in self.modified_repo(pkgset).match(pkg) for fetch in iflatten_instance(pkg.fetchables, fetchable) } @@ -406,7 +408,7 @@ class GitPkgCommitsCheck(GentooRepoCheck, GitCommitsCheck): 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) + yield SuspiciousSrcUriChange(old_uri, new_uri, filename, pkg=pkg) def feed(self, pkgset: list[git.GitPkgChange]): # Mapping of commit types to pkgs, available commit types can be seen diff --git a/tests/checks/test_git.py b/tests/checks/test_git.py index b69893d8..eed40e13 100644 --- a/tests/checks/test_git.py +++ b/tests/checks/test_git.py @@ -702,6 +702,12 @@ class TestGitPkgCommitsCheck(ReportTestCase): 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")) + # revert change and check for no report with same mirror url + self.child_git_repo.run(["git", "reset", "--hard", "origin/main"]) + self.child_repo.create_ebuild("cat/pkg-1", src_uri=old_url, eapi="8") + self.child_git_repo.add_all("cat/pkg: bump EAPI", signoff=True) + self.init_check() + self.assertNoReport(self.check, self.source) class TestGitEclassCommitsCheck(ReportTestCase):
