Re: [PATCH] chainlint.pl: Extend regexp pattern for /proc/cpuinfo on Linux SPARC

[John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>]

Hi Junio,

On Mon, 2024-05-20 at 09:16 -0700, Junio C Hamano wrote:
> John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:
> 
> > 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.

> 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/

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913