On Mon, May 20, 2024 at 12:16 PM Junio C Hamano [off-list ref] 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>
---

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.

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.