Re: tests fail on windows with default git settings

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org, Dave Page <dpage(at)pgadmin(dot)org>
Subject: Re: tests fail on windows with default git settings
Date: 2024-07-08 20:56:10
Message-ID: a49418d3-83ea-42bf-a356-2a057839c2e0@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 2024-07-08 Mo 4:16 PM, Andres Freund wrote:
> On 2024-07-07 06:30:57 -0400, Andrew Dunstan wrote:
>> On 2024-07-07 Su 1:26 AM, Tom Lane wrote:
>>> Andres Freund <andres(at)anarazel(dot)de> writes:
>>>> Do we want to support checking out with core.autocrlf?
>>> -1. It would be a constant source of breakage, and you could never
>>> expect that (for example) making a tarball from such a checkout
>>> would match anyone else's results.
>> Yeah, totally agree.
>>
>>
>>>> If we do not want to support that, ISTM we ought to raise an error somewhere?
>>> +1, if we can figure out how.
>>>
>>>
>>
>>
>> ISTM the right fix is probably to use PG_BINARY_R mode instead of "r" when
>> opening the files, at least in the case if the test_json_parser tests.
> That does seem like it'd fix this issue, assuming the parser can cope with
> \r\n.

Yes, the parser can handle \r\n. Note that they can only be white space
in JSON - they can only be present in string values via escapes.

>
> I'm actually mildly surprised that the tests don't fail when *not* using
> autocrlf, because afaict test_json_parser_incremental.c doesn't set stdout to
> binary and thus we presumably end up with \r\n in the output? Except that that
> can't be true, because the test does pass on repos without autocrlf...
>
>
> That approach does seem to mildly conflict with Tom and your preference for
> fixing this by disallowing core.autocrlf? If we do so, the test never ought to
> see a crlf?
>

IDK. I normally use core.autocrlf=false core.eol=lf on Windows. The
editors I use are reasonably well behaved ;-)

What I suggest (see attached) is we run the diff command with
--strip-trailing-cr on Windows. Then we just won't care if the expected
file and/or the output file has CRs.

Not sure what the issue is with pg_bsd_indent, though.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Attachment Content-Type Size
json_parser_test_cr-fix.patch text/x-patch 1.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2024-07-08 21:09:21 Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible
Previous Message Robert Haas 2024-07-08 20:45:42 Re: 010_pg_basebackup.pl vs multiple filesystems