Request for Help - Too many perl arguments as of 2.43.0-rc0

Request for Help - Too many perl arguments as of 2.43.0-rc0

[rsbecker]

I can no longer test git as the number of arguments (and size) sent to perl
from the make infrastructure is too big for my platform. Is it possible to
use xargs instead?

Thanks,
Randall

--
Brief whoami: NonStop&UNIX developer since approximately
UNIX(421664400)
NonStop(211288444200000000)
-- In real life, I talk too much.

Re: Request for Help - Too many perl arguments as of 2.43.0-rc0

[Eric Sunshine]

On Fri, Nov 3, 2023 at 3:03 PM <rsbecker@nexbridge.com> wrote:
> I can no longer test git as the number of arguments (and size) sent to perl
> from the make infrastructure is too big for my platform. Is it possible to
> use xargs instead?

Presumably you're talking about the invocation of chainlint.pl?
Considering that that's quite an unimportant part of `make test` for
those just running the test suite (as opposed to those authoring new
tests), the simplest workaround is to disable chainlint when you run
the tests by setting environment variable GIT_TEST_EXT_CHAIN_LINT to
0.

RE: Request for Help - Too many perl arguments as of 2.43.0-rc0

[rsbecker]

On November 3, 2023 4:52 PM, Eric Sunshine wrote:
>On Fri, Nov 3, 2023 at 3:03 PM <rsbecker@nexbridge.com> wrote:
>> I can no longer test git as the number of arguments (and size) sent to
>> perl from the make infrastructure is too big for my platform. Is it
>> possible to use xargs instead?
>
>Presumably you're talking about the invocation of chainlint.pl?
>Considering that that's quite an unimportant part of `make test` for those just
>running the test suite (as opposed to those authoring new tests), the simplest
>workaround is to disable chainlint when you run the tests by setting environment
>variable GIT_TEST_EXT_CHAIN_LINT to 0.

I tried both ways to specify this variable with no success:

/home/randall/git: make -C t/ all SHELL=/usr/coreutils/bin/bash GIT_TEST_EXT_CHAIN_LINT=0
make: Entering directory '/home/randall/git/t'
rm -f -r 'test-results'
/usr/coreutils/bin/bash: /usr/bin/perl: Argument list too long
Makefile:125: recipe for target 'test-lint-shell-syntax' failed
make: *** [test-lint-shell-syntax] Error 126
make: Leaving directory '/home/randall/git/t'
/home/randall/git: GIT_TEST_EXT_CHAIN_LINT=0 make -C t/ all SHELL=/usr/coreutils/bin/bash
make: Entering directory '/home/randall/git/t'
rm -f -r 'test-results'
/usr/coreutils/bin/bash: /usr/bin/perl: Argument list too long
Makefile:125: recipe for target 'test-lint-shell-syntax' failed
make: *** [test-lint-shell-syntax] Error 126
make: Leaving directory '/home/randall/git/t'

Re: Request for Help - Too many perl arguments as of 2.43.0-rc0

[Eric Sunshine]

On Fri, Nov 3, 2023 at 5:31 PM <rsbecker@nexbridge.com> wrote:
> On November 3, 2023 4:52 PM, Eric Sunshine wrote:
> >On Fri, Nov 3, 2023 at 3:03 PM <rsbecker@nexbridge.com> wrote:
> >> I can no longer test git as the number of arguments (and size) sent to
> >> perl from the make infrastructure is too big for my platform. Is it
> >> possible to use xargs instead?
> >
> >Presumably you're talking about the invocation of chainlint.pl?
> >Considering that that's quite an unimportant part of `make test` for those just
> >running the test suite (as opposed to those authoring new tests), the simplest
> >workaround is to disable chainlint when you run the tests by setting environment
> >variable GIT_TEST_EXT_CHAIN_LINT to 0.
>
> I tried both ways to specify this variable with no success:
>
> /usr/coreutils/bin/bash: /usr/bin/perl: Argument list too long
> Makefile:125: recipe for target 'test-lint-shell-syntax' failed

