The ACPI test data check needs to analyse a list of all files in a
commit, so can use the new hook for processing the file list.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
scripts/checkpatch.pl | 61 ++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b74391e63a..6a7b543ddf 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1330,29 +1330,6 @@ sub WARN {
}
}
-# According to tests/qtest/bios-tables-test.c: do not
-# change expected file in the same commit with adding test
-sub checkfilename {
- my ($name, $acpi_testexpected, $acpi_nontestexpected) = @_;
-
- # Note: shell script that rebuilds the expected files is in the same
- # directory as files themselves.
- # Note: allowed diff list can be changed both when changing expected
- # files and when changing tests.
- if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) {
- $$acpi_testexpected = $name;
- } elsif ($name !~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
- $$acpi_nontestexpected = $name;
- }
- if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
- ERROR("Do not add expected files together with tests, " .
- "follow instructions in " .
- "tests/qtest/bios-tables-test.c: both " .
- $$acpi_testexpected . " and " .
- $$acpi_nontestexpected . " found\n");
- }
-}
-
sub checkspdx {
my ($file, $expr) = @_;
@@ -1437,6 +1414,34 @@ sub checkspdx {
# real filenames that were seen in the patch
sub process_file_list {
my @fileinfos = @_;
+
+ # According to tests/qtest/bios-tables-test.c: do not
+ # change expected file in the same commit with adding test
+ my @acpi_testexpected;
+ my @acpi_nontestexpected;
+
+ foreach my $fileinfo (@fileinfos) {
+ # Note: shell script that rebuilds the expected files is in
+ # the same directory as files themselves.
+ # Note: allowed diff list can be changed both when changing
+ # expected files and when changing tests.
+ if ($fileinfo->{filenew} =~ m#^tests/data/acpi/# &&
+ $fileinfo->{filenew} !~ m#^\.sh$#) {
+ push @acpi_testexpected, $fileinfo->{filenew};
+ } elsif ($fileinfo->{filenew} !~
+ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
+ push @acpi_nontestexpected, $fileinfo->{filenew};
+ }
+ }
+ if (int(@acpi_testexpected) > 0 and int(@acpi_nontestexpected) > 0) {
+ ERROR("Do not add expected files together with tests, " .
+ "follow instructions in " .
+ "tests/qtest/bios-tables-test.c. Files\n\n " .
+ join("\n ", @acpi_testexpected) .
+ "\n\nand\n\n " .
+ join("\n ", @acpi_nontestexpected) .
+ "\n\nfound in the same patch\n");
+ }
}
# Called at the start of processing a diff hunk for a file
@@ -1501,9 +1506,6 @@ sub process {
my %suppress_whiletrailers;
my %suppress_export;
- my $acpi_testexpected;
- my $acpi_nontestexpected;
-
# Pre-scan the patch sanitizing the lines.
sanitise_line_reset();
@@ -1642,7 +1644,6 @@ sub process {
$fileold =~ s@^([^/]*)/@@ if (!$file);
$filenew =~ s@^([^/]*)/@@ if (!$file);
$realfile = $filenew;
- checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);
$fileinfo = {
"isgit" => 1,
@@ -1676,8 +1677,6 @@ sub process {
$realfile = $1;
$realfile =~ s@^([^/]*)/@@ if (!$file);
- checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);
-
$p1_prefix = $1;
if (!$file && $tree && $p1_prefix ne '' &&
-e "$root/$p1_prefix") {
@@ -1770,9 +1769,7 @@ sub process {
$line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ ||
($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
(defined($1) || defined($2)))) &&
- !(($realfile ne '') &&
- defined($acpi_testexpected) &&
- ($realfile eq $acpi_testexpected))) {
+ $realfile !~ m#^tests/data/acpi/#) {
$reported_maintainer_file = 1;
WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
}
--
2.49.0
On Thu, 15 May 2025 at 14:59, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The ACPI test data check needs to analyse a list of all files in a
> commit, so can use the new hook for processing the file list.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> scripts/checkpatch.pl | 61 ++++++++++++++++++++-----------------------
> 1 file changed, 29 insertions(+), 32 deletions(-)
> @@ -1770,9 +1769,7 @@ sub process {
> $line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ ||
> ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
> (defined($1) || defined($2)))) &&
> - !(($realfile ne '') &&
> - defined($acpi_testexpected) &&
> - ($realfile eq $acpi_testexpected))) {
> + $realfile !~ m#^tests/data/acpi/#) {
Is the indentation off on this line?
> $reported_maintainer_file = 1;
> WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
> }
> --
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
On Mon, May 19, 2025 at 01:29:14PM +0100, Peter Maydell wrote:
> On Thu, 15 May 2025 at 14:59, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > The ACPI test data check needs to analyse a list of all files in a
> > commit, so can use the new hook for processing the file list.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > scripts/checkpatch.pl | 61 ++++++++++++++++++++-----------------------
> > 1 file changed, 29 insertions(+), 32 deletions(-)
>
>
>
> > @@ -1770,9 +1769,7 @@ sub process {
> > $line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ ||
> > ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
> > (defined($1) || defined($2)))) &&
> > - !(($realfile ne '') &&
> > - defined($acpi_testexpected) &&
> > - ($realfile eq $acpi_testexpected))) {
> > + $realfile !~ m#^tests/data/acpi/#) {
>
> Is the indentation off on this line?
It looks like it from this diff, but it is actually correct, as it was
moved outside the inner two sets of brackets.
>
> > $reported_maintainer_file = 1;
> > WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
> > }
> > --
>
> Otherwise
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> thanks
> -- PMM
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 5/15/25 15:59, Daniel P. Berrangé wrote:
> The ACPI test data check needs to analyse a list of all files in a
> commit, so can use the new hook for processing the file list.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>> ---
> scripts/checkpatch.pl | 61 ++++++++++++++++++++-----------------------
> 1 file changed, 29 insertions(+), 32 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b74391e63a..6a7b543ddf 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1330,29 +1330,6 @@ sub WARN {
> }
> }
>
> -# According to tests/qtest/bios-tables-test.c: do not
> -# change expected file in the same commit with adding test
> -sub checkfilename {
> - my ($name, $acpi_testexpected, $acpi_nontestexpected) = @_;
> -
> - # Note: shell script that rebuilds the expected files is in the same
> - # directory as files themselves.
> - # Note: allowed diff list can be changed both when changing expected
> - # files and when changing tests.
> - if ($name =~ m#^tests/data/acpi/# and not $name =~ m#^\.sh$#) {
> - $$acpi_testexpected = $name;
> - } elsif ($name !~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> - $$acpi_nontestexpected = $name;
> - }
> - if (defined $$acpi_testexpected and defined $$acpi_nontestexpected) {
> - ERROR("Do not add expected files together with tests, " .
> - "follow instructions in " .
> - "tests/qtest/bios-tables-test.c: both " .
> - $$acpi_testexpected . " and " .
> - $$acpi_nontestexpected . " found\n");
> - }
> -}
> -
> sub checkspdx {
> my ($file, $expr) = @_;
>
> @@ -1437,6 +1414,34 @@ sub checkspdx {
> # real filenames that were seen in the patch
> sub process_file_list {
> my @fileinfos = @_;
> +
> + # According to tests/qtest/bios-tables-test.c: do not
> + # change expected file in the same commit with adding test
> + my @acpi_testexpected;
> + my @acpi_nontestexpected;
> +
> + foreach my $fileinfo (@fileinfos) {
> + # Note: shell script that rebuilds the expected files is in
> + # the same directory as files themselves.
> + # Note: allowed diff list can be changed both when changing
> + # expected files and when changing tests.
> + if ($fileinfo->{filenew} =~ m#^tests/data/acpi/# &&
> + $fileinfo->{filenew} !~ m#^\.sh$#) {
> + push @acpi_testexpected, $fileinfo->{filenew};
> + } elsif ($fileinfo->{filenew} !~
> + m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
> + push @acpi_nontestexpected, $fileinfo->{filenew};
> + }
> + }
> + if (int(@acpi_testexpected) > 0 and int(@acpi_nontestexpected) > 0) {
> + ERROR("Do not add expected files together with tests, " .
> + "follow instructions in " .
> + "tests/qtest/bios-tables-test.c. Files\n\n " .
> + join("\n ", @acpi_testexpected) .
> + "\n\nand\n\n " .
> + join("\n ", @acpi_nontestexpected) .
> + "\n\nfound in the same patch\n");
> + }
> }
>
> # Called at the start of processing a diff hunk for a file
> @@ -1501,9 +1506,6 @@ sub process {
> my %suppress_whiletrailers;
> my %suppress_export;
>
> - my $acpi_testexpected;
> - my $acpi_nontestexpected;
> -
> # Pre-scan the patch sanitizing the lines.
>
> sanitise_line_reset();
> @@ -1642,7 +1644,6 @@ sub process {
> $fileold =~ s@^([^/]*)/@@ if (!$file);
> $filenew =~ s@^([^/]*)/@@ if (!$file);
> $realfile = $filenew;
> - checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);
>
> $fileinfo = {
> "isgit" => 1,
> @@ -1676,8 +1677,6 @@ sub process {
> $realfile = $1;
> $realfile =~ s@^([^/]*)/@@ if (!$file);
>
> - checkfilename($realfile, \$acpi_testexpected, \$acpi_nontestexpected);
> -
> $p1_prefix = $1;
> if (!$file && $tree && $p1_prefix ne '' &&
> -e "$root/$p1_prefix") {
> @@ -1770,9 +1769,7 @@ sub process {
> $line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ ||
> ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
> (defined($1) || defined($2)))) &&
> - !(($realfile ne '') &&
> - defined($acpi_testexpected) &&
> - ($realfile eq $acpi_testexpected))) {
> + $realfile !~ m#^tests/data/acpi/#) {
> $reported_maintainer_file = 1;
> WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
> }
* https://lore.kernel.org/qemu-devel/20250403151829.44858-12-philmd@linaro.org/
WARNING: Does new file 'tests/data/acpi/aarch64/virt/APIC.its_off' need 'SPDX-License-Identifier'?
WARNING: Does new file 'tests/data/acpi/aarch64/virt/FACP.its_off' need 'SPDX-License-Identifier'?
WARNING: Does new file 'tests/data/acpi/aarch64/virt/IORT.its_off' need 'SPDX-License-Identifier'?
ERROR: Do not add expected files together with tests, follow instructions in tests/qtest/bios-tables-test.c. Files
tests/data/acpi/aarch64/virt/APIC.its_off
tests/data/acpi/aarch64/virt/FACP.its_off
tests/data/acpi/aarch64/virt/IORT.its_off
and
tests/qtest/bios-tables-test.c
found in the same patch
total: 1 errors, 3 warnings, 11 lines checked
Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
© 2016 - 2025 Red Hat, Inc.