If the process documented in tests/qtest/bios-tables-test.c
is followed, then same patch never touches both expected
files and code. Teach checkpatch to enforce this rule.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Peter, Igor what do you think?
scripts/checkpatch.pl | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b27e4ff5e9..96583e3fff 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -35,6 +35,8 @@ my $summary_file = 0;
my $root;
my %debug;
my $help = 0;
+my $testexpected;
+my $nontestexpected;
sub help {
my ($exitcode) = @_;
@@ -1256,6 +1258,26 @@ 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) = @_;
+ if ($name =~ m#^tests/data/acpi/# and
+ # make exception for a shell script that rebuilds the files
+ not $name =~ m#^\.sh$# or
+ $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) {
+ $testexpected = $name;
+ } else {
+ $nontestexpected = $name;
+ }
+ if (defined $testexpected and defined $nontestexpected) {
+ ERROR("Do not add expected files together with tests, " .
+ "follow instructions in " .
+ "tests/qtest/bios-tables-test.c: both " .
+ $testexpected . " and " . $nontestexpected . " found\n");
+ }
+}
+
sub process {
my $filename = shift;
@@ -1431,9 +1453,11 @@ sub process {
if ($line =~ /^diff --git.*?(\S+)$/) {
$realfile = $1;
$realfile =~ s@^([^/]*)/@@ if (!$file);
+ checkfilename($realfile);
} elsif ($line =~ /^\+\+\+\s+(\S+)/) {
$realfile = $1;
$realfile =~ s@^([^/]*)/@@ if (!$file);
+ checkfilename($realfile);
$p1_prefix = $1;
if (!$file && $tree && $p1_prefix ne '' &&
--
MST
On Sun, Mar 15, 2020 at 07:35:46AM -0400, Michael S. Tsirkin wrote: > If the process documented in tests/qtest/bios-tables-test.c > is followed, then same patch never touches both expected > files and code. Teach checkpatch to enforce this rule. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > Peter, Igor what do you think? Minor comments below: Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > scripts/checkpatch.pl | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index b27e4ff5e9..96583e3fff 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -35,6 +35,8 @@ my $summary_file = 0; > my $root; > my %debug; > my $help = 0; > +my $testexpected; > +my $nontestexpected; If you respin, please add acpi to these variable names since they are specific to acpi. > > sub help { > my ($exitcode) = @_; > @@ -1256,6 +1258,26 @@ 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) = @_; There is a tab instead of spaces here that could be fixed if you decide to respin. > + if ($name =~ m#^tests/data/acpi/# and > + # make exception for a shell script that rebuilds the files > + not $name =~ m#^\.sh$# or > + $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) { > + $testexpected = $name; > + } else { > + $nontestexpected = $name; > + } > + if (defined $testexpected and defined $nontestexpected) { > + ERROR("Do not add expected files together with tests, " . > + "follow instructions in " . > + "tests/qtest/bios-tables-test.c: both " . > + $testexpected . " and " . $nontestexpected . " found\n"); > + } > +} > + > sub process { > my $filename = shift; > > @@ -1431,9 +1453,11 @@ sub process { > if ($line =~ /^diff --git.*?(\S+)$/) { > $realfile = $1; > $realfile =~ s@^([^/]*)/@@ if (!$file); > + checkfilename($realfile); > } elsif ($line =~ /^\+\+\+\s+(\S+)/) { > $realfile = $1; > $realfile =~ s@^([^/]*)/@@ if (!$file); > + checkfilename($realfile); The surrounding lines in this hunk use tab indentation, not spaces.
On Sun, 15 Mar 2020 07:35:46 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote: > If the process documented in tests/qtest/bios-tables-test.c > is followed, then same patch never touches both expected > files and code. Teach checkpatch to enforce this rule. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Tested-by: Igor Mammedov <imammedo@redhat.com> > --- > > Peter, Igor what do you think? > > scripts/checkpatch.pl | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index b27e4ff5e9..96583e3fff 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -35,6 +35,8 @@ my $summary_file = 0; > my $root; > my %debug; > my $help = 0; > +my $testexpected; > +my $nontestexpected; > > sub help { > my ($exitcode) = @_; > @@ -1256,6 +1258,26 @@ 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) = @_; > + if ($name =~ m#^tests/data/acpi/# and > + # make exception for a shell script that rebuilds the files > + not $name =~ m#^\.sh$# or > + $name =~ m#^tests/qtest/bios-tables-test-allowed-diff.h$#) { > + $testexpected = $name; > + } else { > + $nontestexpected = $name; > + } > + if (defined $testexpected and defined $nontestexpected) { > + ERROR("Do not add expected files together with tests, " . > + "follow instructions in " . > + "tests/qtest/bios-tables-test.c: both " . > + $testexpected . " and " . $nontestexpected . " found\n"); > + } > +} > + > sub process { > my $filename = shift; > > @@ -1431,9 +1453,11 @@ sub process { > if ($line =~ /^diff --git.*?(\S+)$/) { > $realfile = $1; > $realfile =~ s@^([^/]*)/@@ if (!$file); > + checkfilename($realfile); > } elsif ($line =~ /^\+\+\+\s+(\S+)/) { > $realfile = $1; > $realfile =~ s@^([^/]*)/@@ if (!$file); > + checkfilename($realfile); > > $p1_prefix = $1; > if (!$file && $tree && $p1_prefix ne '' &&
© 2016 - 2024 Red Hat, Inc.