On Wed, 12 Mar 2025 at 08:05, Richard Biener <richard.guent...@gmail.com> wrote:
>
> On Tue, Mar 11, 2025 at 5:58 PM Christophe Lyon
> <christophe.l...@linaro.org> wrote:
> >
> > In https://gcc.gnu.org/contribute.html#patches we ask to use [PRnnnn]
> > without the Bugzilla component identifier and with no space between
> > 'PR' and the number, but git_check_commit.py accepts all forms. The
> > patch enforces what we document.
> >
> > Note that this would reject a few of the recent commits.
>
> Why would we be this restrictive? I personally am using
Well, someone made me realize that I should have used [PRnnnn] rather
than (PR nnnn) in a recent commit, as documented in "Contributing to
GCC".
Since I use gcc-verify, I looked at why I missed that, and hence
proposed this patch, so that our tools match what we document....
>
> bugzilla-component/number - description
>
> IMO 'PR' is redundant and the component helps screening for area of
> maintenance.
>
> That said, I suppose my form wouldn't be rejected, so I'm happy. I just
> wonder why we should force something I don't remember we discussed
> at any point.
>
.... so another alternative is to change our document, to match the
actual practice and what the tools do?
Thanks,
Christophe
> Richard.
>
> > contrib/ChangeLog:
> >
> > * gcc-changelog/git_commit.py (subject_pr_regex): Rename into
> > subject_pr_component_regex.
> > (subject_pr_space_regex): New.
> > (subject_pr_paren_regex): New.
> > (subject_pr2_regex): Remove matching parentheses and rename into
> > subject_pr_regex.
> > (GitCommit): Add checks for new regexps.
> > ---
> > contrib/gcc-changelog/git_commit.py | 20 ++++++++++++--------
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/contrib/gcc-changelog/git_commit.py
> > b/contrib/gcc-changelog/git_commit.py
> > index 5c0596c2627..245c8496553 100755
> > --- a/contrib/gcc-changelog/git_commit.py
> > +++ b/contrib/gcc-changelog/git_commit.py
> > @@ -167,8 +167,10 @@ 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:?')
> > -subject_pr_regex =
> > re.compile(r'(^|\W)PR\s+(?P<component>[a-zA-Z0-9+-]+)/(?P<pr>\d{4,7})')
> > -subject_pr2_regex = re.compile(r'[(\[]PR\s*(?P<pr>\d{4,7})[)\]]')
> > +subject_pr_regex = re.compile(r'\[PR(?P<pr>\d{4,7})\]')
> > # [PRnnnn]
> > +subject_pr_space_regex = re.compile(r'\[PR\s+(?P<pr>\d{4,7})\]')
> > # [PR nnnn]
> > +subject_pr_paren_regex = re.compile(r'\(PR\s*(?P<pr>\d{4,7})\)')
> > # (PRnnnn) / (PR nnnn)
> > +subject_pr_component_regex =
> > re.compile(r'(^|\W)PR\s*(?P<component>[a-zA-Z0-9+-]+)/(?P<pr>\d{4,7})') #
> > PRcomponent/nnnn or PR component/nnnn
> > pr_regex = re.compile(r'\tPR (?P<component>[a-z0-9+-]+\/)?(?P<pr>[0-9]+)$')
> > dr_regex = re.compile(r'\tDR ([0-9]+)$')
> > star_prefix_regex = re.compile(r'\t\*(?P<spaces>\ *)(?P<content>.*)')
> > @@ -346,13 +348,15 @@ class GitCommit:
> > self.check_commit_email()
> >
> > # Extract PR numbers form the subject line
> > - # Match either [PRnnnn] / (PRnnnn) or PR component/nnnn
> > + # Reject [PR nnnn] / (PR nnnn) or PR component/nnnn
> > if self.info.lines and not self.revert_commit:
> > - self.subject_prs = {m.group('pr') for m in
> > subject_pr2_regex.finditer(info.lines[0])}
> > - for m in subject_pr_regex.finditer(info.lines[0]):
> > - if not m.group('component') in bug_components:
> > - self.errors.append(Error('invalid PR component in
> > subject', info.lines[0]))
> > - self.subject_prs.add(m.group('pr'))
> > + self.subject_prs = {m.group('pr') for m in
> > subject_pr_regex.finditer(info.lines[0])}
> > + for m in subject_pr_space_regex.finditer(info.lines[0]):
> > + self.errors.append(Error('Use [PRnnn] in subject, not [PR
> > nnn]', info.lines[0]))
> > + for m in subject_pr_paren_regex.finditer(info.lines[0]):
> > + self.errors.append(Error('Use [PRnnn] in subject, not
> > (PRnnn)', info.lines[0]))
> > + for m in subject_pr_component_regex.finditer(info.lines[0]):
> > + self.errors.append(Error('Do not use PR component in
> > subject', info.lines[0]))
> >
> > # Allow complete deletion of ChangeLog files in a commit
> > project_files = [f for f in self.info.modified_files
> > --
> > 2.34.1
> >
