From 15cff72f56eb3008c2d6df5bd5c1be8e3b2e6732 Mon Sep 17 00:00:00 2001 From: Tim Harder Date: Sat, 5 Jun 2021 21:11:53 -0600 Subject: GitCommitMessageCheck: flag pkg additions missing versions in the summary Fixes #298. --- src/pkgcheck/addons/git.py | 19 +++++++++++++------ src/pkgcheck/checks/git.py | 20 +++++++++++++------- tests/addons/test_git.py | 18 +++++++++++------- tests/checks/test_git.py | 27 +++++++++++++++++++++------ 4 files changed, 58 insertions(+), 26 deletions(-) diff --git a/src/pkgcheck/addons/git.py b/src/pkgcheck/addons/git.py index ff7ee770..79becc34 100644 --- a/src/pkgcheck/addons/git.py +++ b/src/pkgcheck/addons/git.py @@ -5,11 +5,11 @@ import os import re import shlex import subprocess -from collections import deque +from collections import defaultdict, deque from dataclasses import dataclass from datetime import datetime from functools import partial -from itertools import chain, takewhile +from itertools import takewhile from pathspec import PathSpec from pkgcore.ebuild import cpv @@ -21,7 +21,7 @@ from pkgcore.restrictions import packages from snakeoil.cli import arghparse from snakeoil.contexts import GitStash from snakeoil.klass import jit_attr -from snakeoil.mappings import OrderedSet +from snakeoil.mappings import ImmutableDict, OrderedSet from snakeoil.osutils import pjoin from snakeoil.process import CommandNotFound, find_binary from snakeoil.strings import pluralism @@ -41,7 +41,7 @@ class GitCommit: author: str committer: str message: tuple - pkgs: tuple = () + pkgs: ImmutableDict = ImmutableDict() def __str__(self): return self.hash @@ -180,8 +180,15 @@ class GitRepoCommits(_ParseGitRepo): author = next(self.git_log) committer = next(self.git_log) message = list(takewhile(lambda x: x != '\x00', self.git_log)) - pkgs = list(chain.from_iterable(pkgs for _, pkgs in self.changes)) - return GitCommit(commit_hash, commit_time, author, committer, message, pkgs) + pkgs = defaultdict(set) + for status, atoms in self.changes: + if status == 'R': + old, new = atoms + pkgs['A'].add(new) + pkgs['D'].add(old) + else: + pkgs[status].update(atoms) + return GitCommit(commit_hash, commit_time, author, committer, message, ImmutableDict(pkgs)) class GitRepoPkgs(_ParseGitRepo): diff --git a/src/pkgcheck/checks/git.py b/src/pkgcheck/checks/git.py index 212ce2dd..47f659b9 100644 --- a/src/pkgcheck/checks/git.py +++ b/src/pkgcheck/checks/git.py @@ -562,18 +562,24 @@ class GitCommitMessageCheck(GentooRepoCheck, GitCommitsCheck): # categorize package changes pkg_changes = defaultdict(set) - for pkg in commit.pkgs: - pkg_changes[pkg.category].add(pkg.package) + for atom in chain.from_iterable(commit.pkgs.values()): + pkg_changes[atom.category].add(atom) # check git commit summary formatting if len(pkg_changes) == 1: - category, pkgs = pkg_changes.popitem() - if len(pkgs) == 1: + category, atoms = pkg_changes.popitem() + if len({x.package for x in atoms}) == 1: # changes to a single cat/pn - pkg = commit.pkgs[0] - if not re.match(rf'^({re.escape(pkg.key)}|{re.escape(pkg.cpvstr)}): ', summary): - error = f'summary missing {pkg.key!r} package prefix' + atom = next(iter(atoms)) + if not re.match(rf'^({re.escape(atom.key)}|{re.escape(atom.cpvstr)}): ', summary): + error = f'summary missing {atom.key!r} package prefix' yield BadCommitSummary(error, summary, commit=commit) + # check for version in summary for singular version bumps + if len(commit.pkgs['A']) == 1: + version = next(iter(commit.pkgs['A'])).version + if not re.match(rf'^.+\b{version}\b.*$', summary): + error = f'summary missing package version {version!r}' + yield BadCommitSummary(error, summary, commit=commit) else: # mutiple pkg changes in the same category if not re.match(rf'^{re.escape(category)}: ', summary): diff --git a/tests/addons/test_git.py b/tests/addons/test_git.py index 9a6d78eb..cf39efc3 100644 --- a/tests/addons/test_git.py +++ b/tests/addons/test_git.py @@ -250,7 +250,7 @@ class TestGitRepoCommits: commits = list(git.GitRepoCommits(path, 'HEAD')) assert len(commits) == 1 assert commits[0].message == ['foo'] - assert commits[0].pkgs == [] + assert commits[0].pkgs == {} orig_commit = commits[0] # make another commit @@ -258,7 +258,7 @@ class TestGitRepoCommits: commits = list(git.GitRepoCommits(path, 'HEAD')) assert len(commits) == 2 assert commits[0].message == ['bar'] - assert commits[0].pkgs == [] + assert commits[0].pkgs == {} assert commits[1] == orig_commit assert len(set(commits)) == 2 @@ -268,7 +268,7 @@ class TestGitRepoCommits: commits = list(git.GitRepoCommits(path, 'HEAD')) assert len(commits) == 3 assert commits[0].message == ['cat/pkg-0'] - assert commits[0].pkgs == [atom_cls('=cat/pkg-0')] + assert commits[0].pkgs == {'A': {atom_cls('=cat/pkg-0')}} # make a multiple pkg commit repo.create_ebuild('newcat/newpkg-0') @@ -277,19 +277,23 @@ class TestGitRepoCommits: commits = list(git.GitRepoCommits(path, 'HEAD')) assert len(commits) == 4 assert commits[0].message == ['newcat: various updates'] - assert commits[0].pkgs == [atom_cls('=newcat/newpkg-0'), atom_cls('=newcat/newpkg-1')] + assert commits[0].pkgs == { + 'A': {atom_cls('=newcat/newpkg-0'), atom_cls('=newcat/newpkg-1')}} # remove the old version git_repo.remove('newcat/newpkg/newpkg-0.ebuild') commits = list(git.GitRepoCommits(path, 'HEAD')) assert len(commits) == 5 - assert commits[0].pkgs == [atom_cls('=newcat/newpkg-0')] + assert commits[0].pkgs == {'D': {atom_cls('=newcat/newpkg-0')}} # rename the pkg git_repo.move('newcat', 'newcat2') commits = list(git.GitRepoCommits(path, 'HEAD')) assert len(commits) == 6 - assert commits[0].pkgs == [atom_cls('=newcat/newpkg-1'), atom_cls('=newcat2/newpkg-1')] + assert commits[0].pkgs == { + 'A': {atom_cls('=newcat2/newpkg-1')}, + 'D': {atom_cls('=newcat/newpkg-1')}, + } # malformed atoms don't show up as pkgs repo.create_ebuild('cat/pkg-3') @@ -298,7 +302,7 @@ class TestGitRepoCommits: fake_atom.side_effect = MalformedAtom('bad atom') commits = list(git.GitRepoCommits(path, 'HEAD')) assert len(commits) == 7 - assert commits[0].pkgs == [] + assert commits[0].pkgs == {} class TestGitRepoPkgs: diff --git a/tests/checks/test_git.py b/tests/checks/test_git.py index 9b65c31b..809d128e 100644 --- a/tests/checks/test_git.py +++ b/tests/checks/test_git.py @@ -285,14 +285,29 @@ class TestGitCommitMessageRepoCheck(ReportTestCase): # poorly prefixed commit summary self.child_repo.create_ebuild('cat/pkg-4') - self.child_git_repo.add_all('version bump', signoff=True) - commit = self.child_git_repo.HEAD + self.child_git_repo.add_all('version bump to 4', signoff=True) + commit1 = self.child_git_repo.HEAD + # commit summary missing package version + self.child_repo.create_ebuild('cat/pkg-5') + self.child_git_repo.add_all('cat/pkg: version bump', signoff=True) + commit2 = self.child_git_repo.HEAD + # commit summary missing renamed package version + self.child_git_repo.move( + 'cat/pkg/pkg-3.ebuild', 'cat/pkg/pkg-6.ebuild', + msg='cat/pkg: version bump and remove old', signoff=True) + commit3 = self.child_git_repo.HEAD self.init_check() - r = self.assertReport(self.check, self.source) - expected = git_mod.BadCommitSummary( + results = self.assertReports(self.check, self.source) + r1 = git_mod.BadCommitSummary( "summary missing 'cat/pkg' package prefix", - 'version bump', commit=commit) - assert r == expected + 'version bump to 4', commit=commit1) + r2 = git_mod.BadCommitSummary( + "summary missing package version '5'", + 'cat/pkg: version bump', commit=commit2) + r3 = git_mod.BadCommitSummary( + "summary missing package version '6'", + 'cat/pkg: version bump and remove old', commit=commit3) + assert set(results) == {r1, r2, r3} def test_bad_commit_summary_category(self): # properly prefixed commit summary -- cgit v1.2.3-65-gdbad