Re: [PATCH] chainlint.pl: recognize test bodies defined via heredoc

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 10, 2024, 03:02 UTC
Message-ID: <CAPig+cRXkOesS_ctvxY2X=rwesTzgrBB0=5fvQLQsG3hZVY9TQ@mail.gmail.com>
In-Reply-To: <20240710010927.GA1529709@coredump.intra.peff.net>

On Tue, Jul 9, 2024 at 9:09 PM Jeff King <peff@peff.net> wrote:

Show 6 quoted lines

> The chainlint.pl parser chokes on CRLF line endings. So Windows CI
> produces:
>
>   runneradmin@fv-az1390-742 MINGW64 /d/a/git/git/t
>   # perl chainlint.pl chainlint/for-loop.test
>   'nternal error scanning character '

As far as I understand, chainlint is disabled in the Windows CI. Did you manually re-enable it for testing? Or are you just running it manually in the Windows CI?

Show 13 quoted lines

> We really just want to have perl handle the line endings on read. And doing
> this works:
>
>   # PERLIO=:crlf perl chainlint.pl chainlint/for-loop.test
>   # chainlint: chainlint/for-loop.test
>   # chainlint: for-loop
>   [...etc, normal output...]
>
> Which gives me all sorts of questions:
>
>   - isn't crlf handling usually the default for perl builds on Windows?
>     I guess this is probably getting into weird mingw vs native Windows
>     distinctions that generally leave me perplexed.

Could be. I'm not sure how the Windows CI is provisioned, whether with some native-compiled Perl or with msys2/mingw Perl.

Show 5 quoted lines

>   - why wasn't this a problem before? I'm guessing again in the "weird
>     mingw stuff" hand-waving way that when we used "sed" to assemble
>     everything, it stripped the CR's in the "chainlinttmp/tests" file.
>     And in my series, that "cat" is replaced with a perl script (that
>     writes the "tests" and "expect" files together).

Assuming you manually re-enabled chaintlint in the Windows CI for this testing or are running it manually, it may be the case that chainlint.pl has never been run in the Windows CI. Specifically, chainlint in Windows CI was disabled by a87e427e35 (ci: speed up Windows phase, 2019-01-29) which predates the switchover from chainlint.sed to chainlint.pl by d00113ec34 (t/Makefile: apply chainlint.pl to existing self-tests, 2022-09-01). So, it's quite possible that chainlint.pl has never run in Windows CI. But, perhaps I'm misunderstanding or missing some piece of information.

That said, I did thoroughly test chainlint.pl on Windows using Git For Windows, and it did run in that environment. (But if the Windows CI environment is somehow different, then that might explain the problem?)

>   - why doesn't "PERLIO=:crlf make check-chainlint" work? It seems that
>     perl spawned from "make" behaves differently. More mingw weirdness?

That could indeed be an msys2 issue. It will automatically convert colon ":" to semicolon ";" in environment variables since the PATH separator on Windows is ";", not ":" as it is on Unix. Moreover, the ":" to ";" switcheroo logic is not restricted only to PATH since there are other PATH-like variables in common use, so it's applied to all environment variables.

Show 10 quoted lines

> I'm tempted to just do this:
>
>         while (my $path = $next_script->()) {
>                 $nscripts++;
>                 my $fh;
> -               unless (open($fh, "<", $path)) {
> +               unless (open($fh, "<:unix:crlf", $path)) {
>
> It feels like a hack, but it makes the parser's assumptions explicit,
> and it should just work everywhere.

Yep, if this makes it work, then it seems like a good way forward, especially since I don't think there's any obvious way to work around the ":" to ";" switcheroo performed by msys2.