Messages in this thread Patches in this message | | | From | Claudio Fontana <> | Subject | in_commit_log: is the wrong token being matched for the MAINTAINERS need updating message? | Date | Mon, 10 Aug 2020 18:45:52 +0200 |
| |
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
| |