[Patch] contrig/gcc-changelog: Check that PR in subject in in changelog (was:: git gcc-commit-mklog doesn't extract PR number to ChangeLog)

[Tobias Burnus <tobias@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 779 bytes --]

On 10.06.21 10:07, Martin Liška wrote:
> On 6/10/21 8:35 AM, Tobias Burnus wrote:
>> One options would be to require a 'PR <comp>/<nnnnn+>' line if there is
>> 'PRnnnnn+' in the commit title, rejecting the commit otherwise.
>
> Quite interesting idea! Are you willing to prepare a patch for it?

Done.

I was thinking of 'PRs 1234 and 4566' but it soon became too special and
I did not want to reject valid cases (false positive), hence, I settled
on the attached variant. (It should detect the most common issues, thus,
I don't worry about false negatives.)

OK? Comments?

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

[-- Attachment #2: gcc-changelog.diff --]
[-- Type: text/x-patch, Size: 10962 bytes --]

contrig/gcc-changelog: Check that PR in subject in in changelog

This patch checks that a '[PRnnnn]' and '(PRnnnn)' also appears as PR in the
changelog part of the commit message.  And it does likewise for 'PR comp/nnnn'
except that then also the component name is checked.  (Note that the reverse
is permitted, i.e. PR(s) only appearing in the changelog.)
To avoid false positives, PR numbers in the subject line are ignored,
if 'revert' appears.

contrib/ChangeLog:

	* gcc-changelog/git_commit.py (pr_regex): Add ?P<pr> for group('pr').
	(subject_pr_skip_regex, subject_pr_regex, subject_pr2_regex): New.
	(GitInfo.__init__, GitCommit.parse_changelog): Check subject PRs.
	* gcc-changelog/git_email.py (SUBJECT_PREFIX): New.
	(GitEmail.__init__): Parse 'Subject:' and pass it to GitInfo.
	* gcc-changelog/git_repository.py (parse_git_revisions): Pass first
	line to GitInfo as subject line.
	* gcc-changelog/test_email.py (test_pr_only_in_subject,
	test_wrong_pr_comp_in_subject): New.
	* gcc-changelog/test_patches.txt (0030-PR-c-92746, pr-check1.patch):
	Update to avoid triggering the new check.
	(0001-rs6000-Support-doubleword, pr-wrong-comp.patch): New.

diff --git a/contrib/gcc-changelog/git_commit.py b/contrib/gcc-changelog/git_commit.py
index bd8c1ff7af2..5513e1d32e4 100755
--- a/contrib/gcc-changelog/git_commit.py
+++ b/contrib/gcc-changelog/git_commit.py
@@ -156,7 +156,11 @@ author_line_regex = \
         re.compile(r'^(?P<datetime>\d{4}-\d{2}-\d{2})\ {2}(?P<name>.*  <.*>)')
 additional_author_regex = re.compile(r'^\t(?P<spaces>\ *)?(?P<name>.*  <.*>)')
 changelog_regex = re.compile(r'^(?:[fF]or +)?([a-z0-9+-/]*)ChangeLog:?')
-pr_regex = re.compile(r'\tPR (?P<component>[a-z+-]+\/)?([0-9]+)$')
+subject_pr_skip_regex = re.compile(r'[Rr]evert')
+subject_pr_regex = \
+        re.compile(r'(^|\W)PR\s+(?P<component>[a-zA-Z+-]+)/(?P<pr>\d{4,7})')
+subject_pr2_regex = re.compile(r'[(\[]PR\s*(?P<pr>\d{4,7})[)\]]')
+pr_regex = re.compile(r'\tPR (?P<component>[a-z+-]+\/)?(?P<pr>[0-9]+)$')
 dr_regex = re.compile(r'\tDR ([0-9]+)$')
 star_prefix_regex = re.compile(r'\t\*(?P<spaces>\ *)(?P<content>.*)')
 end_of_location_regex = re.compile(r'[\[<(:]')
@@ -279,10 +283,11 @@ class ChangeLogEntry:
 
 
 class GitInfo:
-    def __init__(self, hexsha, date, author, lines, modified_files):
+    def __init__(self, hexsha, date, author, subject, lines, modified_files):
         self.hexsha = hexsha
         self.date = date
         self.author = author
+        self.subject = subject
         self.lines = lines
         self.modified_files = modified_files
 
@@ -298,6 +303,7 @@ class GitCommit:
         self.top_level_authors = []
         self.co_authors = []
         self.top_level_prs = []
+        self.subject_prs = set()
         self.cherry_pick_commit = None
         self.revert_commit = None
         self.commit_to_info_hook = commit_to_info_hook
@@ -307,6 +313,18 @@ class GitCommit:
         if self.info.lines and self.info.lines[0] == 'Update copyright years.':
             return
 
+        # Extract PR numbers form the subject line
+        # Match either [PRnnnn] / (PRnnnn) or PR component/nnnn
+        if not subject_pr_skip_regex.search(info.subject):
+            self.subject_prs = \
+                 set([m.group('pr')
+                      for m in subject_pr2_regex.finditer(info.subject)])
+            for m in subject_pr_regex.finditer(info.subject):
+                if not m.group('component') in bug_components:
+                    self.errors.append(Error('invalid PR component in subject',
+                                             info.subject))
+                self.subject_prs.add(m.group('pr'))
+
         # Identify first if the commit is a Revert commit
         for line in self.info.lines:
             m = revert_regex.match(line)
@@ -346,6 +364,10 @@ class GitCommit:
             if not self.errors:
                 self.check_mentioned_files()
                 self.check_for_correct_changelog()
+        if self.subject_prs:
+            self.errors.append(Error("PR %s in subject but not in changelog" %
+                                     ", ".join(self.subject_prs),
+                                     self.info.subject))
 
     @property
     def success(self):
@@ -460,7 +482,8 @@ class GitCommit:
                     else:
                         author_tuple = (m.group('name'), None)
                 elif pr_regex.match(line):
-                    component = pr_regex.match(line).group('component')
+                    m = pr_regex.match(line)
+                    component = m.group('component')
                     if not component:
                         self.errors.append(Error('missing PR component', line))
                         continue
@@ -469,6 +492,8 @@ class GitCommit:
                         continue
                     else:
                         pr_line = line.lstrip()
+                    if m.group('pr') in self.subject_prs:
+                        self.subject_prs.remove(m.group('pr'))
                 elif dr_regex.match(line):
                     pr_line = line.lstrip()
 
diff --git a/contrib/gcc-changelog/git_email.py b/contrib/gcc-changelog/git_email.py
index fa62e3ad2f7..0f6460c3f0d 100755
--- a/contrib/gcc-changelog/git_email.py
+++ b/contrib/gcc-changelog/git_email.py
@@ -28,6 +28,7 @@ from unidiff import PatchSet, PatchedFile
 
 DATE_PREFIX = 'Date: '
 FROM_PREFIX = 'From: '
+SUBJECT_PREFIX = 'Subject: '
 unidiff_supports_renaming = hasattr(PatchedFile(), 'is_rename')
 
 
@@ -37,7 +38,9 @@ class GitEmail(GitCommit):
         diff = PatchSet.from_filename(filename)
         date = None
         author = None
+        subject = None
 
+        subject_last = False
         with open(self.filename, 'r') as f:
             lines = f.read().splitlines()
         lines = list(takewhile(lambda line: line != '---', lines))
@@ -46,6 +49,15 @@ class GitEmail(GitCommit):
                 date = parse(line[len(DATE_PREFIX):])
             elif line.startswith(FROM_PREFIX):
                 author = GitCommit.format_git_author(line[len(FROM_PREFIX):])
+            elif line.startswith(SUBJECT_PREFIX):
+                subject = line[len(SUBJECT_PREFIX):]
+                subject_last = True
+            elif subject_last and line.startswith(' '):
+                subject += line
+            elif line == '':
+                break
+            else:
+                subject_last = False
         header = list(takewhile(lambda line: line != '', lines))
         body = lines[len(header) + 1:]
 
@@ -67,7 +79,7 @@ class GitEmail(GitCommit):
             else:
                 t = 'M'
             modified_files.append((target if t != 'D' else source, t))
-        git_info = GitInfo(None, date, author, body, modified_files)
+        git_info = GitInfo(None, date, author, subject, body, modified_files)
         super().__init__(git_info,
                          commit_to_info_hook=lambda x: None)
 
diff --git a/contrib/gcc-changelog/git_repository.py b/contrib/gcc-changelog/git_repository.py
index 2d688826ff8..d2aed8fa14f 100755
--- a/contrib/gcc-changelog/git_repository.py
+++ b/contrib/gcc-changelog/git_repository.py
@@ -59,8 +59,9 @@ def parse_git_revisions(repo_path, revisions, ref_name=None):
 
             date = datetime.utcfromtimestamp(c.committed_date)
             author = '%s  <%s>' % (c.author.name, c.author.email)
-            git_info = GitInfo(c.hexsha, date, author,
-                               c.message.split('\n'), modified_files)
+            message = c.message.split('\n')
+            git_info = GitInfo(c.hexsha, date, author, message[0],
+                               message[1:], modified_files)
             return git_info
         except ValueError:
             return None
diff --git a/contrib/gcc-changelog/test_email.py b/contrib/gcc-changelog/test_email.py
index 6d6596370c4..aff2cf855dd 100755
--- a/contrib/gcc-changelog/test_email.py
+++ b/contrib/gcc-changelog/test_email.py
@@ -427,3 +427,12 @@ class TestGccChangelog(unittest.TestCase):
     def test_multi_same_file(self):
         email = self.from_patch_glob('0001-OpenMP-Fix-SIMT')
         assert email.errors[0].message == 'same file specified multiple times'
+
+    def test_pr_only_in_subject(self):
+        email = self.from_patch_glob('0001-rs6000-Support-doubleword')
+        assert (email.errors[0].message ==
+                'PR 100085 in subject but not in changelog')
+
+    def test_wrong_pr_comp_in_subject(self):
+        email = self.from_patch_glob('pr-wrong-comp.patch')
+        assert email.errors[0].message == 'invalid PR component in subject'
diff --git a/contrib/gcc-changelog/test_patches.txt b/contrib/gcc-changelog/test_patches.txt
index 39d40b88618..dbd377fba2d 100644
--- a/contrib/gcc-changelog/test_patches.txt
+++ b/contrib/gcc-changelog/test_patches.txt
@@ -1461,6 +1461,7 @@ Subject: [PATCH 0030/2034] 	PR c++/92746 - ICE with noexcept of function
 Another place that needs to specially handle Concepts TS function-style
 concepts.
 
+	PR c++/92746
 	* except.c (check_noexcept_r): Handle concept-check.
 ---
  gcc/cp/ChangeLog                            | 3 +++
@@ -1977,7 +1978,7 @@ index aac31d02b6c..56c470f6ecf 100644
 From 5194b51ed9714808d88827531e91474895b6c706 Mon Sep 17 00:00:00 2001
 From: Jason Merrill <jason@redhat.com>
 Date: Thu, 16 Jan 2020 16:55:39 -0500
-Subject: [PATCH 0121/2034] PR c++/93286 - ICE with __is_constructible and
+Subject: [PATCH 0121/2034] Revert PR c++/93286 - ICE with __is_constructible and
  variadic template.
 
 Here we had been recursing in tsubst_copy_and_build if type2 was a TREE_LIST
@@ -3406,3 +3407,44 @@ index 00000000000..21540512e23
 +
 -- 
 2.25.1
+=== 0001-rs6000-Support-doubleword ===
+From f700e4b0ee3ef53b48975cf89be26b9177e3a3f3 Mon Sep 17 00:00:00 2001
+From: Xionghu Luo <luoxhu@linux.ibm.com>
+Date: Tue, 8 Jun 2021 21:48:12 -0500
+Subject: [PATCH] rs6000: Support doubleword swaps removal in rot64 load store
+ [PR100085]
+
+gcc/testsuite/ChangeLog:
+
+	* gcc.target/powerpc/pr100085.c: New test.
+---
+diff --git a/gcc/testsuite/gcc.target/powerpc/pr100085.c b/gcc/testsuite/gcc.target/powerpc/pr100085.c
+new file mode 100644
+index 00000000000..7d8b147b127
+--- /dev/null
++++ b/gcc/testsuite/gcc.target/powerpc/pr100085.c
+@@ -0,0 +1,1 @@
++
+-- 
+2.25.1
+=== pr-wrong-comp.patch ===
+From 5194b51ed9714808d88827531e91474895b6c706 Mon Sep 17 00:00:00 2001
+From: Jason Merrill <jason@redhat.com>
+Date: Thu, 16 Jan 2020 16:55:39 -0500
+Subject: [PATCH 0121/2034] PR some/93286 - ICE with __is_constructible and
+ variadic template.
+
+gcc/testsuite/ChangeLog:
+
+	PR c++/93286
+	* gcc.target/powerpc/pr100085.c: New test.
+---
+diff --git a/gcc/testsuite/gcc.target/powerpc/pr100085.c b/gcc/testsuite/gcc.target/powerpc/pr100085.c
+new file mode 100644
+index 00000000000..7d8b147b127
+--- /dev/null
++++ b/gcc/testsuite/gcc.target/powerpc/pr100085.c
+@@ -0,0 +1,1 @@
++
+-- 
+2.25.1