On Tue, Jul 9, 2024 at 9:09 PM Jeff King <peff@xxxxxxxx> wrote:
> 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?
> 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.
> - 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.
> 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.
