GitCommitMessageCheck: flag pkg additions missing versions in the summary

Fixes #298.

Author: radhermit

src/pkgcheck/addons/git.py

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

src/pkgcheck/checks/git.py

@@ -562,18 +562,24 @@ def feed(self, commit):
 
         # 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):

tests/addons/test_git.py

@@ -250,15 +250,15 @@ def test_parsing(self, make_repo, make_git_repo):
         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
         git_repo.add('bar', msg='bar', create=True)
         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 @@ def test_parsing(self, make_repo, make_git_repo):
         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 @@ def test_parsing(self, make_repo, make_git_repo):
         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 @@ def test_parsing(self, make_repo, make_git_repo):
             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:

tests/checks/test_git.py

@@ -285,14 +285,29 @@ def test_bad_commit_summary_pkg(self):
 
         # 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