# [PATCH] chainlint.pl: fix /proc/cpuinfo regexp 6 messages from 2022-11-22 to 2022-11-23. Participants: Andreas Hasenack via GitGitGadget, Eric Sunshine, Junio C Hamano, Andreas Hasenack. Thread: https://gitlist.dev/t/58840 ## Andreas Hasenack via GitGitGadget, 2022-11-22 20:27 Subject: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp Message-ID: URL: https://gitlist.dev/e/pull.1385.git.git.1669148861635.gitgitgadget%40gmail.com ``` From: Andreas Hasenack 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. Signed-off-by: Andreas Hasenack --- 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(-) diff --git a/t/chainlint.pl b/t/chainlint.pl index 976db4b8a01..31cc086f964 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -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 -- gitgitgadget ``` ## Eric Sunshine, 2022-11-22 20:57 Subject: Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp Message-ID: URL: https://gitlist.dev/e/CAPig%2BcQ6_7wf6C280Rqi7mcTCiQp-n5GiLWTPazfcUcGFeZi0g%40mail.gmail.com In-Reply-To: ``` On Tue, Nov 22, 2022 at 3:29 PM Andreas Hasenack via GitGitGadget wrote: > git commit 29fb2ec384a867ca577335a12f4b45c184e7b642 introduced a > function that gets the number of cores from /proc/cpuinfo on 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 > 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. > Signed-off-by: Andreas Hasenack > --- > diff --git a/t/chainlint.pl b/t/chainlint.pl > @@ -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/ ``` ## Junio C Hamano, 2022-11-23 01:21 Subject: Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp Message-ID: URL: https://gitlist.dev/e/xmqqsfiao47q.fsf%40gitster.g In-Reply-To: ``` Eric Sunshine writes: > 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. ``` ## Andreas Hasenack, 2022-11-23 12:16 Subject: Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp Message-ID: URL: https://gitlist.dev/e/CANYNYEETCaaQGOXVLrRCC8wdS-uD66BHqr98Eetb%2B1GVk5WkTg%40mail.gmail.com In-Reply-To: ``` 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. On Tue, Nov 22, 2022 at 10:21 PM Junio C Hamano wrote: > > Eric Sunshine writes: > > > 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. ``` ## Eric Sunshine, 2022-11-23 15:52 Subject: Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp Message-ID: URL: https://gitlist.dev/e/CAPig%2BcSVjnrHEZxosbS1Au7LRbDRZt_r9dr78S37FPXdb3W2QA%40mail.gmail.com In-Reply-To: ``` On Wed, Nov 23, 2022 at 7:17 AM Andreas Hasenack wrote: > 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 ``` ## Junio C Hamano, 2022-11-23 23:53 Subject: Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp Message-ID: URL: https://gitlist.dev/e/xmqqfse9mdm2.fsf%40gitster.g In-Reply-To: ``` Andreas Hasenack writes: > 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. ```