* [PATCH] checkpatch: improve error message for p1-check
@ 2011-10-19 12:33 Wolfram Sang
2011-10-26 20:30 ` David Rientjes
0 siblings, 1 reply; 10+ messages in thread
From: Wolfram Sang @ 2011-10-19 12:33 UTC (permalink / raw)
To: linux-kernel; +Cc: Jonathan Cameron, Wolfram Sang, Andy Whitcroft
The old error message was not precise. State now explicitly that the
prefix exists as a file/directory. Also change the conclusion ("appears
to be") into an instruction ("make sure").
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
Cc: Andy Whitcroft <apw@canonical.com>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3dfc471..4bb4f88 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1506,7 +1506,7 @@ sub process {
if (!$file && $tree && $p1_prefix ne '' &&
-e "$root/$p1_prefix") {
WARN("PATCH_PREFIX",
- "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n");
+ "patch prefix '$p1_prefix' exists as file/directory. Make sure this isn't a -p0 patch\n");
}
if ($realfile =~ m@^include/asm/@) {
--
1.7.6.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-19 12:33 [PATCH] checkpatch: improve error message for p1-check Wolfram Sang
@ 2011-10-26 20:30 ` David Rientjes
2011-10-26 22:51 ` Joe Perches
0 siblings, 1 reply; 10+ messages in thread
From: David Rientjes @ 2011-10-26 20:30 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, Jonathan Cameron, Andy Whitcroft
On Wed, 19 Oct 2011, Wolfram Sang wrote:
> The old error message was not precise. State now explicitly that the
> prefix exists as a file/directory. Also change the conclusion ("appears
> to be") into an instruction ("make sure").
>
Why can't we suppress this warning unless both prefixes appear as files or
directories in the cwd? ('a' and 'b' if using git format-patch)
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> Cc: Andy Whitcroft <apw@canonical.com>
> ---
> scripts/checkpatch.pl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 3dfc471..4bb4f88 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1506,7 +1506,7 @@ sub process {
> if (!$file && $tree && $p1_prefix ne '' &&
> -e "$root/$p1_prefix") {
> WARN("PATCH_PREFIX",
> - "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n");
> + "patch prefix '$p1_prefix' exists as file/directory. Make sure this isn't a -p0 patch\n");
> }
>
> if ($realfile =~ m@^include/asm/@) {
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-26 20:30 ` David Rientjes
@ 2011-10-26 22:51 ` Joe Perches
2011-10-26 23:08 ` David Rientjes
0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2011-10-26 22:51 UTC (permalink / raw)
To: David Rientjes
Cc: Wolfram Sang, linux-kernel, Jonathan Cameron, Andy Whitcroft
On Wed, 2011-10-26 at 13:30 -0700, David Rientjes wrote:
> On Wed, 19 Oct 2011, Wolfram Sang wrote:
> > The old error message was not precise. State now explicitly that the
> > prefix exists as a file/directory. Also change the conclusion ("appears
> > to be") into an instruction ("make sure").
> Why can't we suppress this warning unless both prefixes appear as files or
> directories in the cwd? ('a' and 'b' if using git format-patch)
I sent a suggested patch that checked $root/$patched_file
https://lkml.org/lkml/2011/10/19/257
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-26 22:51 ` Joe Perches
@ 2011-10-26 23:08 ` David Rientjes
2011-10-26 23:14 ` Joe Perches
0 siblings, 1 reply; 10+ messages in thread
From: David Rientjes @ 2011-10-26 23:08 UTC (permalink / raw)
To: Joe Perches; +Cc: Wolfram Sang, linux-kernel, Jonathan Cameron, Andy Whitcroft
On Wed, 26 Oct 2011, Joe Perches wrote:
> > > The old error message was not precise. State now explicitly that the
> > > prefix exists as a file/directory. Also change the conclusion ("appears
> > > to be") into an instruction ("make sure").
> > Why can't we suppress this warning unless both prefixes appear as files or
> > directories in the cwd? ('a' and 'b' if using git format-patch)
>
> I sent a suggested patch that checked $root/$patched_file
> https://lkml.org/lkml/2011/10/19/257
>
So we need a combination of the two approaches then, it makes sense to
only emit the warning if the patched file exists in both prefixes.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-26 23:08 ` David Rientjes
@ 2011-10-26 23:14 ` Joe Perches
2011-10-26 23:34 ` David Rientjes
0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2011-10-26 23:14 UTC (permalink / raw)
To: David Rientjes
Cc: Wolfram Sang, linux-kernel, Jonathan Cameron, Andy Whitcroft
On Wed, 2011-10-26 at 16:08 -0700, David Rientjes wrote:
> On Wed, 26 Oct 2011, Joe Perches wrote:
> > > > The old error message was not precise. State now explicitly that the
> > > > prefix exists as a file/directory. Also change the conclusion ("appears
> > > > to be") into an instruction ("make sure").
> > > Why can't we suppress this warning unless both prefixes appear as files or
> > > directories in the cwd? ('a' and 'b' if using git format-patch)
> > I sent a suggested patch that checked $root/$patched_file
> > https://lkml.org/lkml/2011/10/19/257
> So we need a combination of the two approaches then, it makes sense to
> only emit the warning if the patched file exists in both prefixes.
I don't think so. What about something like:
$ diff -urN kernel/foo~ kernel/foo > patch
I think we should only care about the patched file,.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-26 23:14 ` Joe Perches
@ 2011-10-26 23:34 ` David Rientjes
2011-10-26 23:44 ` Joe Perches
0 siblings, 1 reply; 10+ messages in thread
From: David Rientjes @ 2011-10-26 23:34 UTC (permalink / raw)
To: Joe Perches; +Cc: Wolfram Sang, linux-kernel, Jonathan Cameron, Andy Whitcroft
On Wed, 26 Oct 2011, Joe Perches wrote:
> > So we need a combination of the two approaches then, it makes sense to
> > only emit the warning if the patched file exists in both prefixes.
>
> I don't think so. What about something like:
>
> $ diff -urN kernel/foo~ kernel/foo > patch
>
> I think we should only care about the patched file,.
>
I mean it only makes sense if both prefixes exist (otherwise patch and
git-apply will assume it's not a -p0 patch).
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-26 23:34 ` David Rientjes
@ 2011-10-26 23:44 ` Joe Perches
2011-10-27 20:11 ` David Rientjes
0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2011-10-26 23:44 UTC (permalink / raw)
To: David Rientjes
Cc: Wolfram Sang, linux-kernel, Jonathan Cameron, Andy Whitcroft
On Wed, 2011-10-26 at 16:34 -0700, David Rientjes wrote:
> On Wed, 26 Oct 2011, Joe Perches wrote:
> > > So we need a combination of the two approaches then, it makes sense to
> > > only emit the warning if the patched file exists in both prefixes.
> > I don't think so. What about something like:
> > $ diff -urN kernel/foo~ kernel/foo > patch
> > I think we should only care about the patched file,.
> I mean it only makes sense if both prefixes exist (otherwise patch and
> git-apply will assume it's not a -p0 patch).
I think we should not care about the prefixes at all,
only whether or not the patched file exists.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-26 23:44 ` Joe Perches
@ 2011-10-27 20:11 ` David Rientjes
2011-10-27 20:29 ` Joe Perches
0 siblings, 1 reply; 10+ messages in thread
From: David Rientjes @ 2011-10-27 20:11 UTC (permalink / raw)
To: Joe Perches; +Cc: Wolfram Sang, linux-kernel, Jonathan Cameron, Andy Whitcroft
On Wed, 26 Oct 2011, Joe Perches wrote:
> > I mean it only makes sense if both prefixes exist (otherwise patch and
> > git-apply will assume it's not a -p0 patch).
>
> I think we should not care about the prefixes at all,
> only whether or not the patched file exists.
>
Nack, there's nothing wrong with storing original files that you're
modifying in a subdirectory with a name of your choice in the kernel tree.
It doesn't imply a -p0 patch unless both prefixes appear and that's the
best indication that it appears in both the patch author and patch
applier's tree whereas the file being modified is ambiguous.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-27 20:11 ` David Rientjes
@ 2011-10-27 20:29 ` Joe Perches
2011-10-27 21:01 ` David Rientjes
0 siblings, 1 reply; 10+ messages in thread
From: Joe Perches @ 2011-10-27 20:29 UTC (permalink / raw)
To: David Rientjes
Cc: Wolfram Sang, linux-kernel, Jonathan Cameron, Andy Whitcroft
On Thu, 2011-10-27 at 13:11 -0700, David Rientjes wrote:
> On Wed, 26 Oct 2011, Joe Perches wrote:
> > > I mean it only makes sense if both prefixes exist (otherwise patch and
> > > git-apply will assume it's not a -p0 patch).
> > I think we should not care about the prefixes at all,
> > only whether or not the patched file exists.
> Nack,
Hi David.
It might be better if you would submit patches
to checkpatch before you nack others.
How about you track the --- and +++ lines and
submit a suggested patch yourself?
> there's nothing wrong with storing original files that you're
> modifying in a subdirectory with a name of your choice in the kernel tree.
Just as there's nothing wrong with storing original
and modified versions of subdirectories too.
> It doesn't imply a -p0 patch unless both prefixes appear and that's the
> best indication that it appears in both the patch author and patch
> applier's tree whereas the file being modified is ambiguous.
There's no single perfect test and it's just a silly
warning anyway.
I think the most common case is the direct editing of
a single file and production of a diff to submit as
a patch.
$ emacs <file>
# make changes, save original as ~ backup
$ diff -urN <file>~ <file> > ./foo.diff
$ make mrproper; make allyesconfig ; make
# deletes backup files
$ ./scripts/checkpatch.pl foo.diff
I didn't bother even finding out why the message
was emitted for me even though I had a b temp
directory in my tree. I just ignored it.
cheers, Joe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] checkpatch: improve error message for p1-check
2011-10-27 20:29 ` Joe Perches
@ 2011-10-27 21:01 ` David Rientjes
0 siblings, 0 replies; 10+ messages in thread
From: David Rientjes @ 2011-10-27 21:01 UTC (permalink / raw)
To: Joe Perches; +Cc: Wolfram Sang, linux-kernel, Jonathan Cameron, Andy Whitcroft
On Thu, 27 Oct 2011, Joe Perches wrote:
> It might be better if you would submit patches
> to checkpatch before you nack others.
>
A patch without a changelog or sign off is not up for consideration to
merge.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-10-27 21:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-19 12:33 [PATCH] checkpatch: improve error message for p1-check Wolfram Sang
2011-10-26 20:30 ` David Rientjes
2011-10-26 22:51 ` Joe Perches
2011-10-26 23:08 ` David Rientjes
2011-10-26 23:14 ` Joe Perches
2011-10-26 23:34 ` David Rientjes
2011-10-26 23:44 ` Joe Perches
2011-10-27 20:11 ` David Rientjes
2011-10-27 20:29 ` Joe Perches
2011-10-27 21:01 ` David Rientjes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).