[Suggestion] about latest commit for "scripts/get_maintainers.pl"

[Chen Gang]

Hello Joe:

I meet a failure about "scripts/get_maintainers.pl", it is about the
commit "750432d get_maintainer.pl incomplete output", if use original
"scripts/get_maintainer.pl", it will be OK.

Please help check, thanks.


The related information:

-------------------------information begin---------------------------

[root@gchenlinux linux-next]# ./scripts/get_maintainer.pl /tmp/0001-hexagon-kernel-remove-useless-variables-dn-r-and-err.patch
Can't use string ("GitAuthor: Chen Gang <gang.chen@"...) as an ARRAY ref while "strict refs" in use at ./scripts/get_maintainer.pl line 1883.

[root@gchenlinux linux-next]# cat /tmp/0001-hexagon-kernel-remove-useless-variables-dn-r-and-err.patch
From ef7384078bacdc5151039ea710943e5710d7c5ed Mon Sep 17 00:00:00 2001
From: Chen Gang <gang...@asianux.com>
Date: Fri, 1 Nov 2013 18:58:18 +0800
Subject: [PATCH] hexagon: kernel: remove useless variables 'dn', 'r' and 'err' in time_init_deferred() in "time.c"

Remove them, since they are useless. The related warnings (with allmodconfig for v4):

CC arch/hexagon/kernel/time.o
arch/hexagon/kernel/time.c: In function 'time_init_deferred':
arch/hexagon/kernel/time.c:196: warning: unused variable 'err'
arch/hexagon/kernel/time.c:195: warning: unused variable 'r'
arch/hexagon/kernel/time.c:194: warning: unused variable 'dn'


Signed-off-by: Chen Gang <gang...@asianux.com>
---
arch/hexagon/kernel/time.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/hexagon/kernel/time.c b/arch/hexagon/kernel/time.c
index 9903fad..d0c4f5a 100644
--- a/arch/hexagon/kernel/time.c
+++ b/arch/hexagon/kernel/time.c
@@ -191,9 +191,6 @@ void __init time_init_deferred(void)
{
struct resource *resource = NULL;
struct clock_event_device *ce_dev = &hexagon_clockevent_dev;
- struct device_node *dn;
- struct resource r;
- int err;

ce_dev->cpumask = cpu_all_mask;

--
1.7.7.6

-------------------------information end-----------------------------


Thanks.
--
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Joe Perches]

On Fri, 2013-11-01 at 19:20 +0800, Chen Gang wrote:
> Hello Joe:

Hello Chen Gang.

> I meet a failure about "scripts/get_maintainers.pl", it is about the
> commit "750432d get_maintainer.pl incomplete output", if use original
> "scripts/get_maintainer.pl", it will be OK.
>
> Please help check, thanks.

I don't get that effect.
I'll look into it and see what I find next week.
You seem to have a work-around available.

Joe

[Chen Gang F T]

On 11/02/2013 02:32 AM, Joe Perches wrote:
> On Fri, 2013-11-01 at 19:20 +0800, Chen Gang wrote:
>> > Hello Joe:
> Hello Chen Gang.
>
>> > I meet a failure about "scripts/get_maintainers.pl", it is about the
>> > commit "750432d get_maintainer.pl incomplete output", if use original
>> > "scripts/get_maintainer.pl", it will be OK.
>> >
>> > Please help check, thanks.
> I don't get that effect.

For me now, I made 3 patches for hexagon, 2 of them can cause issue. I
use sfr next-20131101 tree, the related patch is in attachment (1, 3 can
cause issue), hope they are useful for us.

> I'll look into it and see what I find next week.

Thank you very much, just please help analyze it when you have time.

> You seem to have a work-around available.

Yes, at least now, it doesn't matter to me. I guess this issue is not
urgent to us.


Thanks.
--
Chen Gang

[Joe Perches]

Add this to "try this"...

Chen Gang's defect is because his git repository branch
had a commit he authored but where did not add his signature.

Also, there's a defect in function vcs_find_signers.
It should only return the commit count and array references.

If there are no signers in the commit history interval specified,
then the "authors" array is added to the "signers" array and
returned as a single array.

Use references instead.

Make sure that references are defined in the places
that vcs_find_signers is called.

