* [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
@ 2016-08-29 21:00 Jacob Keller
2016-08-31 20:27 ` Brown, Aaron F
0 siblings, 1 reply; 8+ messages in thread
From: Jacob Keller @ 2016-08-29 21:00 UTC (permalink / raw)
To: intel-wired-lan
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.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
scripts/checkpatch.pl | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4de3cc42fc50..ad8d3509f2d7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -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) {
--
2.10.0.rc2.311.g2bd286e
^ permalink raw reply related [flat|nested] 8+ messages in thread* [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
2016-08-29 21:00 [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root Jacob Keller
@ 2016-08-31 20:27 ` Brown, Aaron F
2016-08-31 22:37 ` Keller, Jacob E
0 siblings, 1 reply; 8+ messages in thread
From: Brown, Aaron F @ 2016-08-31 20:27 UTC (permalink / raw)
To: intel-wired-lan
> 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 <intel-wired-lan@lists.osuosl.org>
> Subject: [Intel-wired-lan] [PATCH v2] 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.
>
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> 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...
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
2016-08-31 20:27 ` Brown, Aaron F
@ 2016-08-31 22:37 ` Keller, Jacob E
2016-09-01 1:04 ` Brown, Aaron F
0 siblings, 1 reply; 8+ messages in thread
From: Keller, Jacob E @ 2016-08-31 22:37 UTC (permalink / raw)
To: intel-wired-lan
> -----Original Message-----
> From: Brown, Aaron F
> Sent: Wednesday, August 31, 2016 1:28 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>; 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
>
> > 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 <intel-wired-lan@lists.osuosl.org>
> > Subject: [Intel-wired-lan] [PATCH v2] 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.
> >
> > Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> > 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...
>
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
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
2016-08-31 22:37 ` Keller, Jacob E
@ 2016-09-01 1:04 ` Brown, Aaron F
2016-09-01 2:33 ` Jeff Kirsher
0 siblings, 1 reply; 8+ messages in thread
From: Brown, Aaron F @ 2016-09-01 1:04 UTC (permalink / raw)
To: intel-wired-lan
> From: Keller, Jacob E
> Sent: Wednesday, August 31, 2016 3:37 PM
> To: Brown, Aaron F <aaron.f.brown@intel.com>; 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>
> > 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.
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 ;)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
2016-09-01 1:04 ` Brown, Aaron F
@ 2016-09-01 2:33 ` Jeff Kirsher
2016-09-01 2:46 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Kirsher @ 2016-09-01 2:33 UTC (permalink / raw)
To: intel-wired-lan
On Thu, 2016-09-01 at 01:04 +0000, Brown, Aaron F wrote:
> > From: Keller, Jacob E
> > Sent: Wednesday, August 31, 2016 3:37 PM
> > To: Brown, Aaron F <aaron.f.brown@intel.com>; 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>
> > > 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.
>
> 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>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
2016-09-01 2:33 ` Jeff Kirsher
@ 2016-09-01 2:46 ` Joe Perches
2016-09-01 3:17 ` Jeff Kirsher
0 siblings, 1 reply; 8+ messages in thread
From: Joe Perches @ 2016-09-01 2:46 UTC (permalink / raw)
To: intel-wired-lan
On Wed, 2016-08-31 at 19:33 -0700, Jeff Kirsher wrote:
> On Thu, 2016-09-01 at 01:04 +0000, Brown, Aaron F wrote:
> > From: Keller, Jacob E
Hi all.
> > > > ---------------------------------------------------------------------
> > > > 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
> 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
2016-09-01 2:46 ` Joe Perches
@ 2016-09-01 3:17 ` Jeff Kirsher
2016-09-01 3:26 ` Joe Perches
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Kirsher @ 2016-09-01 3:17 UTC (permalink / raw)
To: intel-wired-lan
On Wed, 2016-08-31 at 19:46 -0700, Joe Perches wrote:
> On Wed, 2016-08-31 at 19:33 -0700, Jeff Kirsher wrote:
> >
> > On Thu, 2016-09-01 at 01:04 +0000, Brown, Aaron F wrote:
> > >
> > > From: Keller, Jacob E
>
> Hi all.
>
> >
> > >
> > > >
> > > > >
> > > > > ---------------------------------------------------------------
> > > > > ------
> > > > > 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
> > 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?
I thought you got $0.02 for every checkpatch.pl warning and $0.05 for every
error found? :-)
> Perl is already basically unintelligible.
> 80 column perl would be a whole lot worse.
Agreed.
> 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 for
drivers/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>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root
2016-09-01 3:17 ` Jeff Kirsher
@ 2016-09-01 3:26 ` Joe Perches
0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2016-09-01 3:26 UTC (permalink / raw)
To: intel-wired-lan
On Wed, 2016-08-31 at 20:17 -0700, Jeff Kirsher wrote:
> 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.
> > 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 for
> drivers/net/*, so it should be easy to apply whatever checks are pertinent
> for scripts/*?
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-01 3:26 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-29 21:00 [Intel-wired-lan] [PATCH v2] checkpatch.pl: seed camelcase from the provided kernel tree root Jacob Keller
2016-08-31 20:27 ` Brown, Aaron F
2016-08-31 22:37 ` Keller, Jacob E
2016-09-01 1:04 ` Brown, Aaron F
2016-09-01 2:33 ` Jeff Kirsher
2016-09-01 2:46 ` Joe Perches
2016-09-01 3:17 ` Jeff Kirsher
2016-09-01 3:26 ` Joe Perches
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.