public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrew Dunstan <[email protected]>
To: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: [email protected]
Cc: Dave Page <[email protected]>
Subject: Re: tests fail on windows with default git settings
Date: Mon, 8 Jul 2024 16:56:10 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
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 <[email protected]> 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
Attachments:
[text/x-patch] json_parser_test_cr-fix.patch (1.9K, ../[email protected]/2-json_parser_test_cr-fix.patch)
download | inline diff:
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)
view thread (8+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: tests fail on windows with default git settings
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox