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 <and...@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
diff --git a/src/test/modules/test_json_parser/t/003_test_semantic.pl b/src/test/modules/test_json_parser/t/003_test_semantic.pl
index 74e0fa5bb1..2a430ad0c4 100644
--- a/src/test/modules/test_json_parser/t/003_test_semantic.pl
+++ b/src/test/modules/test_json_parser/t/003_test_semantic.pl
@@ -29,7 +29,9 @@ print $fh $stdout, "\n";
close($fh);
-($stdout, $stderr) = run_command([ "diff", "-u", $fname, $test_out ]);
+my @diffopts = ("-u");
+push(@diffopts, "--strip-railing-cr") if $windows_os;
+($stdout, $stderr) = run_command([ "diff", @diffopts, $fname, $test_out ]);
is($stdout, "", "no output diff");
is($stderr, "", "no diff error");
diff --git a/src/test/modules/test_json_parser/test_json_parser_incremental.c b/src/test/modules/test_json_parser/test_json_parser_incremental.c
index f4c442ac36..47040e1e42 100644
--- a/src/test/modules/test_json_parser/test_json_parser_incremental.c
+++ b/src/test/modules/test_json_parser/test_json_parser_incremental.c
@@ -124,7 +124,7 @@ main(int argc, char **argv)
makeJsonLexContextIncremental(&lex, PG_UTF8, need_strings);
initStringInfo(&json);
- if ((json_file = fopen(testfile, "r")) == NULL)
+ if ((json_file = fopen(testfile, PG_BINARY_R)) == NULL)
pg_fatal("error opening input: %m");
if (fstat(fileno(json_file), &statbuf) != 0)
diff --git a/src/test/modules/test_json_parser/test_json_parser_perf.c b/src/test/modules/test_json_parser/test_json_parser_perf.c
index ea85626cbd..74cc5f3f54 100644
--- a/src/test/modules/test_json_parser/test_json_parser_perf.c
+++ b/src/test/modules/test_json_parser/test_json_parser_perf.c
@@ -55,7 +55,7 @@ main(int argc, char **argv)
sscanf(argv[1], "%d", &iter);
- if ((json_file = fopen(argv[2], "r")) == NULL)
+ if ((json_file = fopen(argv[2], PG_BINARY_R)) == NULL)
pg_fatal("Could not open input file '%s': %m", argv[2]);
while ((n_read = fread(buff, 1, 6000, json_file)) > 0)