This is a separate issue from chainlint (though, it too will likely
suffer the same problem). In this case, it's trying to run the
check-non-portable-shell.pl script and the command-line is too long.
Unfortunately, unlike chainlint for which there is a knob to disble
it, there is no direct knob to disable `test-lint-shell-syntax`. You
may be able to skip all the shell-script linting by setting TEST_LINT
to an empty value.

RE: Request for Help - Too many perl arguments as of 2.43.0-rc0

[rsbecker]

On November 3, 2023 5:51 PM, Eric Sunshine wrote:
>On Fri, Nov 3, 2023 at 5:31 PM <rsbecker@nexbridge.com> wrote:
>> On November 3, 2023 4:52 PM, Eric Sunshine wrote:
>> >On Fri, Nov 3, 2023 at 3:03 PM <rsbecker@nexbridge.com> wrote:
>> >> I can no longer test git as the number of arguments (and size) sent
>> >> to perl from the make infrastructure is too big for my platform. Is
>> >> it possible to use xargs instead?
>> >
>> >Presumably you're talking about the invocation of chainlint.pl?
>> >Considering that that's quite an unimportant part of `make test` for
>> >those just running the test suite (as opposed to those authoring new
>> >tests), the simplest workaround is to disable chainlint when you run
>> >the tests by setting environment variable GIT_TEST_EXT_CHAIN_LINT to 0.
>>
>> I tried both ways to specify this variable with no success:
>>
>> /usr/coreutils/bin/bash: /usr/bin/perl: Argument list too long
>> Makefile:125: recipe for target 'test-lint-shell-syntax' failed
>
>This is a separate issue from chainlint (though, it too will likely suffer the same
>problem). In this case, it's trying to run the check-non-portable-shell.pl script and
>the command-line is too long.
>Unfortunately, unlike chainlint for which there is a knob to disble it, there is no direct
>knob to disable `test-lint-shell-syntax`. You may be able to skip all the shell-script
>linting by setting TEST_LINT to an empty value.

That worked. I wonder whether it might be useful to set up a knob for this.

Re: Request for Help - Too many perl arguments as of 2.43.0-rc0

[Eric Sunshine]

On Fri, Nov 3, 2023 at 6:04 PM <rsbecker@nexbridge.com> wrote:
> On November 3, 2023 5:51 PM, Eric Sunshine wrote:
> >On Fri, Nov 3, 2023 at 5:31 PM <rsbecker@nexbridge.com> wrote:
> >> /usr/coreutils/bin/bash: /usr/bin/perl: Argument list too long
> >> Makefile:125: recipe for target 'test-lint-shell-syntax' failed
> >
> >This is a separate issue from chainlint (though, it too will likely suffer the same
> >problem). In this case, it's trying to run the check-non-portable-shell.pl script and
> >the command-line is too long.
> >Unfortunately, unlike chainlint for which there is a knob to disble it, there is no direct
> >knob to disable `test-lint-shell-syntax`. You may be able to skip all the shell-script
> >linting by setting TEST_LINT to an empty value.
>
> That worked. I wonder whether it might be useful to set up a knob for this.