Signed-off-by: Joe Perches <j...@perches.com>
---
scripts/get_maintainer.pl | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index ee9adb8..9c3986f 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -128,7 +128,7 @@ my %VCS_cmds_git = (
"blame_commit_pattern" => "^([0-9a-f]+) ",
"author_pattern" => "^GitAuthor: (.*)",
"subject_pattern" => "^GitSubject: (.*)",
- "stat_pattern" => "(\\d+)\\t(\\d+)\\t\$file",
+ "stat_pattern" => "^(\\d+)\\t(\\d+)\\t\$file\$",
);

my %VCS_cmds_hg = (
@@ -156,7 +156,7 @@ my %VCS_cmds_hg = (
"blame_commit_pattern" => "^([ 0-9a-f]+):",
"author_pattern" => "^HgAuthor: (.*)",
"subject_pattern" => "^HgSubject: (.*)",
- "stat_pattern" => "(\\d+)\t(\\d+)\t\$file",
+ "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
);

my $conf = which_conf(".get_maintainer.conf");
@@ -1297,7 +1297,7 @@ sub vcs_find_signers {

# print("stats: <@stats>\n");

- return (0, @signatures, @authors) if !@signatures;
+ return (0, \@signatures, \@authors, \@stats) if !@signatures;

save_commits_by_author(@lines) if ($interactive);
save_commits_by_signer(@lines) if ($interactive);
@@ -1880,9 +1880,10 @@ sub vcs_file_signoffs {
$cmd =~ s/(\$\w+)/$1/eeg; # interpolate $cmd

($commits, $signers_ref, $authors_ref, $stats_ref) = vcs_find_signers($cmd, $file);
- @signers = @{$signers_ref};
- @authors = @{$authors_ref};
- @stats = @{$stats_ref};
+
+ @signers = @{$signers_ref} if defined $signers_ref;
+ @authors = @{$authors_ref} if defined $authors_ref;
+ @stats = @{$stats_ref} if defined $stats_ref;

# print("commits: <$commits>\nsigners:<@signers>\nauthors: <@authors>\nstats: <@stats>\n");

@@ -1965,8 +1966,8 @@ sub vcs_file_blame {
$cmd =~ s/(\$\w+)/$1/eeg; #substitute variables in $cmd

($commit_count, $commit_signers_ref, $commit_authors_ref, $stats_ref) = vcs_find_signers($cmd, $file);
- @commit_authors = @{$commit_authors_ref};
- @commit_signers = @{$commit_signers_ref};
+ @commit_authors = @{$commit_authors_ref} if defined $commit_authors_ref;
+ @commit_signers = @{$commit_signers_ref} if defined $commit_signers_ref;

push(@signers, @commit_signers);
} else {
@@ -1983,8 +1984,8 @@ sub vcs_file_blame {
$cmd =~ s/(\$\w+)/$1/eeg; #substitute variables in $cmd

($commit_count, $commit_signers_ref, $commit_authors_ref, $stats_ref) = vcs_find_signers($cmd, $file);
- @commit_authors = @{$commit_authors_ref};
- @commit_signers = @{$commit_signers_ref};
+ @commit_authors = @{$commit_authors_ref} if defined $commit_authors_ref;
+ @commit_signers = @{$commit_signers_ref} if defined $commit_signers_ref;

push(@signers, @commit_signers);

[Chen Gang]

On 11/05/2013 05:54 AM, Joe Perches wrote:
> Add this to "try this"...
>
> Chen Gang's defect is because his git repository branch
> had a commit he authored but where did not add his signature.
>

Hmm... for pure next-20131101 tree in my git directory and the demo
patches in attachment, it will cause this issue.

And excuse me, I am not quite familiar with git, could you provide more
details (e.g. where the special difference of the demo patches or the
pure next-20131101 tree in my git directory)? thanks.

> Also, there's a defect in function vcs_find_signers.
> It should only return the commit count and array references.
>
> If there are no signers in the commit history interval specified,
> then the "authors" array is added to the "signers" array and
> returned as a single array.
>
> Use references instead.
>
> Make sure that references are defined in the places
> that vcs_find_signers is called.
>

After apply the patch below, it can work well.

Tested-by Chen Gang <gang...@asianux.com>

Chen Gang

[Chen Gang]

On 11/05/2013 10:22 AM, Chen Gang wrote:
> On 11/05/2013 05:54 AM, Joe Perches wrote:
>> Add this to "try this"...
>>
>> Chen Gang's defect is because his git repository branch
>> had a commit he authored but where did not add his signature.
>>

Hmm... in fact, when I make a patch, I really let "git log" contents a
commit I author but where do not add my signature.

But after I let it pure next-20131101 ("git reset HEAD^; git stash"), it
still cause this issue.

[Joe Perches]

On Tue, 2013-11-05 at 10:37 +0800, Chen Gang wrote:
> But after I let it pure next-20131101 ("git reset HEAD^; git stash"), it
> still cause this issue.

It's also because:

> >> Also, there's a defect in function vcs_find_signers.
> >> It should only return the commit count and array references.
> >>
> >> If there are no signers in the commit history interval specified,
> >> then the "authors" array is added to the "signers" array and
> >> returned as a single array.

[Chen Gang]

On 11/05/2013 01:23 PM, Joe Perches wrote:
> On Tue, 2013-11-05 at 10:37 +0800, Chen Gang wrote:
>> But after I let it pure next-20131101 ("git reset HEAD^; git stash"), it
>> still cause this issue.
>
> It's also because:
>
>>>> Also, there's a defect in function vcs_find_signers.
>>>> It should only return the commit count and array references.
>>>>
>>>> If there are no signers in the commit history interval specified,
>>>> then the "authors" array is added to the "signers" array and
>>>> returned as a single array.
>
>
>
>

OK, thanks. Do you mean: in pure next-20131101 contents an author which
commit a patch, but not signed himself? (if so, it seems the related
patch need be re-committed again by the related author).


Thanks.
--
Chen Gang

[Chen Gang]

On 11/05/2013 01:42 PM, Chen Gang wrote:
> On 11/05/2013 01:23 PM, Joe Perches wrote:
>> On Tue, 2013-11-05 at 10:37 +0800, Chen Gang wrote:
>>> But after I let it pure next-20131101 ("git reset HEAD^; git stash"), it
>>> still cause this issue.
>>
>> It's also because:
>>
>>>>> Also, there's a defect in function vcs_find_signers.
>>>>> It should only return the commit count and array references.
>>>>>
>>>>> If there are no signers in the commit history interval specified,
>>>>> then the "authors" array is added to the "signers" array and
>>>>> returned as a single array.
>>
>>
>>
>>
>
> OK, thanks. Do you mean: in pure next-20131101 contents an author which
> commit a patch, but not signed himself? (if so, it seems the related
> patch need be re-committed again by the related author).
>

If really as what I guess above (some patches no Signed-of-by), is there
a tool to check and find this issue in time?

[Joe Perches]

On Tue, 2013-11-05 at 13:45 +0800, Chen Gang wrote:
> If really as what I guess above (some patches no Signed-of-by), is there
> a tool to check and find this issue in time?

scripts/checkpatch.pl bleats a message on missing sign-offs.

For instance:

$ cat cache.diff
include/linux/cache.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/cache.h b/include/linux/cache.h
index 4c57065..17e7e82 100644
--- a/include/linux/cache.h
+++ b/include/linux/cache.h
@@ -1,11 +1,11 @@
#ifndef __LINUX_CACHE_H
#define __LINUX_CACHE_H

-#include <linux/kernel.h>
+#include <uapi/linux/kernel.h>
#include <asm/cache.h>

#ifndef L1_CACHE_ALIGN
-#define L1_CACHE_ALIGN(x) ALIGN(x, L1_CACHE_BYTES)
+#define L1_CACHE_ALIGN(x) __ALIGN_KERNEL(x, L1_CACHE_BYTES)
#endif

#ifndef SMP_CACHE_BYTES

$ ./scripts/checkpatch.pl cache.diff
ERROR: Missing Signed-off-by: line(s)

total: 1 errors, 0 warnings, 13 lines checked

cache.diff has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

[Joe Perches]

On Tue, 2013-11-05 at 13:42 +0800, Chen Gang wrote:
> On 11/05/2013 01:23 PM, Joe Perches wrote:
> > On Tue, 2013-11-05 at 10:37 +0800, Chen Gang wrote:
> >> But after I let it pure next-20131101 ("git reset HEAD^; git stash"), it
> >> still cause this issue.
> >
> > It's also because:
> >
> >>>> Also, there's a defect in function vcs_find_signers.
> >>>> It should only return the commit count and array references.
> >>>>
> >>>> If there are no signers in the commit history interval specified,
> >>>> then the "authors" array is added to the "signers" array and
> >>>> returned as a single array.
>
> OK, thanks. Do you mean: in pure next-20131101 contents an author which
> commit a patch, but not signed himself?

Yes, I mean that.

> (if so, it seems the related
> patch need be re-committed again by the related author).

No it doesn't mean that.

Prior to this patch, get_maintainer only used "sign-off-by"
style lines to find interested parties to send patches to
when the MAINTAINERS file did not have a specific section
entry with a matching file pattern to the files that that
the patch modified.

And there was a defect in the script when no commit history
at all was found for the files modified during the period
selected (1 year of git log history by default)

Now, using this newly modified get_maintainer.pl script,
the commit authors shown in the git log history as well as
the commit signers are selected.

If there is no git history, the script defect is also fixed.

[Chen Gang]

On 11/05/2013 01:54 PM, Joe Perches wrote:
> On Tue, 2013-11-05 at 13:45 +0800, Chen Gang wrote:
>> > If really as what I guess above (some patches no Signed-of-by), is there
>> > a tool to check and find this issue in time?
> scripts/checkpatch.pl bleats a message on missing sign-offs.

Yeah, the author/maintainer can use it for checking.

But do we have additional tools to let the version tree integrator (e.g.
sfr) notice about it?


Thanks.
--
Chen Gang

[Chen Gang]

On 11/05/2013 01:50 PM, Joe Perches wrote:
> On Tue, 2013-11-05 at 13:42 +0800, Chen Gang wrote:
>> On 11/05/2013 01:23 PM, Joe Perches wrote:
>>> On Tue, 2013-11-05 at 10:37 +0800, Chen Gang wrote:
>
>> (if so, it seems the related
>> patch need be re-committed again by the related author).
>
> No it doesn't mean that.
>
> Prior to this patch, get_maintainer only used "sign-off-by"
> style lines to find interested parties to send patches to
> when the MAINTAINERS file did not have a specific section
> entry with a matching file pattern to the files that that
> the patch modified.
>
> And there was a defect in the script when no commit history
> at all was found for the files modified during the period
> selected (1 year of git log history by default)
>
> Now, using this newly modified get_maintainer.pl script,
> the commit authors shown in the git log history as well as
> the commit signers are selected.
>
> If there is no git history, the script defect is also fixed.
>

Yeah, our "get_maintainer.pl" need be smart enough to bear it (this
patch let our "get_maintainer.pl" smart enough).

But for this kind of patch, need it re-commit again? (for me, I
recommend to re-commit it again by the related author).


Thanks.
--
Chen Gang

[Joe Perches]

On Tue, 2013-11-05 at 14:04 +0800, Chen Gang wrote:
> On 11/05/2013 01:54 PM, Joe Perches wrote:
> > On Tue, 2013-11-05 at 13:45 +0800, Chen Gang wrote:
> >> > If really as what I guess above (some patches no Signed-of-by), is there
> >> > a tool to check and find this issue in time?
> > scripts/checkpatch.pl bleats a message on missing sign-offs.
>
> Yeah, the author/maintainer can use it for checking.
>
> But do we have additional tools to let the version tree integrator (e.g.
> sfr) notice about it?

git pre-commit hooks work.

btw: there's some development efforts going on
that might help here eventually.

Read this thread:
https://lkml.org/lkml/2013/10/26/158

[Chen Gang]

On 11/05/2013 02:16 PM, Joe Perches wrote:
> On Tue, 2013-11-05 at 14:04 +0800, Chen Gang wrote:
>> On 11/05/2013 01:54 PM, Joe Perches wrote:
>>> On Tue, 2013-11-05 at 13:45 +0800, Chen Gang wrote:
>>>>> If really as what I guess above (some patches no Signed-of-by), is there
>>>>> a tool to check and find this issue in time?
>>> scripts/checkpatch.pl bleats a message on missing sign-offs.
>>
>> Yeah, the author/maintainer can use it for checking.
>>
>> But do we have additional tools to let the version tree integrator (e.g.
>> sfr) notice about it?
>
> git pre-commit hooks work.
>
> btw: there's some development efforts going on
> that might help here eventually.
>
> Read this thread:
> https://lkml.org/lkml/2013/10/26/158
>

OK, thanks. It seems it is about commit one patch, and it also seems no
"git pre-commit" command (my git version is "git version 1.7.7.6").

And do we have a tool to type a command to check all patches whether
missing sign-offs? (I guess, the version integrator need not check patch
one by one only for whether contents sign-offs).

Oh, I forgot one thing, the original "scripts/get_maintainers.pl" (e.g.
in next-20130927), it is already smart enough. So can we treat our patch
as the fix patch for the latest changes?


Thanks.
--
Chen Gang