Re: git gcc-commit-mklog doesn't extract PR number to ChangeLog

[Tobias Burnus <tobias@codesourcery.com>]

On 10.06.21 16:55, Martin Sebor via Gcc wrote:
> I'm a little lost as to what's being changed, and, truth be told,
> what exactly the current "right" format is.  Where are the PRnnnnn
> strings recognized as special?

For my version of the patch at least, which is
https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572483.html :

If the first line (subject line) contains '.... [PR12345]', that commit
is now rejected unless 'PR comp/12345' also in in the 'body'/changelog
part of the patch.

(This does not apply to your commit as you don't have such number.)

That avoids issues where one has added it to the subject line (which
usually matches also the email subject) but has forgotten to include it
also in GCC's patch format. That avoids a common pitfall of not having
the PR in the generated changelog files and the commit is not added as
Bugzilla comment.

That's orthogonal to questions like: how should commits be formatted in
general; do we want to extend the requirements, what exactly should be
written in the first line of the commit etc.

Regarding:

> The ChangeLog description doesn't seem to cover this and I've been
> assuming they're recognized anywhere in the ChangeLog message, but
> I think I also noticed they don't always end up updating all
> the bugs.
>
> FWIW, in commits for multiple PRs I've been adding a Resolves
> line like this:
>   https://gcc.gnu.org/pipermail/gcc-cvs/2021-May/347414.html

The current pattern matches something like '^\tPR component/12345$'. In
your commits, you have the Bugzilla title after the PR number, which is
currently not matched. — Your commit also includes the same numbers
without tailing strings, hence, for the linked commit, the script should
have extracted the PR number.

(One can argue whether the accepted format should/could be extended but
in the ChangeLog files, it also was just <tab>PR <comp>/<number><new line>.)

> I usually also add the PR numbers under each ChangeLog but I'm not
> sure it's necessary.

You can also have the PR lines before the first 'directory/' or
'directory/ChangeLog' line, then it applies to all ChangeLog files. —
Thus, you do not need to repeat it. (With the caveat of above.)

> It would be good to know and for the ChangeLog
> convention to document how exactly this works, and if something
> changes, to update the documentation.

I think the accepted changelog convention has not really changed. And
the changelog script mostly diagnosed issues which were also wrong
before – or are obviously wrong. In my experience, it finds a lot of
valid issues and rejects only very little which should be accepted.

There is an unofficial convention to use [PR1234] in the first line of
the commit (and the email subject), but that's not enforced by the script.

Independent of the PR matching and checking issues, I think Jonathan was
thinking about extending the documentation (as I gathered from IRC
#gcc); I did not quite follow whether it is about best practice or
contained bits which should be enforced.

Tobias


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