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

[Jason Merrill]

On Wed, Jun 16, 2021 at 5:46 PM Martin Sebor <msebor@gmail.com> wrote:

> On 6/16/21 2:49 PM, Jason Merrill wrote:
> > On 6/15/21 11:42 PM, Jason Merrill wrote:
> >> On Tue, Jun 15, 2021 at 10:04 PM Martin Sebor via Gcc <gcc@gcc.gnu.org
> >> <mailto:gcc@gcc.gnu.org>> wrote:
> >>
> >>     On 6/15/21 6:56 PM, Hans-Peter Nilsson wrote:
> >>      > On Fri, 11 Jun 2021, Martin Sebor via Gcc wrote:
> >>      >
> >>      >> On 6/11/21 11:32 AM, Jonathan Wakely wrote:
> >>      >>> On Fri, 11 Jun 2021 at 18:02, Martin Sebor wrote:
> >>      >>>> My objection is to making our policies and tools more
> >> restrictive
> >>      >>>> than they need to be.  We shouldn't expect everyone to study
> >> whole
> >>      >>>> manuals just to figure out how to successfully commit a
> >> change (or
> >>      >>>> learn how to format it just the right way).  It should be
> easy.
> >>      >>>
> >>      >>> I agree, to some extent. But consistency is also good. The
> >>     conventions
> >>      >>> for GNU ChangeLog formatting exist for a reason, and so do the
> >>      >>> conventions for good Git commit messages.
> >>      >>>
> >>      >>>> Setting this discussion aside for a moment and using a
> >> different
> >>      >>>> example, the commit hook rejects commit messages that don't
> >> start
> >>      >>>> ChangeLog entries with tabs.  It also rejects commit
> >> messages that
> >>      >>>> don't list all the same test files as those changed by the
> >> commit
> >>      >>>> (and probably some others as well).  That's in my view
> >> unnecessary
> >>      >>>> when the hook could just replace the leading spaces with
> >> tabs and
> >>      >>>> automatically mention all the tests.
> >>      >>>>
> >>      >>>> I see this proposal as heading in the same direction.
> >> Rather than
> >>      >>>> making the script fix things up if we get them wrong it would
> >>     reject
> >>      >>>> the commit, requiring the user to massage the ChangeLog by
> >>     hand into
> >>      >>>> an unnecessarily rigid format.
> >>      >>>
> >>      >>> You cannot "fix things up" in a server-side receive hook,
> >> because
> >>      >>> changing the commit message would alter the commit hash, which
> >>     would
> >>      >>> require the committer to do a rebase to proceed. That breaks
> the
> >>      >>> expected behaviour and workflow of a git repo.
> >>      >>>
> >>      >>> You can use the scripts on the client side to verify your
> commit
> >>      >>> message before pushing, so you don't have to be surprised
> >> when the
> >>      >>> server rejects it.
> >>      >>
> >>      >> That sounds like a killer argument.  Do we have shared
> >> client-side
> >>      >> scripts that could fix things up for us, or are we each on our
> >> own
> >>      >> to write them?
> >>      >
> >>      > I hope I got your view wrong.  If not: the "scripts fixing
> >>      > things up for us" direction is flawed (compared to the "scripts
> >>      > rejecting bad formats"), unless offered as a non-default option;
> >>      > please don't proceed.
> >>      >
> >>      > Why?  For one, there'll always be bugs in the scripting.
> >>      > Mitigate those situations: while wrongly rejecting a commit is
> >>      > bad, wrongly "fixing things up" is worse, as a general rule.
> >>      > Better avoid that.  (There's probably a popular "pattern name"
> >>      > for what I try to describe.)
> >>
> >>     The word that comes to mind is Technophobia.  Is it wise to trust
> >>     compilers to transform programs from their source form into
> >>     executables?  What if there are bugs in either?  What about the OS?
> >>     The whole computer, or the Internet?  Our cars?  Fortunately,
> there's
> >>     more to gain than to lose by trusting automation.  If there weren't
> >>     human progress would be stuck sometime in the 1700's.
> >>
> >>     But we're not talking about anything anywhere that sophisticated
> >>     here: a sed script to copy and paste a piece of text in
> >>     the description of a change from one place to another.  It's been
> >>     done a few times before with more important data than ChangeLogs.
> >>
> >>
> >> git gcc-commit-mklog already automates most of the process.  It could
> >> also automate adding [PRxxxxx] to the first line.  Is that what you're
> >> asking for?
> >
> > Like, say:
>
> I don't think this solves the problem Xionghu Luo was asking about:
>    https://gcc.gnu.org/pipermail/gcc/2021-June/236346.html


Indeed, their problem was not mentioning the PR in the testcase, which a
script isn't going to fix.

i.e., they did have a [PRnnnn] in the one line subject but not in
> their ChangeLog entries.  It also not clear if they used mklog.py
> at all.  IME, mklog.py already puts in a [PRnnnn] near the top of
> a patch if it finds in one of the tests.  Though it doesn't seem
> to put in the ChangeLog entries.  Odd.
>

It currently puts in

 PR comp/nnnnn

at the beginning of the ChangeLog entries; it used to put them in the
entries for each ChangeLog file, but that changed in r12-771.  My patch
also adds the [PRnnnn] to the subject line.

I was suggesting to make this (and the other things the commit
> hook rejects commit for) happen automatically by running a script
> at the same time as git commit.
>
> But to be clear, I'm not asking for anything for myself.  Although
> I use mklog.py I have my own script that does what I suggest that
> I could go back to.  I responded to this thread because I think
> these minute details could be automated for everyone's benefit.
> Before moving to Git we talked about doing much more, including
> automatically running a format/style checker on the patch and
> (IIRC) Jeff even wanted it to do some basic tweaks on its own
> (like replace spaces with tabs).
>

We could do various cleanup in the 'commit-msg' hook on the user side.  Or,
probably better, git gcc-verify could clean up some of the issues it finds.

Jason