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
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
On Wed, 11 Dec 2019 at 01:02, Joe Perches <[email protected]> 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
On Wed, 2019-12-11 at 07:41 +0100, Vegard Nossum wrote:
> On Wed, 11 Dec 2019 at 01:02, Joe Perches <[email protected]> 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...
I get the gist, but I think it's also not particularly important
to be repeatable. I think it's more important to get more pattern
coverage of the MAINTAINERS file as that is more important than
any use of git history.