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

From: Jeff King <peff@peff.net>
Date: Jul 10, 2024, 01:09 UTC
Message-ID: <20240710010927.GA1529709@coredump.intra.peff.net>
In-Reply-To: <20240708090837.GD819809@coredump.intra.peff.net>

On Mon, Jul 08, 2024 at 05:08:37AM -0400, Jeff King wrote:

Show 6 quoted lines

> I took a look at running each test individually. It's surprisingly quite
> a bit slower! About 4s instead of 200ms. There's a bit of low-hanging
> fruit to get it down to ~1.7s (which I'll include in my series). But in
> the end I punted on that for now, but did add line-number checks. Each
> expect file just knows its own numbers, and I use a bit of perl to
> handle the running offset.

By the way, in case you are wondering why I haven't sent out the next iteration of the series: I am stuck trying to figure out some Windows line-ending nonsense.

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 '

(the funny overwrite is because the invalid char is a CR). I tried a few simple things to skip past this error, but the problem is pervasive. 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.

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

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

I'm tempted to just do this:

--- a/t/chainlint.pl
+++ b/t/chainlint.pl
@@ -779,7 +779,7 @@ sub check_script {
        while (my $path = $next_script->()) {
                $nscripts++;
                my $fh;
-               unless (open($fh, "<", $path)) {
+               unless (open($fh, "<:unix:crlf", $path)) {
                        $emit->("?!ERR?! $path: $!\n");
                        next;
                }

It feels like a hack, but it makes the parser's assumptions explicit,
and it should just work everywhere.

-Peff