Re: [PATCH 1/2] gitlog-to-changelog: support multi-author commits.

[Jim Meyering]

Gary V. Vaughan wrote:
> More generally useful gnulib-local goodness from my Libtool `next' branch:
>
> I'm sure this is far from idiomatic Perl, but I'd very much like for this
> patch or something similar to be pushed so that FSF projects have a means
> to correctly track multiple patch authors with a generated ChangeLog file.

I like the idea.

> The idea is to add bare 'Co-authored-by: ' lines to the git log message,

You mentioned consensus.
What other projects already use that syntax?

> which gitlog-to-changelog adds to the ChangeLog output in the usual format.
> Libtool at least, and I believe other FSF projects have the concept of a
> main author (the one on the date line) and additional authors (following)
> which gels well with keeping the coauthors in a separate array, and only
> suppressing consecutive headers blocks (date-line + coauthors) when the
> main author, date, and co-authors all match.  That is, if the same group
> of people apply consecutive patches on the same day, but with a different
> "main author" each time, then a full header block is output each time.
>
> Okay to push?
>
> The FSF cares about keeping track of all authors of patches to its
> projects, but Git doesn't provide obvious support for multi-author
> changesets. Consensus seems to be forming around the use of extra
> Signed-off-by inspired lines in the log message formatted as
> `Co-authored-by: A U Thor <address@hidden>' for round-tripping
> multi-author commits between version control systems.
> * gitlog-to-changelog: Extract `Co-authored-by:' lines from the git
> log message and output in standard ChangeLog multi-author format.
> Reported by Peter Rosin <address@hidden>
...
> @@ -124,6 +124,7 @@ sub quoted_cmd(@)
>              . "(Is your Git too old?  Version 1.5.1 or later is 
> required.)\n");
>
>    my $prev_date_line = '';
> +  my @prev_coauthors = ();

Please drop the initializer.

>    while (1)
>      {
>        defined (my $in = <PIPE>)
> @@ -146,18 +147,28 @@ sub quoted_cmd(@)
>            . "(expected date/author/email):\n$author_line\n";
>
>        my $date_line = sprintf "%s  $2\n", strftime ("%F", localtime ($1));
> -      # If this line would be the same as the previous date/name/email
> -      # line, then arrange not to print it.
> -      if ($date_line ne $prev_date_line)
> +
> +      # Format 'Co-authored-by: A U Thor <address@hidden>' lines in
> +      # standard multi-author ChangeLog format.
> +      my @coauthors = grep /^Co-authored-by:.*>$/, @line;

Here you require only the trailing ">", yet below, the
transformation happens only if there is also a "<".
This is different from Signed-off-by: in that if we ignore a
line because it lacks an email address, then that probably
deserves a diagnostic.

Any annotation that we standardize like this should be
syntax-checked via a git log hook like the one I recently
added to coreutils (see scripts/git-hooks).

An alternative is to accept anything after the ":" and then, to use
"s/\s*</  </" to ensure that the number of spaces before the "<" is two.

> +      s/^Co-authored-by:\s*([^<]+)\s+</\t    \1  </

Please use $1, not \1.

> +        for (@coauthors);

> +
> +      # If this header would be the same as the previous date/name/email/
> +      # coauthors header, then arrange not to print it.
> +      if ($date_line ne $prev_date_line or "@coauthors" ne "@prev_coauthors")

s/or/||/

>          {
>            $prev_date_line eq ''
>              or print "\n";
>            print $date_line;
> +          print join ("\n", @coauthors), "\n"
> +            unless (@coauthors == 0);

Please write that so that the conditional part is indented:

             @coauthors
               and print join ("\n", @coauthors), "\n"

>          }
>        $prev_date_line = $date_line;
> +      @prev_coauthors = @coauthors;
>
> -      # Omit "Signed-off-by..." lines.
> -      @line = grep !/^Signed-off-by: .*>$/, @line;
> +      # Omit "Co-authored-by..." and "Signed-off-by..." lines.
> +      @line = grep !/^(Co-authored|Signed-off)-by: .*>$/, @line;
>
>        # Remove leading and trailing blank lines.
>        while ($line[0] =~ /^\s*$/) { shift @line; }
> --
> 1.7.7.1
>
> Cheers,