Message-ID: <79284195-4993-7b00-f6df-8db28ca60fa3@gmail.com>
Date: Mon, 28 Oct 2024 12:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: Better error reporting from extension scripts (Was: Extend ALTER
 OPERATOR)
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrew Dunstan <andrew@dunslane.net>, buildfarm@sraoss.co.jp,
 Pavel Stehule <pavel.stehule@gmail.com>, Christoph Berg <myon@debian.org>,
 Michael Banck <mbanck@gmx.net>, Tommy Pavlicek <tommypav122@gmail.com>,
 =?UTF-8?Q?Dagfinn_Ilmari_Manns=c3=a5ker?= <ilmari@ilmari.org>,
 pgsql-hackers@lists.postgresql.org, jian.universality@gmail.com
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>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <3792414.1730050914@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/79284195-4993-7b00-f6df-8db28ca60fa3%40gmail.com>
Precedence: bulk

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