git gcc-commit-mklog doesn't extract PR number to ChangeLog
Jason Merrill
jason@redhat.com
Thu Jun 17 01:46:17 GMT 2021
On 6/16/21 9:01 PM, Martin Sebor wrote:
> On 6/16/21 6:40 PM, Jason Merrill wrote:
>> On 6/16/21 8:17 PM, Martin Sebor wrote:
>>> On 6/16/21 5:45 PM, Jason Merrill wrote:
>>>> On Wed, Jun 16, 2021 at 5:46 PM Martin Sebor <msebor@gmail.com
>>>> <mailto: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>
>>>> >> <mailto: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.
>>>
>>> To say I'm not good at Python would be an understatement but I hacked
>>> up the attached patch that:
>>>
>>> 1) extracts PR numbers from test names,
>>> 2) gets the component for each PR from Bugzilla,
>>
>> That seems useful for testcases like the OP's that put the PR number
>> in the filename rather than a comment. Maybe submit it as a patch?
>
> Will do.
>
>>> 3) adds the PR component/nnnnn to each ChangeLog
>>
>> This would be reverting the r12-771 change, which seems both unrelated
>> and undesirable.
>
> Now I'm confused. Isn't that just what caused the problem to begin
> with? (The bug not being updated with the commit because it's not
> in the ChangeLog entries?)
The PR number wasn't in the ChangeLog entries because mklog didn't know
the number, because it wasn't in a comment in the testcase; your #1 and
#2 should fix that.
The bugzilla integration works fine with the current output of mklog
when it can find the PR number.
Jason
More information about the Gcc
mailing list