Messages in this thread Patches in this message | | | From | Vegard Nossum <> | Date | Wed, 11 Dec 2019 07:41:34 +0100 | Subject | Re: get_maintainer.pl produces non-deterministic results |
| |
On Wed, 11 Dec 2019 at 01:02, Joe Perches <joe@perches.com> wrote: > > On Tue, 2019-12-10 at 14:47 +0100, Dmitry Vyukov wrote: > > Hi Joe, > > > > scripts/get_maintainer.pl fs/proc/task_mmu.c > > non-deterministically gives me from 13 to 16 results, different number > > every time (on upstream 6794862a). Perl v5.28.1. Michael confirmed > > this with v5.28.2. > > Vergard suggested to check PERL_HASH_SEED=0. Indeed it fixes > > non-determinism. But I guess it's not the right solution, there should > > be some logical problem. > > My perl-fo is weak, I appreciate if somebody with proper perl-fo takes a look. > > > > Thanks > > https://lkml.org/lkml/2017/7/13/789
Right, so you can make it reproducible if you add a tie-break to the sorting:
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 34085d146fa2c..109d9fb134dad 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -2179,7 +2179,7 @@ sub vcs_assign { $hash{$_}++ for @lines;
# sort -rn - foreach my $line (sort {$hash{$b} <=> $hash{$a}} keys %hash) { + foreach my $line (sort {$hash{$b} <=> $hash{$a} || $a cmp $b} keys %hash) { my $sign_offs = $hash{$line}; my $percent = $sign_offs * 100 / $divisor;
This would actually favour names that start with early letters (A, B, ...) over late letters (..., Y, Z), which might also be a bad thing. I think to fix that you could include everybody who has the same number of signoffs at the cutoff: diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl index 34085d146fa2c..80d3ed2ee6d70 100755 --- a/scripts/get_maintainer.pl +++ b/scripts/get_maintainer.pl @@ -2179,7 +2179,8 @@ sub vcs_assign { $hash{$_}++ for @lines;
# sort -rn - foreach my $line (sort {$hash{$b} <=> $hash{$a}} keys %hash) { + my $prev_sign_offs = -1; + foreach my $line (sort {$hash{$b} <=> $hash{$a} || $a cmp $b} keys %hash) { my $sign_offs = $hash{$line}; my $percent = $sign_offs * 100 / $divisor;
@@ -2187,7 +2188,7 @@ sub vcs_assign { next if (ignore_email_address($line)); $count++; last if ($sign_offs < $email_git_min_signatures || - $count > $email_git_max_maintainers || + ($prev_sign_offs != $sign_offs && $count > $email_git_max_maintainers) || $percent < $email_git_min_percent); push_email_address($line, ''); if ($output_rolestats) { @@ -2196,6 +2197,8 @@ sub vcs_assign { } else { add_role($line, $role); } + + $prev_sign_offs = $sign_offs; } }
These patches are probably horribly whitespace damaged, hopefully you get the gist of it though...
Vegard
| |