If a file exists in git and checkpatch is used without the -f
flag for scanning a file, then checkpatch will scan the file
assuming it's a patch and emit:
ERROR: Does not appear to be a unified-diff format patch
Change the behavior to assume the -f flag if the file exists
in git.
Signed-off-by: Joe Perches <[email protected]>
---
scripts/checkpatch.pl | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 79fc357b18cd..cdee7cfadc11 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -976,6 +976,16 @@ sub seed_camelcase_includes {
}
}
+sub git_is_single_file {
+ my ($filename) = @_;
+
+ return 0 if ((which("git") eq "") || !(-e "$gitroot"));
+
+ my $output = `${git_command} ls-files -- $filename`;
+ my $count = $output =~ tr/\n//;
+ return $count eq 1 && $output =~ m{^${filename}$};
+}
+
sub git_commit_info {
my ($commit, $id, $desc) = @_;
@@ -1049,6 +1059,9 @@ my $vname;
$allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"};
for my $filename (@ARGV) {
my $FILE;
+ my $is_git_file = git_is_single_file($filename);
+ my $oldfile = $file;
+ $file = 1 if ($is_git_file);
if ($git) {
open($FILE, '-|', "git format-patch -M --stdout -1 $filename") ||
die "$P: $filename: git format-patch failed - $!\n";
@@ -1093,6 +1106,7 @@ for my $filename (@ARGV) {
@modifierListFile = ();
@typeListFile = ();
build_types();
+ $file = $oldfile if ($is_git_file);
}
if (!$quiet) {
On 25/08/2020 02.09, Joe Perches wrote:
> If a file exists in git and checkpatch is used without the -f
> flag for scanning a file, then checkpatch will scan the file
> assuming it's a patch and emit:
>
> ERROR: Does not appear to be a unified-diff format patch
>
> Change the behavior to assume the -f flag if the file exists
> in git.
Heh, I read the patch subject to mean you introduced a way for subsystem
maintainers to prevent running checkpatch -f on their files, which I
think some would like ;)
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 79fc357b18cd..cdee7cfadc11 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -976,6 +976,16 @@ sub seed_camelcase_includes {
> }
> }
>
> +sub git_is_single_file {
> + my ($filename) = @_;
> +
> + return 0 if ((which("git") eq "") || !(-e "$gitroot"));
> +
> + my $output = `${git_command} ls-files -- $filename`;
> + my $count = $output =~ tr/\n//;
> + return $count eq 1 && $output =~ m{^${filename}$};
> +}
Isn't that somewhat expensive to do for each file? Why not postpone that
check till we're about to complain that the file is not a diff (haven't
looked at how such a refactoring would look).
Rasmus
On Tue, 2020-08-25 at 14:23 +0200, Rasmus Villemoes wrote:
> On 25/08/2020 02.09, Joe Perches wrote:
> > If a file exists in git and checkpatch is used without the -f
> > flag for scanning a file, then checkpatch will scan the file
> > assuming it's a patch and emit:
> >
> > ERROR: Does not appear to be a unified-diff format patch
> >
> > Change the behavior to assume the -f flag if the file exists
> > in git.
>
> Heh, I read the patch subject to mean you introduced a way for subsystem
> maintainers to prevent running checkpatch -f on their files, which I
> think some would like ;)
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 79fc357b18cd..cdee7cfadc11 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -976,6 +976,16 @@ sub seed_camelcase_includes {
> > }
> > }
> >
> > +sub git_is_single_file {
> > + my ($filename) = @_;
> > +
> > + return 0 if ((which("git") eq "") || !(-e "$gitroot"));
> > +
> > + my $output = `${git_command} ls-files -- $filename`;
> > + my $count = $output =~ tr/\n//;
> > + return $count eq 1 && $output =~ m{^${filename}$};
> > +}
>
> Isn't that somewhat expensive to do for each file? Why not postpone that
> check till we're about to complain that the file is not a diff (haven't
> looked at how such a refactoring would look).
It's necessary because you need the --file option set _before_
analyzing the file content.
Oddly, I didn't receive this email directly so I couldn't reply
to it earlier.
On Tue, 2020-08-25 at 14:23 +0200, Rasmus Villemoes wrote:
> On 25/08/2020 02.09, Joe Perches wrote:
> > If a file exists in git and checkpatch is used without the -f
> > flag for scanning a file, then checkpatch will scan the file
> > assuming it's a patch
[]
> > +sub git_is_single_file {
> > + my ($filename) = @_;
> > +
> > + return 0 if ((which("git") eq "") || !(-e "$gitroot"));
> > +
> > + my $output = `${git_command} ls-files -- $filename`;
> > + my $count = $output =~ tr/\n//;
> > + return $count eq 1 && $output =~ m{^${filename}$};
> > +}
>
> Isn't that somewhat expensive to do for each file?
Just FYI: Not really.
On my 4 year old laptop git ls-files -- <file> takes
about .02 seconds.
$ time git ls-files -- 'net/l2tp/l2tp_ip.c'
net/l2tp/l2tp_ip.c
real 0m0.013s
user 0m0.009s
sys 0m0.004s
Even uncached, it's quite quick.
# sync; echo 3 > /proc/sys/vm/drop_caches
$ time git ls-files -- 'net/l2tp/l2tp_ip.c'
net/l2tp/l2tp_ip.c
real 0m0.079s
user 0m0.004s
sys 0m0.037s
cheers, Joe
On Mon, 2020-08-24 at 17:09 -0700, Joe Perches wrote:
> If a file exists in git and checkpatch is used without the -f
> flag for scanning a file, then checkpatch will scan the file
> assuming it's a patch and emit:
>
> ERROR: Does not appear to be a unified-diff format patch
>
> Change the behavior to assume the -f flag if the file exists
> in git.
Andrew? ping?
> Signed-off-by: Joe Perches <[email protected]>
> ---
> scripts/checkpatch.pl | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 79fc357b18cd..cdee7cfadc11 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -976,6 +976,16 @@ sub seed_camelcase_includes {
> }
> }
>
> +sub git_is_single_file {
> + my ($filename) = @_;
> +
> + return 0 if ((which("git") eq "") || !(-e "$gitroot"));
> +
> + my $output = `${git_command} ls-files -- $filename`;
> + my $count = $output =~ tr/\n//;
> + return $count eq 1 && $output =~ m{^${filename}$};
> +}
> +
> sub git_commit_info {
> my ($commit, $id, $desc) = @_;
>
> @@ -1049,6 +1059,9 @@ my $vname;
> $allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"};
> for my $filename (@ARGV) {
> my $FILE;
> + my $is_git_file = git_is_single_file($filename);
> + my $oldfile = $file;
> + $file = 1 if ($is_git_file);
> if ($git) {
> open($FILE, '-|', "git format-patch -M --stdout -1 $filename") ||
> die "$P: $filename: git format-patch failed - $!\n";
> @@ -1093,6 +1106,7 @@ for my $filename (@ARGV) {
> @modifierListFile = ();
> @typeListFile = ();
> build_types();
> + $file = $oldfile if ($is_git_file);
> }
>
> if (!$quiet) {
>
Hi Joe,
On Tue, Aug 25, 2020 at 2:12 AM Joe Perches <[email protected]> wrote:
> If a file exists in git and checkpatch is used without the -f
> flag for scanning a file, then checkpatch will scan the file
> assuming it's a patch and emit:
>
> ERROR: Does not appear to be a unified-diff format patch
>
> Change the behavior to assume the -f flag if the file exists
> in git.
>
> Signed-off-by: Joe Perches <[email protected]>
Thanks for your patch!
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -976,6 +976,16 @@ sub seed_camelcase_includes {
> }
> }
>
> +sub git_is_single_file {
> + my ($filename) = @_;
> +
> + return 0 if ((which("git") eq "") || !(-e "$gitroot"));
> +
> + my $output = `${git_command} ls-files -- $filename`;
> + my $count = $output =~ tr/\n//;
> + return $count eq 1 && $output =~ m{^${filename}$};
> +}
> +
> sub git_commit_info {
> my ($commit, $id, $desc) = @_;
>
This is now commit f5f613259f3fea81 ("checkpatch: allow not using -f
with files that are in git"), causing:
Global symbol "$gitroot" requires explicit package name (did you
forget to declare "my $gitroot"?) at scripts/checkpatch.pl line 980.
Execution of scripts/checkpatch.pl aborted due to compilation errors.
FWIW, host system is running Ubuntu 18.04.5 LTS (upgrade to 20.04 LTS
planned soon ;-).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Sun, 2020-10-18 at 16:03 +0200, Geert Uytterhoeven wrote:
> Hi Joe,
>
> On Tue, Aug 25, 2020 at 2:12 AM Joe Perches <[email protected]> wrote:
> > If a file exists in git and checkpatch is used without the -f
> > flag for scanning a file, then checkpatch will scan the file
> > assuming it's a patch and emit:
> >
> > ERROR: Does not appear to be a unified-diff format patch
> >
> > Change the behavior to assume the -f flag if the file exists
> > in git.
> >
> > Signed-off-by: Joe Perches <[email protected]>
>
> Thanks for your patch!
>
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -976,6 +976,16 @@ sub seed_camelcase_includes {
> > }
> > }
> >
> > +sub git_is_single_file {
> > + my ($filename) = @_;
> > +
> > + return 0 if ((which("git") eq "") || !(-e "$gitroot"));
> > +
> > + my $output = `${git_command} ls-files -- $filename`;
> > + my $count = $output =~ tr/\n//;
> > + return $count eq 1 && $output =~ m{^${filename}$};
> > +}
> > +
> > sub git_commit_info {
> > my ($commit, $id, $desc) = @_;
> >
>
> This is now commit f5f613259f3fea81 ("checkpatch: allow not using -f
> with files that are in git"), causing:
>
> Global symbol "$gitroot" requires explicit package name (did you
> forget to declare "my $gitroot"?) at scripts/checkpatch.pl line 980.
> Execution of scripts/checkpatch.pl aborted due to compilation errors.
>
> FWIW, host system is running Ubuntu 18.04.5 LTS (upgrade to 20.04 LTS
> planned soon ;-).
I believe there is a dependency on another patch
in -next that wasn't pushed to Linus' tree.
commit 5ec1f7de97b26a3fa364bbb31fdd2e42c8e6fa22
Author: Joe Perches <[email protected]>
Date: Thu Oct 8 11:53:44 2020 +1100
checkpatch: test $GIT_DIR changes
So it'd be better to revert right now until
this other patch is accepted or pushed.
Hi Joe,
On Sun, Oct 18, 2020 at 6:07 PM Joe Perches <[email protected]> wrote:
> On Sun, 2020-10-18 at 16:03 +0200, Geert Uytterhoeven wrote:
> > On Tue, Aug 25, 2020 at 2:12 AM Joe Perches <[email protected]> wrote:
> > > If a file exists in git and checkpatch is used without the -f
> > > flag for scanning a file, then checkpatch will scan the file
> > > assuming it's a patch and emit:
> > >
> > > ERROR: Does not appear to be a unified-diff format patch
> > >
> > > Change the behavior to assume the -f flag if the file exists
> > > in git.
> > >
> > > Signed-off-by: Joe Perches <[email protected]>
> >
> > Thanks for your patch!
> >
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -976,6 +976,16 @@ sub seed_camelcase_includes {
> > > }
> > > }
> > >
> > > +sub git_is_single_file {
> > > + my ($filename) = @_;
> > > +
> > > + return 0 if ((which("git") eq "") || !(-e "$gitroot"));
> > > +
> > > + my $output = `${git_command} ls-files -- $filename`;
> > > + my $count = $output =~ tr/\n//;
> > > + return $count eq 1 && $output =~ m{^${filename}$};
> > > +}
> > > +
> > > sub git_commit_info {
> > > my ($commit, $id, $desc) = @_;
> > >
> >
> > This is now commit f5f613259f3fea81 ("checkpatch: allow not using -f
> > with files that are in git"), causing:
> >
> > Global symbol "$gitroot" requires explicit package name (did you
> > forget to declare "my $gitroot"?) at scripts/checkpatch.pl line 980.
> > Execution of scripts/checkpatch.pl aborted due to compilation errors.
> >
> > FWIW, host system is running Ubuntu 18.04.5 LTS (upgrade to 20.04 LTS
> > planned soon ;-).
>
> I believe there is a dependency on another patch
> in -next that wasn't pushed to Linus' tree.
>
> commit 5ec1f7de97b26a3fa364bbb31fdd2e42c8e6fa22
> Author: Joe Perches <[email protected]>
> Date: Thu Oct 8 11:53:44 2020 +1100
>
> checkpatch: test $GIT_DIR changes
>
> So it'd be better to revert right now until
> this other patch is accepted or pushed.
Thanks, after cherry-picking that one from next, checkpatch works again.
However, there are some issues with that commit:
1. ERROR: Missing Signed-off-by: line by nominal patch author 'Joe
Perches <[email protected]>',
2. The Link: is bogus, and gives 404.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Sun, 2020-10-18 at 20:15 +0200, Geert Uytterhoeven wrote:
> Hi Joe,
rehi Geert
> On Sun, Oct 18, 2020 at 6:07 PM Joe Perches <[email protected]> wrote:
> > On Sun, 2020-10-18 at 16:03 +0200, Geert Uytterhoeven wrote:
[]
> > > This is now commit f5f613259f3fea81 ("checkpatch: allow not using -f
> > > with files that are in git"), causing:
> > >
> > > Global symbol "$gitroot" requires explicit package name (did you
> > > forget to declare "my $gitroot"?) at scripts/checkpatch.pl line 980.
> > > Execution of scripts/checkpatch.pl aborted due to compilation errors.
[]
> > I believe there is a dependency on another patch
> > in -next that wasn't pushed to Linus' tree.
> >
> > commit 5ec1f7de97b26a3fa364bbb31fdd2e42c8e6fa22
> > Author: Joe Perches <[email protected]>
> > Date: Thu Oct 8 11:53:44 2020 +1100
> >
> > checkpatch: test $GIT_DIR changes
> >
> > So it'd be better to revert right now until
> > this other patch is accepted or pushed.
>
> Thanks, after cherry-picking that one from next, checkpatch works again.
> However, there are some issues with that commit:
> 1. ERROR: Missing Signed-off-by: line by nominal patch author 'Joe
> Perches <[email protected]>',
> 2. The Link: is bogus, and gives 404.
I generally create patches against -next.
The above commit was a test patch for Andrew who
had some inconvenience because he doesn't generally
use git or has a git repo in some non-standard path.
I believe it works well enough to be OK, but I
didn't test it and don't have the same setup.
I'll post it again as a reply to this email with a
with a sign-off and a better commit description and
Linus/Andrew can decide if it's better to revert
f5f613259f3f or apply it separately.
On Sun, Oct 18, 2020 at 11:26:59AM -0700, Joe Perches wrote:
> I'll post it again as a reply to this email with a
> with a sign-off and a better commit description and
> Linus/Andrew can decide if it's better to revert
> f5f613259f3f or apply it separately.
Gentle reminder on this, it is the last weekday of the merge window
and checkpatch is still failing on Linus's latest (b76f733c)..
Global symbol "$gitroot" requires explicit package name (did you forget to declare "my $gitroot"?) at scripts/checkpatch.pl line 980.
Thanks,
Jason