On Sat 2024-12-21 01:05:35, Siddharth Menon wrote:
> This patch adds a script to validate that the current kernel configuration
> satisfies the requirements for selftests. The script compares the current
> kernel configs against the required selftest configs.
>
> A config mismatch exits with error value 1 while matching configs or missing
> config files exit with value 0.In order to get debug output, set the
^
missing space before the next sentence
> environment variable LOCALMODCONFIG_DEBUG=1.
I would prefer to print the missing dependencies by default.
Otherwise, it is far from clear why the test was skipped.
Also LOCALMODCONFIG_DEBUG looks like a pretty non-standard way to get
debug output. People will have troubles to discover this way
and memorize the long name.
> diff --git a/tools/testing/selftests/mktest.pl b/tools/testing/selftests/mktest.pl
> new file mode 100755
> index 000000000000..60462f323bde
> --- /dev/null
> +++ b/tools/testing/selftests/mktest.pl
I would give it a more meaningful name.
For example, using the same name as for the makefile target
just with .pl suffix. I mean "check_config_deps.pl".
That said, I have just realized that that there already was
"kselftest_deps.sh". It seems to check missing compile dependencies
by scanning various variants of LDLIBS variables.
It seems that "kselftest_deps.sh" should be called separately.
I mean that it is not integrated with the make files.
It is a bit strange to handle two types of dependencies differently.
It would be nice somehow consolidate the two check scripts
so that they behave similar way and use a similar name.
I personally like the integration with the Makefile.
> @@ -0,0 +1,138 @@
> +#!/usr/bin/env perl
> +# SPDX-License-Identifier: GPL-2.0
> +use warnings;
> +use strict;
> +use Getopt::Long;
> +use File::Spec;
> +
> +# set the environment variable LOCALMODCONFIG_DEBUG to get
> +# debug output.
> +my $debugprint = 0;
> +$debugprint = 1 if (defined($ENV{LOCALMODCONFIG_DEBUG}));
> +
> +sub dprint {
> + return if (!$debugprint);
> + print STDERR @_;
> +}
> +
> +my $uname = `uname -r`;
> +chomp $uname;
> +
> +my @searchconfigs = (
> + {
> + "file" => ".config",
> + "exec" => "cat",
Nit: I would prefer to use a tab instead of 8 spaces. At least, the 8 spaces
are considered a bad formatting in the kernel .c sources.
> + },
> + {
> + "file" => "/proc/config.gz",
> + "exec" => "zcat",
> + },
> + {
> + "file" => "/boot/config-$uname",
> + "exec" => "cat",
> + },
> + {
> + "file" => "/boot/vmlinuz-$uname",
> + "exec" => "scripts/extract-ikconfig",
> + "test" => "scripts/extract-ikconfig",
> + },
> + {
> + "file" => "vmlinux",
> + "exec" => "scripts/extract-ikconfig",
> + "test" => "scripts/extract-ikconfig",
> + },
> + {
> + "file" => "/lib/modules/$uname/kernel/kernel/configs.ko",
> + "exec" => "scripts/extract-ikconfig",
> + "test" => "scripts/extract-ikconfig",
> + },
> + {
> + "file" => "/lib/modules/$uname/build/.config",
> + "exec" => "cat",
> + },
> + {
> + "file" => "kernel/configs.ko",
> + "exec" => "scripts/extract-ikconfig",
> + "test" => "scripts/extract-ikconfig",
> + },
> + {
> + "file" => "kernel/configs.o",
> + "exec" => "scripts/extract-ikconfig",
> + "test" => "scripts/extract-ikconfig",
> + },
> +);
> +
> +sub read_config {
> + foreach my $conf (@searchconfigs) {
> + my $file = $conf->{"file"};
> +
> + next if (! -f "$file");
> +
Nit: There are 4 spaces on the above empty line. "git am" complains
about it.
I guess that also ./scripts/checkpatch.pl would complain about it.
> + if (defined($conf->{"test"})) {
> + `$conf->{"test"} $conf->{"file"} 2>/dev/null`;
> + next if ($?);
> + }
> +
same here
> + my $exec = $conf->{"exec"};
> +
and here
> + dprint "Kernel config: '$file'\n";
> +
and here.
> + open(my $infile, '-|', "$exec $file") || die "Failed to run $exec $file";
> + my @x = <$infile>;
> + close $infile;
> + return @x;
> + }
> + dprint "Unable to find kernel config file, skipping check\n";
> + exit 0;
> +}
> +
> +# Check if selftest path is provided
> +die "Usage: $0 <selftest_path>\n" unless @ARGV == 1;
> +
> +my $file_path = $ARGV[0];
> +
> +my @config_file = read_config();
> +
> +my %kern_configs;
> +my $valid = "A-Za-z_0-9";
> +foreach my $line (@config_file) {
> + chomp $line;
> + next if $line =~ /^\s*$/ || $line =~ /^#/;
> +
another spaces on empty line
> + if ($line =~ /^(CONFIG_\w+)=(.+)$/) {
> + $kern_configs{$1} = $2;
> + }
> +}
> +
> +my %test_configs;
> +# Continue as normal if /config file does not exist
trailing space on the above line.
> +open(my $fh, '<', $file_path) or exit 0;
> +
> +while (my $line = <$fh>) {
> + chomp $line;
> + next if $line =~ /^\s*$/ || $line =~ /^#/;
> +
spaces on empty line
> + if ($line =~ /^(CONFIG_\w+)=(.+)$/) {
> + $test_configs{$1} = $2;
> + }
> +}
> +close $fh;
> +
Best Regards,
Petr
