[PATCH] chainlint.pl: fix /proc/cpuinfo regexp

[PATCH] chainlint.pl: fix /proc/cpuinfo regexp

[Andreas Hasenack via GitGitGadget]

From: Andreas Hasenack <andreas.hasenack@canonical.com>

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 <andreas.hasenack@canonical.com>
---
    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

Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp

[Eric Sunshine]

On Tue, Nov 22, 2022 at 3:29 PM Andreas Hasenack via GitGitGadget
<gitgitgadget@gmail.com> 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 <andreas.hasenack@canonical.com>
> ---
> 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/

Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> 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.

Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp

[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.

On Tue, Nov 22, 2022 at 10:21 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Eric Sunshine <sunshine@sunshineco.com> 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.

Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp

[Eric Sunshine]

On Wed, Nov 23, 2022 at 7:17 AM Andreas Hasenack <andreas@canonical.com> 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

Re: [PATCH] chainlint.pl: fix /proc/cpuinfo regexp

[Junio C Hamano]

Andreas Hasenack <andreas@canonical.com> 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.