Messages in this thread | | | Subject | Re: [PATCH] checkpatch: warn for p0 patch only if prefix is not b | From | Dafna Hirschfeld <> | Date | Tue, 11 Jan 2022 14:06:27 +0200 |
| |
On 11.01.22 13:27, Lukas Bulwahn wrote: > On Tue, Jan 11, 2022 at 11:47 AM Dafna Hirschfeld > <dafna.hirschfeld@collabora.com> wrote: >> >> >> >> On 11.01.22 12:18, Lukas Bulwahn wrote: >>> Dafna, >>> >>> On Mon, Jan 10, 2022 at 9:06 PM Dafna Hirschfeld >>> <dafna.hirschfeld@collabora.com> wrote: >>>> >>>> It might be that file 'b' happens to exit. In that >>>> case, if the prefix is also 'b' (which is the >>>> common case) we get the falsely warning: >>>> >>>> patch prefix 'b' exists, appears to be a -p0 patch >>>> >>>> So warn only if prefix is not 'b' >>>> >>> >>> The checkpatch script that is maintained here is really only intended >>> for its use in the kernel development. You may use checkpatch anywhere >>> else, but any changes that increase complexity for those other use >>> cases is really difficult to argue for inclusion in the kernel >>> repository. The checkpatch script currently is already large and >>> complex enough and all rules need to be understood as rough >>> heuristics, not as strict rules. >>> >>> So, can you point to a kernel repository where there is actually a >>> file 'b' included? On a quick scan, I could not find a file 'b' in the >>> current trees of the repositories on my machine. >>> >>> I am just letting you know about what I have observed; I do not decide >>> on the inclusion of this patch, though. >> >> Hi, a 'b' file might make it to the source folder as an untracked file. >> This actually happened to me since I was too lazy to give it a meaningful name. >> Then I got this warning and it took me some time to figure out what is the problem. >> > > Well, but you run checkpatch.pl on a patch, right? So, you need to add > the file explicitly with git (where you notice adding a file called b, > which probably is really not a good name), you create a git commit > (where that is pointed out again), then create a patch from that > (which you may manually look at again) and then run checkpatch.pl > before you submit it (again, submitting a patch with a file 'b' is > probably a good reason to rethink your submission).
Hi , no, the 'b' file is left untracked. It is easily reproducible, inside a kernel source repo do:
touch b
# These two commands will both trigger the warning. ./scripts/checkpatch.pl --strict -g HEAD ./scripts/checkpatch.pl --strict 0001-some-random-patch.patch
> > If it helps, you can add some documentation on the PATCH_PREFIX rule > in the checkpatch documentation at > ./Documentation/dev-tools/checkpatch.rst. Especially, you can note the > situation you encountered there, e.g., that adding files with explicit > name 'a' or 'b' may make this rule trigger. If that documentation of > the rule is helpful, I will ack that documentation patch and request > inclusion of it.
I can do that as well if you think it is better.
Thanks, Dafna
> > Lukas > >> Thanks, >> Dafna >> >>> >>> Lukas >>> >>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >>>> --- >>>> scripts/checkpatch.pl | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >>>> index 1784921c645d..72263b142e39 100755 >>>> --- a/scripts/checkpatch.pl >>>> +++ b/scripts/checkpatch.pl >>>> @@ -2821,7 +2821,7 @@ sub process { >>>> $in_commit_log = 0; >>>> >>>> $p1_prefix = $1; >>>> - if (!$file && $tree && $p1_prefix ne '' && >>>> + if (!$file && $tree && $p1_prefix ne '' && $p1_prefix ne 'b' && >>>> -e "$root/$p1_prefix") { >>>> WARN("PATCH_PREFIX", >>>> "patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n"); >>> >>>> -- >>>> 2.17.1 >>>>
| |