Re: [PATCH] maint: sort contributors in THANKS.in

[Bernhard Voelker]

On 02/08/2015 11:17 PM, Pádraig Brady wrote:
> On 08/02/15 19:40, Bernhard Voelker wrote:
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -172,6 +172,21 @@ THANKS: THANKS.in Makefile.am .mailmap thanks-gen 
>> .version
>>        printf ';; %s\n' 'Local Variables:' 'coding: utf-8' End:;     \
>>      } > $@-t && chmod a-w $@-t && mv $@-t $@
>>  
>> +# Sort the contributor name/email pair list in 'THANKS.in'.
>> +# See sc_THANKS_in_sorted ... which uses the same sort options.
>> +.PHONY: sort-THANKS.in
>> +sort-THANKS.in: Makefile.am

> This separate target is probably overkill,
> as THANKS.in should remain sorted due to the sc_ rule.

okay, removed.

>> diff --git a/cfg.mk b/cfg.mk
>> index f5be6de..26df247 100644
>> --- a/cfg.mk
>> +++ b/cfg.mk
>> @@ -611,6 +611,17 @@ sc_THANKS_in_duplicates:
>>          && { echo '$(ME): remove the above names from THANKS.in'    \
>>                1>&2; exit 1; } || :
>>  
>> +# Ensure the contributor list stays sorted.  The sort order can be fixed
>> +# with 'make sort-THANKS.in' ... which uses the same sort options.
>> +sc_THANKS_in_sorted:
>> +    @sed '/^$$/,/^$$/!d;/^$$/d' THANKS.in > $@.1;                   \
>> +    LC_ALL='en_US.UTF-8' sort -f -k1,1 $@.1 > $@.2
>> +    @diff -u $@.1 $@.2; diff=$$?;                                   \
>> +    rm -f $@.1 $@.2;                                                \
>> +    test "$$diff" = 0                                               \
>> +      || { echo '$(ME): THANKS.in is unsorted; please run'          \
>> +             'make sort-THANKS.in' 1>&2; exit 1; }
>> +
> 
> I don't like the sorting in this locale at all.
> Actually the locale is OK, just the sorting in the patch seems
> to have been done in LC_ALL=C ?
> 
>   $ printf '%s\n' Duncan Dániel Gaël Gary Ørn Oskar | LC_ALL=en_US.utf8 sort
>   Dániel
>   Duncan
>   Gaël
>   Gary
>   Ørn
>   Oskar

Outch, it seems we hit a bug in the downstream I18N implementation when
both -f and -k1,1 are in effect.  This came in when I added -k1,1 late.

  $ printf '%s\n' Duncan Dániel Gaël Gary Ørn Oskar | LC_ALL=en_US.utf8 
/usr/bin/sort -f -k1,1
  Duncan
  Dániel
  Gary
  Gaël
  Oskar
  Ørn

  $ printf '%s\n' Duncan Dániel Gaël Gary Ørn Oskar | LC_ALL=en_US.utf8 
src/sort -f -k1,1
  Dániel
  Duncan
  Gaël
  Gary
  Ørn
  Oskar

AFAIK downstream sort on [your] Fedora shares the bug with that on [my] 
openSUSE?
The sc_ rule now uses our own sort to avoid issues.

> That locale doesn't work correctly with the only cyrillic entry though:
> 
>   $ printf '%s\n' Марк Michal | LC_ALL=en_US.utf8 sort
>   Michal
>   Марк
> 
> I'm thinking we should convert that to latin1 (Mark Korenberg) like all other 
> entries.

done.

Thanks for the review.
Latest patch attached.

Have a nice day,
Berny

0001-maint-sort-contributors-in-THANKS.in.patch
Description: Text Data