From: Alexander Lakhin <exclusion@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrew Dunstan <andrew@dunslane.net>
Cc: buildfarm@sraoss.co.jp, Pavel Stehule <pavel.stehule@gmail.com>
Cc: Christoph Berg <myon@debian.org>
Cc: Michael Banck <mbanck@gmx.net>
Cc: Tommy Pavlicek <tommypav122@gmail.com>
Cc: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Cc: pgsql-hackers@lists.postgresql.org
Cc: jian.universality@gmail.com
Subject: Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR)
Date: Mon, 28 Oct 2024 12:00:00 +0300
Message-ID: <79284195-4993-7b00-f6df-8db28ca60fa3@gmail.com> (raw)
In-Reply-To: <3792414.1730050914@sss.pgh.pa.us>
References: <CAEhP-W_1bMTLan2LtzS+xr=b-Wep56gw+OoirABQ8b2aOw+5yg@mail.gmail.com>
<ZTfL1G1fBPQHVPWV@msg.df7cb.de>
<560086.1698160614@sss.pgh.pa.us>
<ZTfjXzOKFI9AawJF@msg.df7cb.de>
<570833.1698162135@sss.pgh.pa.us>
<66f33840.050a0220.1f9e60.7fb9@mx.google.com>
<ZvV1ClhnbJLCz7Sm@msg.df7cb.de>
<325394.1727365907@sss.pgh.pa.us>
<ZvaiS57G10dITjQm@msg.df7cb.de>
<571930.1727454668@sss.pgh.pa.us>
<Zvbr5zv78AfhB83S@msg.df7cb.de>
<621972.1727459684@sss.pgh.pa.us>
<1092841.1727552736@sss.pgh.pa.us>
<2245576.1728418678@sss.pgh.pa.us>
<CAFj8pRBGquZK3aTYvN559FFqa=bMXEuxZZBLc6kMd671u62hTQ@mail.gmail.com>
<2864637.1728662904@sss.pgh.pa.us>
<CAFj8pRCF2-ObuMHV5sxQbq9sZ+hCv5mV5SUToD8=9Qw8tZ7DgQ@mail.gmail.com>
<CAFj8pRCw09RUROc31nJCZBPXrO5fxaDO-Lg885eXMUPwjBZNSw@mail.gmail.com>
<1967263.1729611423@sss.pgh.pa.us>
<CAFj8pRCtFo2jyJAjXpzB8Rg=mtPXfS44bp2gM4Ea-iRD-wA9Xw@mail.gmail.com>
<2480333.1729784872@sss.pgh.pa.us>
<3792414.1730050914@sss.pgh.pa.us>
Hello Tom,
27.10.2024 20:41, Tom Lane wrote:
> I wrote:
>> In the no-good-deed-goes-unpunished department: buildfarm member
>> hamerkop doesn't like this patch [1]. The diffs look like
>> ...
>> So what I'd like to do to fix this is to change
>> - if ((file = AllocateFile(filename, PG_BINARY_R)) == NULL)
>> + if ((file = AllocateFile(filename, "r")) == NULL)
> Well, that didn't fix it :-(. I went so far as to extract the raw log
> files from the buildfarm database, and what they show is that there is
> absolutely no difference between the lines diff is claiming are
> different:
>
> -QUERY: CREATE FUNCTIN my_erroneous_func(int) RETURNS int LANGUAGE SQL\r\n
> +QUERY: CREATE FUNCTIN my_erroneous_func(int) RETURNS int LANGUAGE SQL\r\n
>
> It's the same both before and after 924e03917, which made the code
> change depicted above, so that didn't help.
>
> So I'm pretty baffled. I suppose the expected and result files must
> actually be different, and something in subsequent processing is
> losing the difference before it gets to the buildfarm database.
> But I don't have the ability to debug that from here. Does anyone
> with access to hamerkop want to poke into this?
>
> Without additional information, the only thing I can think of that
> I have any confidence will eliminate these failures is to reformat
> the affected test cases so that they produce just a single line of
> output. That's kind of annoying from a functionality-coverage point
> of view, but I'm not sure avoiding it is worth moving mountains for.
>
I've managed to reproduce the issue with the core.autocrlf=true git setting
(which sets CR+LF line ending in test_ext7--2.0--2.1bad.sql) and with diff
from msys:
C:\msys64\usr\bin\diff.exe --version
diff (GNU diffutils) 3.8
(Gnu/Win32 Diff [1] doesn't detect those EOL differences and thus the test
doesn't fail.)
I can really see different line endings with hexdump:
hexdump -C ...testrun\test_extensions\regress\regression.diffs
00000230 20 20 20 20 20 20 20 20 5e 0a 2d 51 55 45 52 59 | ^.-QUERY|
00000240 3a 20 20 43 52 45 41 54 45 20 46 55 4e 43 54 49 |: CREATE FUNCTI|
00000250 4e 20 6d 79 5f 65 72 72 6f 6e 65 6f 75 73 5f 66 |N my_erroneous_f|
00000260 75 6e 63 28 69 6e 74 29 20 52 45 54 55 52 4e 53 |unc(int) RETURNS|
00000270 20 69 6e 74 20 4c 41 4e 47 55 41 47 45 20 53 51 | int LANGUAGE SQ|
00000280 4c 0a 2b 51 55 45 52 59 3a 20 20 43 52 45 41 54 |L.+QUERY: CREAT|
00000290 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 72 |E FUNCTIN my_err|
000002a0 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 29 |oneous_func(int)|
000002b0 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 4e | RETURNS int LAN|
000002c0 47 55 41 47 45 20 53 51 4c 0d 0a 20 41 53 20 24 |GUAGE SQL.. AS $|
hexdump -C .../testrun/test_extensions/regress/results/test_extensions.out | grep -C5 FUNCTIN
00000b80 20 5e 0d 0a 51 55 45 52 59 3a 20 20 43 52 45 41 | ^..QUERY: CREA|
00000b90 54 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 |TE FUNCTIN my_er|
00000ba0 72 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 |roneous_func(int|
00000bb0 29 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 |) RETURNS int LA|
00000bc0 4e 47 55 41 47 45 20 53 51 4c 0d 0d 0a 41 53 20 |NGUAGE SQL...AS |
whilst
hexdump -C .../src/test/modules/test_extensions/expected/test_extensions.out | grep -C5 FUNCTIN
00000b80 20 5e 0d 0a 51 55 45 52 59 3a 20 20 43 52 45 41 | ^..QUERY: CREA|
00000b90 54 45 20 46 55 4e 43 54 49 4e 20 6d 79 5f 65 72 |TE FUNCTIN my_er|
00000ba0 72 6f 6e 65 6f 75 73 5f 66 75 6e 63 28 69 6e 74 |roneous_func(int|
00000bb0 29 20 52 45 54 55 52 4e 53 20 69 6e 74 20 4c 41 |) RETURNS int LA|
00000bc0 4e 47 55 41 47 45 20 53 51 4c 0d 0a 41 53 20 24 |NGUAGE SQL..AS $|
It looks like --strip-trailing-cr doesn't work as desired for this diff version.
I've also dumped buf in read_whole_file() and found that in both
PG_BINARY_R and "r" modes the 0d 0a ending is preserved. But it changed
to 0a with the "rt" mode (see [1]), and it makes the test (and the whole
`meson test`) pass for me.
[1] https://gnuwin32.sourceforge.net/packages/diffutils.htm
[2] https://learn.microsoft.com/en-us/cpp/c-runtime-library/file-translation-constants?view=msvc-170
Best regards,
Alexander
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: pgsql-hackers@postgresql.org
Cc: exclusion@gmail.com, tgl@sss.pgh.pa.us, andrew@dunslane.net, pavel.stehule@gmail.com, myon@debian.org, mbanck@gmx.net, tommypav122@gmail.com, ilmari@ilmari.org, pgsql-hackers@lists.postgresql.org, jian.universality@gmail.com
Subject: Re: Better error reporting from extension scripts (Was: Extend ALTER OPERATOR)
In-Reply-To: <79284195-4993-7b00-f6df-8db28ca60fa3@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox