Checkpatch expects entries in MAINTAINERS file in a specific order and
warns if the changes made do not follow the specified order.
E.g., running checkpatch on commit b33bc2b878e0 ("nexthop: Add entry to
MAINTAINERS") reports this warning:
WARNING: Misordered MAINTAINERS entry - list file patterns in
alphabetic order
+F: include/uapi/linux/nexthop.h
+F: include/net/netns/nexthop.h
Provide a simple fix by swapping the unordered lines, if both the lines
are additions (start with '+')
Signed-off-by: Aditya Srivastava <[email protected]>
---
Changes in v2:
modified commit message
Changes in v3:
add check if both the lines are additions(ie start with '+')
scripts/checkpatch.pl | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2749f32dffe9..7ee3f05c354d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3299,13 +3299,22 @@ sub process {
"Unknown MAINTAINERS entry type: '$cur'\n" . $herecurr);
} else {
if ($previndex >= 0 && $curindex < $previndex) {
- WARN("MAINTAINERS_STYLE",
- "Misordered MAINTAINERS entry - list '$cur:' before '$prev:'\n" . $hereprev);
+ if (WARN("MAINTAINERS_STYLE",
+ "Misordered MAINTAINERS entry - list '$cur:' before '$prev:'\n" . $hereprev) &&
+ $fix && $prevrawline =~ /^\+[A-Z]:/) {
+ # swap these lines
+ $fixed[$fixlinenr - 1] = $rawline;
+ $fixed[$fixlinenr] = $prevrawline;
+ }
} elsif ((($prev eq 'F' && $cur eq 'F') ||
($prev eq 'X' && $cur eq 'X')) &&
($prevval cmp $curval) > 0) {
- WARN("MAINTAINERS_STYLE",
- "Misordered MAINTAINERS entry - list file patterns in alphabetic order\n" . $hereprev);
+ if (WARN("MAINTAINERS_STYLE",
+ "Misordered MAINTAINERS entry - list file patterns in alphabetic order\n" . $hereprev) &&
+ $fix && $prevrawline =~ /^\+[A-Z]:/) {
+ $fixed[$fixlinenr - 1] = $rawline;
+ $fixed[$fixlinenr] = $prevrawline;
+ }
}
}
}
--
2.17.1
On Tue, 2020-11-17 at 09:35 +0530, Aditya Srivastava wrote:
> Checkpatch expects entries in MAINTAINERS file in a specific order and
> warns if the changes made do not follow the specified order.
>
> E.g., running checkpatch on commit b33bc2b878e0 ("nexthop: Add entry to
> MAINTAINERS") reports this warning:
>
> WARNING: Misordered MAINTAINERS entry - list file patterns in
> alphabetic order
> +F: include/uapi/linux/nexthop.h
> +F: include/net/netns/nexthop.h
OK, this should work.
Thanks Aditya.
Acked-by: Joe Perches <[email protected]>
>
> Provide a simple fix by swapping the unordered lines, if both the lines
> are additions (start with '+')
>
> Signed-off-by: Aditya Srivastava <[email protected]>
> ---
> Changes in v2:
> modified commit message
>
> Changes in v3:
> add check if both the lines are additions(ie start with '+')
>
> ?scripts/checkpatch.pl | 17 +++++++++++++----
> ?1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2749f32dffe9..7ee3f05c354d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3299,13 +3299,22 @@ sub process {
> ? "Unknown MAINTAINERS entry type: '$cur'\n" . $herecurr);
> ? } else {
> ? if ($previndex >= 0 && $curindex < $previndex) {
> - WARN("MAINTAINERS_STYLE",
> - "Misordered MAINTAINERS entry - list '$cur:' before '$prev:'\n" . $hereprev);
> + if (WARN("MAINTAINERS_STYLE",
> + "Misordered MAINTAINERS entry - list '$cur:' before '$prev:'\n" . $hereprev) &&
> + $fix && $prevrawline =~ /^\+[A-Z]:/) {
> + # swap these lines
> + $fixed[$fixlinenr - 1] = $rawline;
> + $fixed[$fixlinenr] = $prevrawline;
> + }
> ? } elsif ((($prev eq 'F' && $cur eq 'F') ||
> ? ($prev eq 'X' && $cur eq 'X')) &&
> ? ($prevval cmp $curval) > 0) {
> - WARN("MAINTAINERS_STYLE",
> - "Misordered MAINTAINERS entry - list file patterns in alphabetic order\n" . $hereprev);
> + if (WARN("MAINTAINERS_STYLE",
> + "Misordered MAINTAINERS entry - list file patterns in alphabetic order\n" . $hereprev) &&
> + $fix && $prevrawline =~ /^\+[A-Z]:/) {
> + $fixed[$fixlinenr - 1] = $rawline;
> + $fixed[$fixlinenr] = $prevrawline;
> + }
> ? }
> ? }
> ? }
On Tue, 2020-11-17 at 09:35 +0530, Aditya Srivastava wrote:
> Checkpatch expects entries in MAINTAINERS file in a specific order and
> warns if the changes made do not follow the specified order.
>
> E.g., running checkpatch on commit b33bc2b878e0 ("nexthop: Add entry to
> MAINTAINERS") reports this warning:
>
> WARNING: Misordered MAINTAINERS entry - list file patterns in
> alphabetic order
> +F: include/uapi/linux/nexthop.h
> +F: include/net/netns/nexthop.h
>
> Provide a simple fix by swapping the unordered lines, if both the lines
> are additions (start with '+')
On second thought, nak.
This fails when there are 3 consecutive misordered lines.
SECTION
F: c
F: b
F: a
On Mon, 2020-11-16 at 20:26 -0800, Joe Perches wrote:
> On Tue, 2020-11-17 at 09:35 +0530, Aditya Srivastava wrote:
> > Checkpatch expects entries in MAINTAINERS file in a specific order and
> > warns if the changes made do not follow the specified order.
> >
> > E.g., running checkpatch on commit b33bc2b878e0 ("nexthop: Add entry to
> > MAINTAINERS") reports this warning:
> >
> > WARNING: Misordered MAINTAINERS entry - list file patterns in
> > alphabetic order
> > +F: include/uapi/linux/nexthop.h
> > +F: include/net/netns/nexthop.h
> >
> > Provide a simple fix by swapping the unordered lines, if both the lines
> > are additions (start with '+')
>
> On second thought, nak.
>
> This fails when there are 3 consecutive misordered lines.
>
> SECTION
> F: c
> F: b
> F: a
>
btw:
scripts/parse-maintainers.pl already does this reordering properly so
this particular --fix addition isn't all that useful.
On Tue, Nov 17, 2020 at 5:29 AM Joe Perches <[email protected]> wrote:
>
> On Mon, 2020-11-16 at 20:26 -0800, Joe Perches wrote:
> > On Tue, 2020-11-17 at 09:35 +0530, Aditya Srivastava wrote:
> > > Checkpatch expects entries in MAINTAINERS file in a specific order and
> > > warns if the changes made do not follow the specified order.
> > >
> > > E.g., running checkpatch on commit b33bc2b878e0 ("nexthop: Add entry to
> > > MAINTAINERS") reports this warning:
> > >
> > > WARNING: Misordered MAINTAINERS entry - list file patterns in
> > > alphabetic order
> > > +F: include/uapi/linux/nexthop.h
> > > +F: include/net/netns/nexthop.h
> > >
> > > Provide a simple fix by swapping the unordered lines, if both the lines
> > > are additions (start with '+')
> >
> > On second thought, nak.
> >
> > This fails when there are 3 consecutive misordered lines.
> >
> > SECTION
> > F: c
> > F: b
> > F: a
> >
>
> btw:
>
> scripts/parse-maintainers.pl already does this reordering properly so
> this particular --fix addition isn't all that useful.
>
I think the real fix is to provide some more documentation on
scripts/parse-maintainers.pl that explains how to run this script when
an author hits the warning type in checkpatch.pl.
I see these follow-up patches:
- some documentation on scripts/parse-maintainers.pl
- a patch to checkpatch.pl that points out this documentation when
this warning occurs.
- maybe: improve of scripts/parse-maintainers.pl to handle exactly
this use case of a patch author (assuming that the patch was just
created with git format-patch -1) and how to get the corrected diff
for this patch.
- maybe: a patch to checkpatch.pl that can create the command for
scripts/parse-maintainers.pl for a specific patch and which then can
be added with git commit --amend or git commit && git rebase and
squashing that in.
I think once that is done and better understood, you can much better
judge if there is really a need for a fix in checkpatch.pl.
Lukas
On 17/11/20 3:51 pm, Lukas Bulwahn wrote:
> On Tue, Nov 17, 2020 at 5:29 AM Joe Perches <[email protected]> wrote:
>>
>> On Mon, 2020-11-16 at 20:26 -0800, Joe Perches wrote:
>>> On Tue, 2020-11-17 at 09:35 +0530, Aditya Srivastava wrote:
>>>> Checkpatch expects entries in MAINTAINERS file in a specific order and
>>>> warns if the changes made do not follow the specified order.
>>>>
>>>> E.g., running checkpatch on commit b33bc2b878e0 ("nexthop: Add entry to
>>>> MAINTAINERS") reports this warning:
>>>>
>>>> WARNING: Misordered MAINTAINERS entry - list file patterns in
>>>> alphabetic order
>>>> +F: include/uapi/linux/nexthop.h
>>>> +F: include/net/netns/nexthop.h
>>>>
>>>> Provide a simple fix by swapping the unordered lines, if both the lines
>>>> are additions (start with '+')
>>>
>>> On second thought, nak.
>>>
>>> This fails when there are 3 consecutive misordered lines.
>>>
>>> SECTION
>>> F: c
>>> F: b
>>> F: a
>>>
>>
>> btw:
>>
>> scripts/parse-maintainers.pl already does this reordering properly so
>> this particular --fix addition isn't all that useful.
>>
>
> I think the real fix is to provide some more documentation on
> scripts/parse-maintainers.pl that explains how to run this script when
> an author hits the warning type in checkpatch.pl.
>
> I see these follow-up patches:
>
> - some documentation on scripts/parse-maintainers.pl
> - a patch to checkpatch.pl that points out this documentation when
> this warning occurs.
> - maybe: improve of scripts/parse-maintainers.pl to handle exactly
> this use case of a patch author (assuming that the patch was just
> created with git format-patch -1) and how to get the corrected diff
> for this patch.
> - maybe: a patch to checkpatch.pl that can create the command for
> scripts/parse-maintainers.pl for a specific patch and which then can
> be added with git commit --amend or git commit && git rebase and
> squashing that in.
>
> I think once that is done and better understood, you can much better
> judge if there is really a need for a fix in checkpatch.pl.
>
Okay thanks. Will check these out.
Thanks
Aditya
> Lukas
>