chainlint.pl: fix /proc/cpuinfo regexp
PATCH 1/1 chainlint.pl: fix /proc/cpuinfo regexp
From: Andreas Hasenack <[email protected]>
git commit 29fb2ec384a867ca577335a12f4b45c184e7b642 introduced a function that gets the number of cores from /proc/cpuinfo on some systems, notably linux.
The regexp it uses (^processor\s*:) fails to match the desired lines in the s390x architecture, where they look like this:
processor 0: version = FF, identification = 148F67, machine = 2964
As a result, on s390x that function returns 0 as the number of cores, and the chainlint.pl script exits without doing anything.
---
chainlint.pl: fix /proc/cpuinfo regexp
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1385%2Fpanlinux%2Fupstream-fix-cpuinfo-regexp-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1385/panlinux/upstream-fix-cpuinfo-regexp-v1
Pull-Request: https://github.com/git/git/pull/1385
t/chainlint.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
t/chainlint.pl +1 -1
@@ -656,7 +656,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*:/, <>)); } if -r '/proc/cpuinfo'; + do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor[\s\d]*:/, <>)); } if -r '/proc/cpuinfo'; # macOS & BSD return qx/sysctl -n hw.ncpu/ if $^O =~ /(?:^darwin$|bsd)/; return 1;
base-commit: a0789512c5a4ae7da935cd2e419f253cb3cb4ce7
Signature · 1 line
gitgitgadget
Quoted 3 lines · Andreas Hasenack via GitGitGadget
git commit
29fb2ec384a867ca577335a12f4b45c184e7b642introduced a function that gets the number of cores from/proc/cpuinfoon some systems, notably linux.
Thanks for diagnosing this problem and quickly submitting a patch.
I forgot to mention earlier that we typically cite other commits like this:
29fb2ec384 (chainlint.pl: validate test scripts in parallel, 2022-09-01)
which you can generate automatically with:
git log --pretty=reference -1 29fb2ec384
Quoted 7 lines
The regexp it uses (^processor\s*:) fails to match the desired lines in the s390x architecture, where they look like this:
processor 0: version = FF, identification = 148F67, machine = 2964
As a result, on s390x that function returns 0 as the number of cores, and the chainlint.pl script exits without doing anything.
Makes sense. Well explained.
A separate problem is that chainlint.pl doesn't fall back to a sensible non-zero value if ncores() returns 0 (or some other nonsense value). That is, of course, outside the scope of the well-focused problem fix which this standalone patch addresses. I may end up submitting a fix separately to make it fall back sensibly.
t/chainlint.pl sub ncores { · line 662
Quoted 12 lines
Signed-off-by Andreas Hasenack <[email protected]>---
t/chainlint.pl +1
-1@@ -656,7 +656,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*:/, <>)); } if -r '/proc/cpuinfo'; + do { local @ARGV='/proc/cpuinfo'; return scalar(grep(/^processor[\s\d]*:/, <>)); } if -r '/proc/cpuinfo'; # macOS & BSD return qx/sysctl -n hw.ncpu/ if $^O =~ /(?:^darwin$|bsd)/; return 1;
As mentioned elsewhere[1], this code may eventually be dropped altogether, but this fix is good to have in the meantime. Thanks.
[1]: https://lore.kernel.org/git/CAPig+cQrXb-YUSzmfgJ2PRoiOP3goVACRCrX9C39kf3oDH+BHg@mail.gmail.com/
Quoted 10 lines · Eric Sunshine
Makes sense. Well explained.
A separate problem is that chainlint.pl doesn't fall back to a sensible non-zero value if
ncores()returns 0 (or some other nonsense value). That is, of course, outside the scope of the well-focused problem fix which this standalone patch addresses. I may end up submitting a fix separately to make it fall back sensibly.
...
As mentioned elsewhere[1], this code may eventually be dropped altogether, but this fix is good to have in the meantime. Thanks.
Thanks, both. Will apply.
Should I still change the commit message regarding where I mention the commit that introduced this, as explained by Eric? From the activity I saw overnight it looks like things were already picked up and it doesn't matter anymore.
Quoted 15 lines · Junio C Hamano
Quoted 10 lines · Eric Sunshine
Makes sense. Well explained.
A separate problem is that chainlint.pl doesn't fall back to a sensible non-zero value if
ncores()returns 0 (or some other nonsense value). That is, of course, outside the scope of the well-focused problem fix which this standalone patch addresses. I may end up submitting a fix separately to make it fall back sensibly.
...
As mentioned elsewhere[1], this code may eventually be dropped altogether, but this fix is good to have in the meantime. Thanks.Thanks, both. Will apply.
Quoted 4 lines · Andreas Hasenack
Should I still change the commit message regarding where I mention the commit that introduced this, as explained by Eric? From the activity I saw overnight it looks like things were already picked up and it doesn't matter anymore.
No need to resend. Junio tweaked the commit message[1] when he picked up the patch.
[1]: https://github.com/git/git/commit/1f51b77f4f3f4d05f249f80713b2969e4069c109
Quoted 4 lines · Andreas Hasenack
Should I still change the commit message regarding where I mention the commit that introduced this, as explained by Eric? From the activity I saw overnight it looks like things were already picked up and it doesn't matter anymore.
Sorry, I should have said "will apply with tweaked log message" to reduce the need for a round-trip. Please double check what is queued in 'seen' and send in replacements before it gets merged down to 'next' if there are still things you may want to update. If you are happy with what is in 'seen', no further action is needed at least for now.
Thanks.