This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Commit messages and the move to git


On 18/11/2019 20:53, Jason Merrill wrote:
> On Mon, Nov 18, 2019 at 2:51 PM Segher Boessenkool <
> segher@kernel.crashing.org> wrote:
> 
>> On Mon, Nov 18, 2019 at 07:21:22PM +0000, Richard Earnshaw (lists) wrote:
>>> On 18/11/2019 18:53, Segher Boessenkool wrote:
>>>> PR target/92140: clang vs gcc optimizing with adc/sbb
>>>> PR fortran/91926: assumed rank optional
>>>> PR tree-optimization/91532: [SVE] Redundant predicated store in
>> gcc.target/aarch64/fmla_2.c
>>>> PR tree-optimization/92161: ICE in vect_get_vec_def_for_stmt_copy, at
>> tree-vect-stmts.c:1687
>>>> PR tree-optimization/92162: ICE in vect_create_epilog_for_reduction,
>> at tree-vect-loop.c:4252
>>>> PR c++/92015: internal compiler error: in cxx_eval_array_reference, at
>> cp/constexpr.c:2568
>>>> PR tree-optimization/92173: ICE in optab_for_tree_code, at
>> optabs-tree.c:81
>>>> PR tree-optimization/92173: ICE in optab_for_tree_code, at
>> optabs-tree.c:81
>>>> PR fortran/92174: runtime error: index 15 out of bounds for type
>> 'gfc_expr *[15]
>>>>
>>>> Most of these aren't helpful at all, and none of these are good commit
>>>> summaries.  The PR92173 one actually has identical commit messages btw,
>>>> huh.  Ah, the second one (r277288) has the wrong changelog, but in the
>>>> actual changelog file as well, not something any tool could fix up (or
>>>> have we reached the singularity?)
>>>
>>> Identical commits are normally from where the same commit is made to
>>> multiple branches.  It's not uncommon to see this when bugs are fixed.
>>
>> This is an actual mistake.  The commits are not identical at all, just
>> the commit messages are (and the changelog entries, too).  Not something
>> that happens to ften, but of course I hit it in the first random thing I
>> pick :-)
>>
>>> Ultimately the question here is whether something like the above is more
>>> or less useful than what we have today, which is summary lines of the
>> form:
>>>
>>> <date> <user> <email>
>>
>> I already said I would prefer things like
>>   Patch related to PR323
>> as the patch subject lines.  No one argues that the current state of
>> affairs is good.  I argue that replacing this with often wrong and
>> irrelevant information isn't the best we can do.
>>
> 
> How about using the first line that isn't a ChangeLog date/author line,
> without trying to rewrite/augment it?
> 
> Jason
> 

It would certainly be another way of doing it.  Sometimes it would
produce almost the same as an unadulterated PR; sometimes it would
produce something more meaningful and sometimes it would be pretty
useless.  It probably would hit more cases than my current script in
that it wouldn't require the commit to mention a PR in it.

The main problem is that the first line is often incomplete, and much of
it is also wasted with elements like the full path to a file that is
quite deep in the tree.  Lets take a quick example (the first I found in
the dump I have).

1998-12-17  Vladimir N. Makarov  <vmakarov@cygnus.com>
        * config/i60/i960.md (extendqihi2): Fix typo (usage ',' instead of
        ';').
1998-12-17  Michael Tiemann  <tiemann@axon.cygnus.com>
        * i960.md (extend*, zero_extend*): Don't generate rtl that looks
        like (subreg:SI (reg:SI N) 0), because it's wrong, and it hides
        optimizations from the combiner.

Firstly, this example misses a blank line between the author and the
change message itself, which makes distinguishing between this and the
multiple authors case harder.  Secondly, the entry really spans two
lines and cutting it off at the end of the first line would be, well a
bit odd.  We could try to piece things together more, by combining lines
until we find a sentence end ( \.$ or \.\s\s ), and we could also strip
off more of the path components to just leave the name of the file
committed.  For example,

i960.md (extendqihi2): Fix typo (usage ',' instead of ';').

That might work better, but obviously it's harder to handle and thus
more likely to create erroneous summaries.

In fact, the above currently just results in a summary of

[multiple commits]

because there are two authors mentioned with date stamps.

An advantage of this approach, of course, is that it would also work for
pre-egcs commits, to the extent that they have any useful commit message
at all.

The issue with bogus PR numbers will remain, of course, but that's no
worse than it is today.

R.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]