Re: [PATCH] chainlint.pl: Extend regexp pattern for /proc/cpuinfo on Linux SPARC
From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Date: 2024-05-20 16:48:50
Hi Junio, On Mon, 2024-05-20 at 09:16 -0700, Junio C Hamano wrote:
John Paul Adrian Glaubitz [off-list ref] writes:quoted
On SPARC systems running Linux, individual processors are denoted with "CPUnn:" in /proc/cpuinfo instead of the usual "processor NN:" so that the current regexp in ncores() returns 0. Extend the regexp to match lines with "CPUnn:" as well to properly detect the number of available cores on these systems. Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> --- t/chainlint.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/t/chainlint.pl b/t/chainlint.pl index 556ee91a15..63cac942ac 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl@@ -718,7 +718,7 @@ sub ncores { # Windows return $ENV{NUMBER_OF_PROCESSORS} if exists($ENV{NUMBER_OF_PROCESSORS}); # Linux / MSYS2 / Cygwin / WSL - do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor[\s\d]*:/, <>)); } if -r '/proc/cpuinfo'; + do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor[\s\d]*:||^CPU[\d]*:/, <>)); } if -r '/proc/cpuinfo';Is the doubled || intended? Doesn't it introduce an empty pattern that slurps every single line of /proc/cpuinfo?
I'm not a Perl expert by any means, so I wasn't sure what the correct logical OR operator would be. If it turns out to be wrong, let's fix that.
quoted hunk ↗ jump to hunk
I was wondering if we want to first add the "reasonable fallback" Eric mentioned ealier, and then build on top, whose result may look like the attached. You can enable the STDERR thing with your double "||" added back and see what "cd t && perl chainlint.pl" produces. Thanks.diff --git i/t/chainlint.pl w/t/chainlint.pl index 556ee91a15..775f06281b 100755 --- i/t/chainlint.pl +++ w/t/chainlint.pl@@ -718,7 +718,13 @@ sub ncores { # Windows return $ENV{NUMBER_OF_PROCESSORS} if exists($ENV{NUMBER_OF_PROCESSORS}); # Linux / MSYS2 / Cygwin / WSL - do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor[\s\d]*:/, <>)); } if -r '/proc/cpuinfo'; + do { + local @ARGV='/proc/cpuinfo'; + my @num = grep(/^processor[\s\d]*:|^CPU[\d]*:/, <>); +# print STDERR "FOUND <@num>\n"; + return 1 if (!@num); + return scalar(@num); + } if -r '/proc/cpuinfo'; # macOS & BSD return qx/sysctl -n hw.ncpu/ if $^O =~ /(?:^darwin$|bsd)/; return 1;
This seems to work fine for me as well. If you post it as a patch, I'm more than happy to give it a Tested-By. Btw, it would be great if this could be extended to support the output for the Alpha architecture as well since the testsuite fails the same way [1]. The output for /proc/cpuinfo looks like this [2]: (alpha-chroot)root@p100:/# cat /proc/cpuinfo cpu : Alpha cpu model : ev67 cpu variation : 0 cpu revision : 0 cpu serial number : JA00000000 system type : QEMU system variation : QEMU_v8.0.92 system revision : 0 system serial number : AY00000000 cycle frequency [Hz] : 250000000 timer frequency [Hz] : 250.00 page size [bytes] : 8192 phys. address bits : 44 max. addr. space # : 255 BogoMIPS : 2500.00 platform string : AlphaServer QEMU user-mode VM cpus detected : 8 cpus active : 4 cpu active mask : 0000000000000095 L1 Icache : n/a L1 Dcache : n/a L2 cache : n/a L3 cache : n/a Thanks so much for helping with the fix! Adrian
[1] https://buildd.debian.org/status/fetch.php?pkg=git&arch=alpha&ver=1%3A2.45.1-1&stamp=1716194983&raw=0 [2] https://lore.kernel.org/all/20230901204251.137307-4-richard.henderson@linaro.org/ (local)
-- .''`. John Paul Adrian Glaubitz : :' : Debian Developer `. `' Physicist `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913