From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Lakhin <exclusion@gmail.com>
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
Subject: Re: Better error reporting from extension scripts (Was: Extend ALTER
 OPERATOR)
In-reply-to: <79284195-4993-7b00-f6df-8db28ca60fa3@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>
 <79284195-4993-7b00-f6df-8db28ca60fa3@gmail.co!
 m>
Comments: In-reply-to Alexander Lakhin <exclusion@gmail.com>
	message dated "Mon, 28 Oct 2024 12:00:00 +0300"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <3975514.1730131606.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Mon, 28 Oct 2024 12:06:46 -0400
Message-ID: <3975515.1730131606@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3975515.1730131606%40sss.pgh.pa.us>
Precedence: bulk

Alexander Lakhin <exclusion@gmail.com> writes:
> 27.10.2024 20:41, Tom Lane wrote:
>>> In the no-good-deed-goes-unpunished department: buildfarm member
>>> hamerkop doesn't like this patch [1].  The diffs look like

> I've managed to reproduce the issue with the core.autocrlf=3Dtrue git se=
tting
> (which sets CR+LF line ending in test_ext7--2.0--2.1bad.sql) and with di=
ff
> 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 te=
st
> doesn't fail.)

Thank you for looking at this!

> I can really see different line endings with hexdump:
> hexdump -C .../testrun/test_extensions/regress/results/test_extensions.o=
ut | grep -C5 FUNCTIN
> 00000b80=C2=A0 20 5e 0d 0a 51 55 45 52=C2=A0 59 3a 20 20 43 52 45 41=C2=A0=
 | ^..QUERY:=C2=A0 CREA|
> 00000b90=C2=A0 54 45 20 46 55 4e 43 54=C2=A0 49 4e 20 6d 79 5f 65 72=C2=A0=
 |TE FUNCTIN my_er|
> 00000ba0=C2=A0 72 6f 6e 65 6f 75 73 5f=C2=A0 66 75 6e 63 28 69 6e 74 |ro=
neous_func(int|
> 00000bb0=C2=A0 29 20 52 45 54 55 52 4e=C2=A0 53 20 69 6e 74 20 4c 41=C2=A0=
 |) RETURNS int LA|
> 00000bc0=C2=A0 4e 47 55 41 47 45 20 53=C2=A0 51 4c 0d 0d 0a 41 53 20=C2=A0=
 |NGUAGE SQL...AS |

Wow.  How are we producing \r\r\n?  I'm not hugely surprised that some
versions of diff might see that as different from a single newline
even with --strip-trailing-cr --- formally, I think that's supposed
to make \r\n match \n, but the extra \r doesn't fit that pattern.

> 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.

Interesting.  I believe we decided years ago that we didn't need to
use "rt" mode because that was the default on Windows, but was that
a misreading of the documentation?  The link you provided doesn't
give any hint that there are more than two behaviors.

However ... the link you provided also mentions that text mode
includes treating control-Z as EOF; which I'd forgotten, but it
does make it less safe than I thought to use text mode for
reading script files.

What I'm now thinking is that we should revert 924e03917 after
all (that is, go back to using PG_BINARY_R) and instead make
read_whole_file manually squash \r\n to \n if we're on Windows.
Ugly, but I have yet to find anything about that platform that
isn't.

			regards, tom lane