* in_commit_log: is the wrong token being matched for the MAINTAINERS need updating message?
@ 2020-08-10 16:45 Claudio Fontana
2020-08-10 16:59 ` Joe Perches
0 siblings, 1 reply; 2+ messages in thread
From: Claudio Fontana @ 2020-08-10 16:45 UTC (permalink / raw)
To: Andy Whitcroft, Joe Perches; +Cc: linux-kernel
Hi,
while working on a downstream version of the checkpatch.pl script used for qemu,
I experienced a "bug" that I traced back to the kernel upstream version of checkpatch.pl, and hence the question:
what is the exact meaning of the variable in_commit_log, is it supposed to be including the git diff stats or not?
For example, lets take this sample useless patch as input:
-------- cut --------
From 1873ba53039d8824e37452dc39af79b4b0327085 Mon Sep 17 00:00:00 2001
From: Claudio Fontana <cfontana@suse.de>
Date: Mon, 10 Aug 2020 15:36:47 +0200
Subject: [PATCH] XXX test patch moving file
Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
sound/Makefile | 2 +-
sound/{ => ac97}/ac97_bus.c | 0
2 files changed, 1 insertion(+), 1 deletion(-)
rename sound/{ => ac97}/ac97_bus.c (100%)
diff --git a/sound/Makefile b/sound/Makefile
index 797ecdcd35e2..d7a08163ea1a 100644
--- a/sound/Makefile
+++ b/sound/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_SND) += core/ i2c/ drivers/ isa/ pci/ ppc/ arm/ sh/ synth/ usb/ \
obj-$(CONFIG_SND_AOA) += aoa/
# This one must be compilable even if sound is configured out
-obj-$(CONFIG_AC97_BUS) += ac97_bus.o
+obj-$(CONFIG_AC97_BUS) += ac97/ac97_bus.o
obj-$(CONFIG_AC97_BUS_NEW) += ac97/
ifeq ($(CONFIG_SND),y)
diff --git a/sound/ac97_bus.c b/sound/ac97/ac97_bus.c
similarity index 100%
rename from sound/ac97_bus.c
rename to sound/ac97/ac97_bus.c
--
2.16.4
-------- cut --------
and using this simple patch to highlight the issue:
-------- cut --------
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 599b8c4933a7..e87d5d00a9cd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2876,16 +2876,17 @@ sub process {
}
}
+ print $line . "\n";
# Check for added, moved or deleted files
if (!$reported_maintainer_file && !$in_commit_log &&
($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||
$line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ ||
($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
(defined($1) || defined($2))))) {
+ print("1: $1 2: $2\n");
$is_patch = 1;
$reported_maintainer_file = 1;
- WARN("FILE_PATH_CHANGES",
- "added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
+ print("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
}
# Check for adding new DT bindings not in schema format
-------- cut --------
We can see that in the sample patch input we are matching the line
sound/{ => ac97}/ac97_bus.c | 0
with the regex ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/
Is this actually wanted?
I would expect that in this case we want instead to match the lines:
rename from sound/ac97_bus.c
rename to sound/ac97/ac97_bus.c
It does not hurt in the kernel, but it does hurt in my downstream patch, where I try to make use of $realfile ,
which is not set if I match too early in the patch.
Would it make sense to move this chunk earlier:
# Check if it's the start of a commit log
# (not a header line and we haven't seen the patch filename)
if ($in_header_lines && $realfile =~ /^$/ &&
!($rawline =~ /^\s+(?:\S|$)/ ||
$rawline =~ /^(?:commit\b|from\b|[\w-]+:)/i)) {
$in_header_lines = 0;
$in_commit_log = 1;
$has_commit_log = 1;
}
so that the check:
if (!$reported_maintainer_file && !$in_commit_log
fails for the testcase shown (in_commit_log ends up as true), and the match will happen later, where realfile is set and available for use?
Thanks!
Claudio
--
Claudio Fontana
Engineering Manager Virtualization, SUSE Labs Core
SUSE Software Solutions Italy Srl
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: in_commit_log: is the wrong token being matched for the MAINTAINERS need updating message?
2020-08-10 16:45 in_commit_log: is the wrong token being matched for the MAINTAINERS need updating message? Claudio Fontana
@ 2020-08-10 16:59 ` Joe Perches
0 siblings, 0 replies; 2+ messages in thread
From: Joe Perches @ 2020-08-10 16:59 UTC (permalink / raw)
To: Claudio Fontana, Andy Whitcroft; +Cc: linux-kernel
On Mon, 2020-08-10 at 18:45 +0200, Claudio Fontana wrote:
> We can see that in the sample patch input we are matching the line
>
> sound/{ => ac97}/ac97_bus.c | 0
>
> with the regex ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/
>
> Is this actually wanted?
Yes, If this line is found, it's not intended to
be part of a commit message.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-08-10 16:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-10 16:45 in_commit_log: is the wrong token being matched for the MAINTAINERS need updating message? Claudio Fontana
2020-08-10 16:59 ` Joe Perches
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).