Re: tests fail on windows with default git settings
Andrew Dunstan <andrew@dunslane.net>
From: Andrew Dunstan <andrew@dunslane.net>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@postgresql.org,
Dave Page <dpage@pgadmin.org>
Date: 2024-07-08T20:56:10Z
Lists: pgsql-hackers
Attachments
- json_parser_test_cr-fix.patch (text/x-patch) patch
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@anarazel.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
Commits
-
Use diff --strip-trailing-cr in pg_regress.c
- 291c42074721 18.0 landed
-
Use diff's --strip-trailing-cr flag where appropriate on Windows
- 4506d18a9891 17.0 landed
- 628c1d1f2c82 18.0 landed