checkpatch.pl: seed camelcase from the provided kernel tree root
PATCH v2 1/1 checkpatch.pl: seed camelcase from the provided kernel tree root
When checkpatch.pl is run without a git tree, it seeds the camelcase includes from the $root parameter. However, it does not use the $root directory when seeding for a git tree. Fix this by using "cd $root &&" so that a user may run checkpatch with the --root parameter pointing to a valid kernel source tree. In addition, when generating the list of files to check, we must also prefix each file with the $root parameter, in order to properly locate the file when searching.
We avoid the use of -C parameter of git because it is not supported on old versions of git, so we want to avoid breaking checkpatch.pl on those systems.
--- scripts/checkpatch.pl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
scripts/checkpatch.pl +5 -4
@@ -714,8 +714,8 @@ sub seed_camelcase_includes { $camelcase_seeded = 1; - if (-e ".git") { - my $git_last_include_commit = `git log --no-merges --pretty=format:"%h%n" -1 -- include`; + if (-e "$root/.git") { + my $git_last_include_commit = `cd $root && git log --no-merges --pretty=format:"%h%n" -1 -- include`; chomp $git_last_include_commit; $camelcase_cache = ".checkpatch-camelcase.git.$git_last_include_commit"; } else {
@@ -742,9 +742,10 @@ sub seed_camelcase_includes { return; } - if (-e ".git") { - $files = `git ls-files "include/*.h"`; + if (-e "$root/.git") { + $files = `cd $root && git ls-files "include/*.h"`; @include_files = split('\n', $files); + @include_files = map("$root/$_", @include_files); } foreach my $file (@include_files) {
Signature · 1 line
2.10.0.rc2.311.g2bd286e
Brown
PATCH v2 1/1 checkpatch.pl: seed camelcase from the provided kernel tree root
Quoted 15 lines
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jacob Keller
Sent: Monday, August 29, 2016 2:01 PM
To: Intel Wired LAN <[email protected]>
Subject: [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree rootWhen checkpatch.pl is run without a git tree, it seeds the camelcase includes from the $root parameter. However, it does not use the $root directory when seeding for a git tree. Fix this by using "cd $root &&" so that a user may run checkpatch with the --root parameter pointing to a valid kernel source tree. In addition, when generating the list of files to check, we must also prefix each file with the $root parameter, in order to properly locate the file when searching.
Quoted 8 lines
We avoid the use of -C parameter of git because it is not supported on old versions of git, so we want to avoid breaking checkpatch.pl on those systems.
Signed-off-by Jacob Keller <[email protected]>--- scripts/checkpatch.pl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
Well, it does throw a few checkpatch warnings... oh the irony.
---------------------------------------------------------------------
WARNING: A patch subject line should describe the change not the tool that found it #4:
Subject: [PATCH] checkpatch.pl: seed camelcase from the provided kernel tree
WARNING: line over 80 characters
#35: FILE: scripts/checkpatch.pl:718:
+ my $git_last_include_commit = cd $root && git log --no-merges --pretty=format:"%h%n" -1 -- include;
total: 0 errors, 2 warnings, 22 lines checked
---------------------------------------------------------------------
The first is clearly a false warning, thinks checkpatch is the tool that found the error rather than the tool being fixed. ;) The second is just a long line in the perl code, which I don't really consider a blocking issue so...
Keller
PATCH v2 1/1 checkpatch.pl: seed camelcase from the provided kernel tree root
Quoted 47 lines
-----Original Message-----
From: Brown, Aaron F
Sent: Wednesday, August 31, 2016 1:28 PM
To: Keller, Jacob E <[email protected]>; Intel Wired LAN <intel- wired-lan at lists.osuosl.org>
Subject: RE: [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree rootQuoted 1 line
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org]
On
Quoted 4 lines
Behalf Of Jacob Keller
Sent: Monday, August 29, 2016 2:01 PM
To: Intel Wired LAN <[email protected]>
Subject: [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase fromthe
Quoted 10 lines
provided kernel tree root
When checkpatch.pl is run without a git tree, it seeds the camelcase includes from the $root parameter. However, it does not use the $root directory when seeding for a git tree. Fix this by using "cd $root &&" so that a user may run checkpatch with the --root parameter pointing to a valid kernel source tree. In addition, when generating the list of files to check, we must also prefix each file with the $root parameter, in order to properly locate the file when searching.
Quoted 8 lines
We avoid the use of -C parameter of git because it is not supported on old versions of git, so we want to avoid breaking checkpatch.pl on those systems.
Signed-off-by Jacob Keller <[email protected]>--- scripts/checkpatch.pl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)Well, it does throw a few checkpatch warnings... oh the irony.
---------------------------------------------------------------------
WARNING: A patch subject line should describe the change not the tool that found it
#4:
Subject: [PATCH] checkpatch.pl: seed camelcase from the provided kernel treeWARNING: line over 80 characters
#35: FILE:scripts/checkpatch.pl:718:(quoted diff) +1
-1+ my $git_last_include_commit = `cd $root && git log --no-merges - -pretty=format:"%h%n" -1 -- include`;
Quoted 6 lines
total: 0 errors, 2 warnings, 22 lines checked
---------------------------------------------------------------------
The first is clearly a false warning, thinks checkpatch is the tool that found
the error rather than the tool being fixed. ;) The second is just a long line in
the perl code, which I don't really consider a blocking issue so...
Ya I saw those. I saw many lines over 80 characters in the file so I just assumed we don't bother checking the tool with itself.
Thanks,
Jake
Quoted 1 line
Tested-by Aaron Brown <[email protected]>
Brown
PATCH v2 1/1 checkpatch.pl: seed camelcase from the provided kernel tree root
Quoted 7 lines
From: Keller, Jacob E
Sent: Wednesday, August 31, 2016 3:37 PM
To: Brown, Aaron F <[email protected]>; Intel Wired LAN <intel- wired-lan at lists.osuosl.org>
Subject: RE: [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
<snip>
Quoted 10 lines
Quoted 4 lines
total: 0 errors, 2 warnings, 22 lines checked
---------------------------------------------------------------------
The first is clearly a false warning, thinks checkpatch is the tool that found
the error rather than the tool being fixed. ;) The second is just a long linein
Quoted 2 lines
the perl code, which I don't really consider a blocking issue so...
Ya I saw those. I saw many lines over 80 characters in the file so I just assumed we don't bother checking the tool with itself.
Well, it showed up in as a patch so I ran it through... But yeah, it does not make sense to enforce rules intended for the kernel against something else. Especially as the precedent for line length is clearly already in the file ;)
PATCH v2 1/1 checkpatch.pl: seed camelcase from the provided kernel tree root
Quoted 20 lines · Aaron F
Quoted 5 lines
From: Keller, Jacob E
Sent: Wednesday, August 31, 2016 3:37 PM
To: Brown, Aaron F <[email protected]>; Intel Wired LAN <intel- wired-lan at lists.osuosl.org>
Subject: RE: [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcasefrom
Quoted 2 lines
the provided kernel tree root
?<snip>
Quoted 3 lines
Quoted 3 lines
total: 0 errors, 2 warnings, 22 lines checked
---------------------------------------------------------------------
The first is clearly a false warning, thinks checkpatch is the toolthat found
Quoted 1 line
Quoted 1 line
the error rather than the tool being fixed.? ;)?? The second is just
a long line
Quoted 5 lines
in
Quoted 2 lines
the perl code, which I don't really consider a blocking issue so...
?
Ya I saw those. I saw many lines over 80 characters in the file so I
Quoted 7 lines
just
Quoted 1 line
assumed we don't bother checking the tool with itself.
Well, it showed up in as a patch so I ran it through...? But yeah, it does not make sense to enforce rules intended for the kernel against something else.? Especially as the precedent for line length is clearly already in the file
Although it technically is a part of the kernel source since it is in scripts. ?So the question is, should kernel scripts follow the coding standards? ?If so, then yes it should follow its own rules for kernel source.
Personally I do not think it should be an issue for the contents of
/scripts in the kernel source, but that would be an interesting question to
pose to Joe Perches and the other checkpatch.pl warlords.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20160831/8450b5d3/attachment.asc>
Joe Perches
maintainerPATCH v2 1/1 checkpatch.pl: seed camelcase from the provided kernel tree root
Quoted 2 lines · Jeff Kirsher
Quoted 1 line · Aaron F
From: Keller, Jacob E
Hi all.
Quoted 10 lines
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
---------------------------------------------------------------------
The first is clearly a false warning, thinks checkpatch is the tool that found the error rather than the tool being fixed.? ;)?? The second is just a long line in the perl code, which I don't really consider a blocking issue so...Ya I saw those. I saw many lines over 80 characters in the file so I just assumed we don't bother checking the tool with itself.
Well, it showed up in as a patch so I ran it through...? But yeah, it does not make sense to enforce rules intended for the kernel against something else.? Especially as the precedent for line length is clearly already in the file
Quoted 8 lines
Although it technically is a part of the kernel source since it is in scripts. ?So the question is, should kernel scripts follow the coding standards? ?If so, then yes it should follow its own rules for kernel source.
Personally I do not think it should be an issue for the contents of /scripts in the kernel source, but that would be an interesting question to pose to Joe Perches and the other checkpatch.pl warlords.
Warlord? ?Damn. ?Where are my spoils of war?
Perl is already basically unintelligible.
80 column perl would be a whole lot worse.
Anyway, I don't look at scripts with checkpatch.
I think it's really only useful for .[ch] files.
PATCH v2 1/1 checkpatch.pl: seed camelcase from the provided kernel tree root
Quoted 33 lines · Joe Perches
Quoted 4 lines · Jeff Kirsher
Quoted 2 lines · Aaron F
From: Keller, Jacob E
Hi all.
Quoted 25 lines
Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
---------------------------------------------------------------
------
The first is clearly a false warning, thinks checkpatch is the tool
that found the error rather than the tool being fixed.? ;)?? The second is just
a long line in the perl code, which I don't really consider a blocking issue so...Quoted 8 lines
Quoted 3 lines
Ya I saw those. I saw many lines over 80 characters in the file so I
just assumed we don't bother checking the tool with itself.Well, it showed up in as a patch so I ran it through...? But yeah, it does not make sense to enforce rules intended for the kernel against something else.? Especially as the precedent for line length is clearly
already in the fileAlthough it technically is a part of the kernel source since it is in scripts. ?So the question is, should kernel scripts follow the coding standards? ?If so, then yes it should follow its own rules for kernel source.
Quoted 6 lines
Quoted 4 lines
Personally I do not think it should be an issue for the contents of /scripts in the kernel source, but that would be an interesting question to
pose to Joe Perches and the other checkpatch.pl warlords.Warlord? ?Damn. ?Where are my spoils of war?
I thought you got $0.02 for every checkpatch.pl warning and $0.05 for every error found? :-)
Quoted 2 lines
Perl is already basically unintelligible.
80 column perl would be a whole lot worse.
Agreed.
Quoted 3 lines
Anyway, I don't look at scripts with checkpatch.
I think it's really only useful for .[ch] files.
So should a modification be made to anything in scripts/* for when
checkpatch.pl is run? ?I know that there are *special* rules fordrivers/net/*, so it should be easy to apply whatever checks are pertinent
for scripts/*?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20160831/a61db057/attachment.asc>
Joe Perches
maintainerPATCH v2 1/1 checkpatch.pl: seed camelcase from the provided kernel tree root
Quoted 2 lines · Jeff Kirsher
I thought you got $0.02 for every checkpatch.pl warning and $0.05 for every error found? :-)
Well, it's nice to know some credits are accruing in some virtual bank. ?Unfortunate for me that it's like the Hotel California Bank though.
Quoted 6 lines
Quoted 2 lines
I don't look at scripts with checkpatch.
I think it's really only useful for .[ch] files.So should a modification be made to anything in
scripts/*for when checkpatch.pl is run? ?I know that there are *special* rules fordrivers/net/*, so it should be easy to apply whatever checks are pertinent forscripts/*?
I think scripts should rely a little on the skill and intelligence of the user.
I have a hammer so let's go fishing, etc...
cheers, ?Joe