[v2] checkpatch: backport UTF-8 fixes and MAINTAINERS check

[Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check

Posted by Stefan Hajnoczi 5 years, 12 months ago

v2:
 * Resolve missing CHK() [Thomas]
 * Drop first argument to WARN() to match QEMU function arguments

This series cherry picks checkpatch UTF-8 fixes and the MAINTAINERS file check
from Linux.  Thomas Huth original did the backport in January 2017 but the
series was forgotten (<1485436265-12573-1-git-send-email-thuth@redhat.com>).  I
did the cherry pick again from scratch.

The MAINTAINERS file check prints a warning when new files are added without a
modification to ./MAINTAINERS.  It is easy to forget to update ./MAINTAINERS
and this check should help us stay on top of new source files.

Joe Perches (4):
  checkpatch: add a --strict check for utf-8 in commit logs
  checkpatch: ignore email headers better
  checkpatch: emit a warning on file add/move/delete
  checkpatch: reduce MAINTAINERS update message frequency

Pasi Savanainen (1):
  checkpatch: check utf-8 content from a commit log when it's missing
    from charset

 scripts/checkpatch.pl | 56 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 4 deletions(-)

-- 
2.14.3

Re: [Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check

Posted by no-reply@patchew.org 5 years, 12 months ago

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180430124651.10340-1-stefanha@redhat.com
Subject: [Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]            patchew/20180421163957.29872-1-mreitz@redhat.com -> patchew/20180421163957.29872-1-mreitz@redhat.com
 * [new tag]               patchew/20180430124651.10340-1-stefanha@redhat.com -> patchew/20180430124651.10340-1-stefanha@redhat.com
Switched to a new branch 'test'
19015a6f3e checkpatch: reduce MAINTAINERS update message frequency
7fb20f970d checkpatch: emit a warning on file add/move/delete
9303812d85 checkpatch: ignore email headers better
f7cec089ae checkpatch: check utf-8 content from a commit log when it's missing from charset
d6a1291e63 checkpatch: add a --strict check for utf-8 in commit logs

=== OUTPUT BEGIN ===
Checking PATCH 1/5: checkpatch: add a --strict check for utf-8 in commit logs...
WARNING: line over 80 characters
#101: FILE: scripts/checkpatch.pl:1420:
+			WARN("8-bit UTF-8 used in possible commit log\n" . $herecurr);

total: 0 errors, 1 warnings, 66 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/5: checkpatch: check utf-8 content from a commit log when it's missing from charset...
Checking PATCH 3/5: checkpatch: ignore email headers better...
Checking PATCH 4/5: checkpatch: emit a warning on file add/move/delete...
WARNING: line over 80 characters
#46: FILE: scripts/checkpatch.pl:1397:
+		     ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&

ERROR: line over 90 characters
#49: FILE: scripts/checkpatch.pl:1400:
+			WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);

total: 1 errors, 1 warnings, 24 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 5/5: checkpatch: reduce MAINTAINERS update message frequency...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check

Posted by Stefan Hajnoczi 5 years, 12 months ago

On Mon, Apr 30, 2018 at 05:56:30AM -0700, no-reply@patchew.org wrote:
> === OUTPUT BEGIN ===
> Checking PATCH 1/5: checkpatch: add a --strict check for utf-8 in commit logs...
> WARNING: line over 80 characters
> #101: FILE: scripts/checkpatch.pl:1420:
> +			WARN("8-bit UTF-8 used in possible commit log\n" . $herecurr);
> 
> total: 0 errors, 1 warnings, 66 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> Checking PATCH 2/5: checkpatch: check utf-8 content from a commit log when it's missing from charset...
> Checking PATCH 3/5: checkpatch: ignore email headers better...
> Checking PATCH 4/5: checkpatch: emit a warning on file add/move/delete...
> WARNING: line over 80 characters
> #46: FILE: scripts/checkpatch.pl:1397:
> +		     ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
> 
> ERROR: line over 90 characters
> #49: FILE: scripts/checkpatch.pl:1400:
> +			WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
> 
> total: 1 errors, 1 warnings, 24 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> Checking PATCH 5/5: checkpatch: reduce MAINTAINERS update message frequency...
> === OUTPUT END ===

If there are no objections I'd like to keep this in sync with upstream,
and error messages are exempt from the 80 char limit to aid grepping.

Stefan

Re: [Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check

Posted by Thomas Huth 5 years, 12 months ago

On 01.05.2018 15:04, Stefan Hajnoczi wrote:
> On Mon, Apr 30, 2018 at 05:56:30AM -0700, no-reply@patchew.org wrote:
>> === OUTPUT BEGIN ===
>> Checking PATCH 1/5: checkpatch: add a --strict check for utf-8 in commit logs...
>> WARNING: line over 80 characters
>> #101: FILE: scripts/checkpatch.pl:1420:
>> +			WARN("8-bit UTF-8 used in possible commit log\n" . $herecurr);
>>
>> total: 0 errors, 1 warnings, 66 lines checked
>>
>> Your patch has style problems, please review.  If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.
>> Checking PATCH 2/5: checkpatch: check utf-8 content from a commit log when it's missing from charset...
>> Checking PATCH 3/5: checkpatch: ignore email headers better...
>> Checking PATCH 4/5: checkpatch: emit a warning on file add/move/delete...
>> WARNING: line over 80 characters
>> #46: FILE: scripts/checkpatch.pl:1397:
>> +		     ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
>>
>> ERROR: line over 90 characters
>> #49: FILE: scripts/checkpatch.pl:1400:
>> +			WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
>>
>> total: 1 errors, 1 warnings, 24 lines checked
>>
>> Your patch has style problems, please review.  If any of these errors
>> are false positives report them to the maintainer, see
>> CHECKPATCH in MAINTAINERS.
>>
>> Checking PATCH 5/5: checkpatch: reduce MAINTAINERS update message frequency...
>> === OUTPUT END ===
> 
> If there are no objections I'd like to keep this in sync with upstream,
> and error messages are exempt from the 80 char limit to aid grepping.

Yes, please keep it as close as possible to upstream. This will also
help to backport future patches from the Linux version to our version
(so we can cherry-pick without conflicts).

 Thomas

Re: [Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check

Posted by Fam Zheng 5 years, 12 months ago

On Tue, 05/01 14:04, Stefan Hajnoczi wrote:
> On Mon, Apr 30, 2018 at 05:56:30AM -0700, no-reply@patchew.org wrote:
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/5: checkpatch: add a --strict check for utf-8 in commit logs...
> > WARNING: line over 80 characters
> > #101: FILE: scripts/checkpatch.pl:1420:
> > +			WARN("8-bit UTF-8 used in possible commit log\n" . $herecurr);
> > 
> > total: 0 errors, 1 warnings, 66 lines checked
> > 
> > Your patch has style problems, please review.  If any of these errors
> > are false positives report them to the maintainer, see
> > CHECKPATCH in MAINTAINERS.
> > Checking PATCH 2/5: checkpatch: check utf-8 content from a commit log when it's missing from charset...
> > Checking PATCH 3/5: checkpatch: ignore email headers better...
> > Checking PATCH 4/5: checkpatch: emit a warning on file add/move/delete...
> > WARNING: line over 80 characters
> > #46: FILE: scripts/checkpatch.pl:1397:
> > +		     ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
> > 
> > ERROR: line over 90 characters
> > #49: FILE: scripts/checkpatch.pl:1400:
> > +			WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
> > 
> > total: 1 errors, 1 warnings, 24 lines checked
> > 
> > Your patch has style problems, please review.  If any of these errors
> > are false positives report them to the maintainer, see
> > CHECKPATCH in MAINTAINERS.
> > 
> > Checking PATCH 5/5: checkpatch: reduce MAINTAINERS update message frequency...
> > === OUTPUT END ===
> 
> If there are no objections I'd like to keep this in sync with upstream,
> and error messages are exempt from the 80 char limit to aid grepping.

Yeah, I actually like such exceptions for message strings constants in general.

Fam

Re: [Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check

Posted by Stefan Hajnoczi 5 years, 11 months ago

On Mon, Apr 30, 2018 at 01:46:46PM +0100, Stefan Hajnoczi wrote:
> v2:
>  * Resolve missing CHK() [Thomas]
>  * Drop first argument to WARN() to match QEMU function arguments
> 
> This series cherry picks checkpatch UTF-8 fixes and the MAINTAINERS file check
> from Linux.  Thomas Huth original did the backport in January 2017 but the
> series was forgotten (<1485436265-12573-1-git-send-email-thuth@redhat.com>).  I
> did the cherry pick again from scratch.
> 
> The MAINTAINERS file check prints a warning when new files are added without a
> modification to ./MAINTAINERS.  It is easy to forget to update ./MAINTAINERS
> and this check should help us stay on top of new source files.
> 
> Joe Perches (4):
>   checkpatch: add a --strict check for utf-8 in commit logs
>   checkpatch: ignore email headers better
>   checkpatch: emit a warning on file add/move/delete
>   checkpatch: reduce MAINTAINERS update message frequency
> 
> Pasi Savanainen (1):
>   checkpatch: check utf-8 content from a commit log when it's missing
>     from charset
> 
>  scripts/checkpatch.pl | 56 +++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 52 insertions(+), 4 deletions(-)
> 
> -- 
> 2.14.3
> 
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Re: [Qemu-devel] [PATCH v2 0/5] checkpatch: backport UTF-8 fixes and MAINTAINERS check

Posted by Markus Armbruster 5 years, 12 months ago

Stefan Hajnoczi <stefanha@redhat.com> writes:

> v2:
>  * Resolve missing CHK() [Thomas]
>  * Drop first argument to WARN() to match QEMU function arguments
>
> This series cherry picks checkpatch UTF-8 fixes and the MAINTAINERS file check
> from Linux.  Thomas Huth original did the backport in January 2017 but the
> series was forgotten (<1485436265-12573-1-git-send-email-thuth@redhat.com>).  I
> did the cherry pick again from scratch.
>
> The MAINTAINERS file check prints a warning when new files are added without a
> modification to ./MAINTAINERS.  It is easy to forget to update ./MAINTAINERS
> and this check should help us stay on top of new source files.

The matching is a bit sloppy in places, but if it's good enough for the
kernel...

Series:
Reviewed-by: Markus Armbruster <armbru@redhat.com>