I don't see an urgent need for it. Unlike the actual tests themselves
run by `make test` which may catch platform-specific problems in Git
itself, the purpose of the "linting" checks is not to catch
platform-specific problems, but rather to help test authors by
identifying mistakes in the tests which might make them fragile. So,
disabling linting on a particular platform isn't going to cause `make
test` to miss some important Git problem specific to that platform.

Also, in the longer term, as you suggested, `xargs` is likely a more
fruitful solution.

Re: Request for Help - Too many perl arguments as of 2.43.0-rc0

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Also, in the longer term, as you suggested, `xargs` is likely a more
> fruitful solution.

Hmph, the list of our test scripts exceed command line limit?  That
sounds a bit nasty, as we somehow need to prepare a pipe and feed
them into it, in order to drive xargs downstream of the pipe.


Ideally if there were a GNUMake function that slices a list into
sublists of "reasonable" lengths, we could use it to directly drive
N invocations of check-non-portable-shell script instead of xargs,
but I didn't find one.  Here is I came up with, using foreach that
is "slice the list into many sublists of 1 element", but it made me
feel dirty.

 t/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git c/t/Makefile w/t/Makefile
index 3e00cdd801..f9ac6bb4eb 100644
--- c/t/Makefile
+++ w/t/Makefile
@@ -122,7 +122,8 @@ test-lint-executable:
 		echo >&2 "non-executable tests:" $$bad; exit 1; }
 
 test-lint-shell-syntax:
-	@'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
+	@{ $(foreach t,$(T) $(THELPERS) $(TPERF),echo "$t";) } | \
+	xargs '$(PERL_PATH_SQ)' check-non-portable-shell.pl
 
 test-lint-filenames:
 	@# We do *not* pass a glob to ls-files but use grep instead, to catch

Re: Request for Help - Too many perl arguments as of 2.43.0-rc0

[Jeff King]

On Sat, Nov 04, 2023 at 02:36:48AM -0400, Eric Sunshine wrote:

> I don't see an urgent need for it. Unlike the actual tests themselves
> run by `make test` which may catch platform-specific problems in Git
> itself, the purpose of the "linting" checks is not to catch
> platform-specific problems, but rather to help test authors by
> identifying mistakes in the tests which might make them fragile. So,
> disabling linting on a particular platform isn't going to cause `make
> test` to miss some important Git problem specific to that platform.

Hmm. With compilation, we split the audience of "developers" vs "people
who just want to build the program", and we crank up the number and
severity of warning checks for the former. We could do the same here for
tests. I.e., turn off test linting by default and re-enable it for
DEVELOPER=1.

OTOH, this is the first time I think I've seen the linting cause a
problem (whereas unexpected compile warnings are much more likely, as we
are depending on the system compiler's behavior).

So consider it an idle thought for discussion, and not necessarily a
proposal. ;)

-Peff

RE: Request for Help - Too many perl arguments as of 2.43.0-rc0

[rsbecker]

On November 4, 2023 9:49 AM, Jeff King wrote:
>On Sat, Nov 04, 2023 at 02:36:48AM -0400, Eric Sunshine wrote:
>
>> I don't see an urgent need for it. Unlike the actual tests themselves
>> run by `make test` which may catch platform-specific problems in Git
>> itself, the purpose of the "linting" checks is not to catch
>> platform-specific problems, but rather to help test authors by
>> identifying mistakes in the tests which might make them fragile. So,
>> disabling linting on a particular platform isn't going to cause `make
>> test` to miss some important Git problem specific to that platform.
>
>Hmm. With compilation, we split the audience of "developers" vs "people who just
>want to build the program", and we crank up the number and severity of warning
>checks for the former. We could do the same here for tests. I.e., turn off test linting
>by default and re-enable it for DEVELOPER=1.
>
>OTOH, this is the first time I think I've seen the linting cause a problem (whereas
>unexpected compile warnings are much more likely, as we are depending on the
>system compiler's behavior).
>
>So consider it an idle thought for discussion, and not necessarily a proposal. ;)

In my case, I am building git for users of the platform (a.k.a. mostly a packager), although I'm hoping to be more involved in contributions soon. This involves build + full test under multiple situations on the NonStop platform.
--Randall

Re: Request for Help - Too many perl arguments as of 2.43.0-rc0

[Eric Sunshine]

On Sat, Nov 4, 2023 at 3:59 AM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > Also, in the longer term, as you suggested, `xargs` is likely a more
> > fruitful solution.
>
> Hmph, the list of our test scripts exceed command line limit?  That
> sounds a bit nasty, as we somehow need to prepare a pipe and feed
> them into it, in order to drive xargs downstream of the pipe.
>
> Ideally if there were a GNUMake function that slices a list into
> sublists of "reasonable" lengths, we could use it to directly drive
> N invocations of check-non-portable-shell script instead of xargs,
> but I didn't find one.  Here is I came up with, using foreach that
> is "slice the list into many sublists of 1 element", but it made me
> feel dirty.

Indeed, that's ugly. I hadn't even put any thought into it since there
doesn't seem to be a pressing need for it.

In the long run, Ævar's idea of having `make` notice which, if any,
test scripts have changed, and only perform linting on an as-needed
basis may be the way to go[1,2,3], thus only passing a single script
as argument to each of the linters.

[1]: https://lore.kernel.org/git/220901.86bkrzjm6e.gmgdl@evledraar.gmail.com/
[2]: https://lore.kernel.org/git/221122.86cz9fbyln.gmgdl@evledraar.gmail.com/
[3]: https://github.com/avar/git/commits/avar/t-Makefile-break-T-to-file-association

Re: Request for Help - Too many perl arguments as of 2.43.0-rc0

[Eric Sunshine]

On Sat, Nov 4, 2023 at 9:49 AM Jeff King <peff@peff.net> wrote:
> On Sat, Nov 04, 2023 at 02:36:48AM -0400, Eric Sunshine wrote:
> > I don't see an urgent need for it. Unlike the actual tests themselves
> > run by `make test` which may catch platform-specific problems in Git
> > itself, the purpose of the "linting" checks is not to catch
> > platform-specific problems, but rather to help test authors by
> > identifying mistakes in the tests which might make them fragile. So,
> > disabling linting on a particular platform isn't going to cause `make
> > test` to miss some important Git problem specific to that platform.
>
> Hmm. With compilation, we split the audience of "developers" vs "people
> who just want to build the program", and we crank up the number and
> severity of warning checks for the former. We could do the same here for
> tests. I.e., turn off test linting by default and re-enable it for
> DEVELOPER=1.

My knee-jerk reaction is that this would move us in the wrong
direction since it is probable that most drive-by contributors won't
have DEVELOPER=1 set, yet they are the ones who are likely to benefit
most from test script linting (which is not to say that it doesn't
help seasoned contributors, as well).

> OTOH, this is the first time I think I've seen the linting cause a
> problem (whereas unexpected compile warnings are much more likely, as we
> are depending on the system compiler's behavior).

There have been a few other times when the linting scripts have needed
a tweak or two to work properly on some less-well-represented
platform, but they were minor issues[1,2,3,4].

> So consider it an idle thought for discussion, and not necessarily a
> proposal. ;)

As noted in my response to Junio[5], in the long run, we may want to
go with Ævar's idea of having `make` track changes to the test
scripts, thus only run linting on an as-needed basis.

[1]: a3c4c8841c (tests: use shorter labels in chainlint.sed for AIX
sed, 2018-08-24)
[2]: 2d9ded8acc (tests: fix comment syntax in chainlint.sed for AIX
sed, 2018-08-24)
[3]: b3b753b104 (Fit to Plan 9's ANSI/POSIX compatibility layer, 2020-09-10)
[4]: 1f51b77f4f (chainlint.pl: fix /proc/cpuinfo regexp, 2022-11-22)
[5]: https://lore.kernel.org/git/CAPig+cSC8m5a8PhMw_eJbswwNB-VgBt+n56HSTLLabV9_+y--g@mail.gmail.com/

Re: Request for Help - Too many perl arguments as of 2.43.0-rc0

[Jeff King]

On Sat, Nov 04, 2023 at 08:11:01PM -0400, Eric Sunshine wrote:

> > Hmm. With compilation, we split the audience of "developers" vs "people
> > who just want to build the program", and we crank up the number and
> > severity of warning checks for the former. We could do the same here for
> > tests. I.e., turn off test linting by default and re-enable it for
> > DEVELOPER=1.
> 
> My knee-jerk reaction is that this would move us in the wrong
> direction since it is probable that most drive-by contributors won't
> have DEVELOPER=1 set, yet they are the ones who are likely to benefit
> most from test script linting (which is not to say that it doesn't
> help seasoned contributors, as well).

Yeah, that's a good point. If the linting is not causing frequent
headaches (and I don't think it is), then we are better to leave it on
by default.

-Peff