On Mon, May 20, 2024 at 12:16 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> John Paul Adrian Glaubitz <glaubitz@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
> > ---
> > diff --git 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 was also wondering about `||`; it looks like a typo.
More importantly, though, we can simplify the pattern to:
/^(processor|CPU)[\s\d]*:/
which is much easier to comprehend and gives correct results from the
SPARC /proc/cpuinfo output.
> 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.
I'm fine with a well-focused patch which just fixes the reported
problem; the "reasonable fallback" change can be layered atop at any
time. But, of course, if Adrian wants to tackle it as part of this
series, I would not object.
> diff --git 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;
I had a more all-inclusive change in mind. These number-of-cpu checks
are in order from least to most costly but they are not necessarily
mutually exclusive. As such, my thinking was that the logic would fall
through to the next check if the preceding check produced zero or
nonsense.
