Thread: Re: pgsql: Build src/port files as a library with -fPIC, and usethat in li

Re: pgsql: Build src/port files as a library with -fPIC, and usethat in li

From
Christoph Berg
Date:
Re: Tom Lane 2018-09-27 <E1g5Y8r-0006vs-QA@gemulon.postgresql.org>
> Build src/port files as a library with -fPIC, and use that in libpq.

This made the "pqsignal" symbol disappear from libpq5.so:

13:27:55 dpkg-gensymbols: error: some symbols or patterns disappeared in the symbols file: see diff output below
13:27:55 dpkg-gensymbols: warning: debian/libpq5/DEBIAN/symbols doesn't match completely debian/libpq5.symbols
13:27:55 --- debian/libpq5.symbols (libpq5_12~~devel~20180928.1058-1~226.git92a0342.pgdg+1_amd64)
13:27:55 +++ dpkg-gensymbolsoXZn54    2018-09-28 11:27:55.499157237 +0000
13:27:55 @@ -168,7 +168,7 @@
13:27:55   pg_valid_server_encoding@Base 0
13:27:55   pg_valid_server_encoding_id@Base 8.3~beta1-2~
13:27:55   pgresStatus@Base 0
13:27:55 - pqsignal@Base 0
13:27:55 +#MISSING: 12~~devel~20180928.1058-1~226.git92a0342.pgdg+1# pqsignal@Base 0
13:27:55   printfPQExpBuffer@Base 0
13:27:55   resetPQExpBuffer@Base 0
13:27:55   termPQExpBuffer@Base 0

Is this is a problem for libpq5 users?

Christoph


Re: pgsql: Build src/port files as a library with -fPIC, and use that in li

From
Tom Lane
Date:
Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2018-09-27 <E1g5Y8r-0006vs-QA@gemulon.postgresql.org>
>> Build src/port files as a library with -fPIC, and use that in libpq.

> This made the "pqsignal" symbol disappear from libpq5.so:

Oh, interesting.  I'd seen an actual error on prairiedog, but apparently
some other linkers just silently omit the export, if the symbol is in
a .a file rather than .o.

> Is this is a problem for libpq5 users?

I proposed in
https://www.postgresql.org/message-id/19581.1538077716@sss.pgh.pa.us

that we should remove pqsignal, as well as
pg_utf_mblen
pg_encoding_to_char
pg_char_to_encoding
pg_valid_server_encoding
pg_valid_server_encoding_id

from libpq's exports, on the grounds that (a) nobody should be using
those (they're undocumented as exports), and (b) anybody who is using
them should get them from libpgport/libpgcommon instead.  Thoughts?

            regards, tom lane


Re: pgsql: Build src/port files as a library with -fPIC, and usethat in li

From
Christoph Berg
Date:
Re: Tom Lane 2018-09-28 <19404.1538140436@sss.pgh.pa.us>
> > Is this is a problem for libpq5 users?
> 
> I proposed in
> https://www.postgresql.org/message-id/19581.1538077716@sss.pgh.pa.us
> 
> that we should remove pqsignal, as well as
> pg_utf_mblen
> pg_encoding_to_char
> pg_char_to_encoding
> pg_valid_server_encoding
> pg_valid_server_encoding_id
> 
> from libpq's exports, on the grounds that (a) nobody should be using
> those (they're undocumented as exports), and (b) anybody who is using
> them should get them from libpgport/libpgcommon instead.  Thoughts?

I'm fine with that if we say (a) should be true, and even if it is
not, (b) is an easy workaround. Bumping the libpq SONAME just because
of this seems excessive.

On the Debian side, I can simply remove the symbol from the tracking
file and the buildsystem will be happy again.

Christoph


Re: pgsql: Build src/port files as a library with -fPIC, and use that in li

From
Tom Lane
Date:
Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2018-09-28 <19404.1538140436@sss.pgh.pa.us>
>> I proposed in
>> https://www.postgresql.org/message-id/19581.1538077716@sss.pgh.pa.us
>> 
>> that we should remove pqsignal, as well as
>> pg_utf_mblen
>> pg_encoding_to_char
>> pg_char_to_encoding
>> pg_valid_server_encoding
>> pg_valid_server_encoding_id
>> 
>> from libpq's exports, on the grounds that (a) nobody should be using
>> those (they're undocumented as exports), and (b) anybody who is using
>> them should get them from libpgport/libpgcommon instead.  Thoughts?

> I'm fine with that if we say (a) should be true, and even if it is
> not, (b) is an easy workaround. Bumping the libpq SONAME just because
> of this seems excessive.

Yeah, if anyone insists that this requires a soname bump, I'd probably
look for another solution.  Making the makefiles a bit cleaner is not
worth the churn that would cause, IMO.

The place where we'd probably end up if anyone's unhappy about this
is that we'd still be symlinking the three files pqsignal.c, wchar.c,
and encnames.c into libpq.  That's not very desirable, but at least
it'd be a fixed list rather than something we're continually having
to change.

            regards, tom lane


Re: pgsql: Build src/port files as a library with -fPIC, and usethat in li

From
Christoph Berg
Date:
Re: Tom Lane 2018-09-28 <20671.1538142538@sss.pgh.pa.us>
> >> I proposed in
> >> https://www.postgresql.org/message-id/19581.1538077716@sss.pgh.pa.us
> >> 
> >> that we should remove pqsignal, as well as
> >> pg_utf_mblen
> >> pg_encoding_to_char
> >> pg_char_to_encoding
> >> pg_valid_server_encoding
> >> pg_valid_server_encoding_id
> >> 
> >> from libpq's exports, on the grounds that (a) nobody should be using
> >> those (they're undocumented as exports), and (b) anybody who is using
> >> them should get them from libpgport/libpgcommon instead.  Thoughts?
> 
> > I'm fine with that if we say (a) should be true, and even if it is
> > not, (b) is an easy workaround. Bumping the libpq SONAME just because
> > of this seems excessive.
> 
> Yeah, if anyone insists that this requires a soname bump, I'd probably
> look for another solution.  Making the makefiles a bit cleaner is not
> worth the churn that would cause, IMO.

It took a while to notice, but this change does break 8.2's initdb if
libpq5 from PG12 is installed:

$ /usr/lib/postgresql/8.2/bin/initdb
/usr/lib/postgresql/8.2/bin/initdb: symbol lookup error: /usr/lib/postgresql/8.2/bin/initdb: undefined symbol:
pqsignal

postgres 8.2 itself seems to work fine.

Christoph


Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2018-09-28 <20671.1538142538@sss.pgh.pa.us>
>> I proposed in
>> https://www.postgresql.org/message-id/19581.1538077716@sss.pgh.pa.us
>> that we should remove pqsignal, as well as
>> pg_utf_mblen
>> pg_encoding_to_char
>> pg_char_to_encoding
>> pg_valid_server_encoding
>> pg_valid_server_encoding_id
>>
>> from libpq's exports, on the grounds that (a) nobody should be using
>> those (they're undocumented as exports), and (b) anybody who is using
>> them should get them from libpgport/libpgcommon instead.  Thoughts?

> It took a while to notice, but this change does break 8.2's initdb if
> libpq5 from PG12 is installed:
> $ /usr/lib/postgresql/8.2/bin/initdb
> /usr/lib/postgresql/8.2/bin/initdb: symbol lookup error: /usr/lib/postgresql/8.2/bin/initdb: undefined symbol:
pqsignal

Well, 8.2 is *very* long out of support, and there are plenty of
nasty bugs you're at risk of if you keep using it.  So I don't
find this to be a good argument for contorting what we do in v12.

If you really want to keep using 8.2 (and even make new installations
with it!?), you could back-patch the 8.3 patch that made sure that
initdb didn't absorb pqsignal, pg_encoding_to_char, etc from libpq.
It looks like what you'd need is a portion of the Makefile changes
from 8468146b03c87f86162cee62e0de25f6e2d24b56.

BTW, I noticed this in that patch's commit message:

    Going forward, we want to fix things so that encoding IDs can be changed
    without an ABI break, and this commit includes the changes needed to allow
    libpq's encoding IDs to be treated as fully independent of the backend's.
    The main issue is that libpq clients should not include pg_wchar.h or
    otherwise assume they know the specific values of libpq's encoding IDs,
    since they might encounter version skew between pg_wchar.h and the libpq.so
    they are using.  To fix, have libpq officially export functions needed for
    encoding name<=>ID conversion and validity checking; it was doing this
    anyway unofficially.

So it was wrong of me to propose moving pg_encoding_to_char() et al
into libpgcommon: doing so would've re-introduced the hazard of
client code misinterpreting the encoding IDs returned by
PQclientEncoding() (and how the heck did I miss that libpq.sgml
does document that function for exactly that purpose?).

Fortunately, I didn't do that ...

            regards, tom lane


Re: pgsql: Build src/port files as a library with -fPIC, and usethat in li

From
Christoph Berg
Date:
Re: Tom Lane 2019-01-29 <27653.1548774621@sss.pgh.pa.us>
> > It took a while to notice, but this change does break 8.2's initdb if
> > libpq5 from PG12 is installed:
> > $ /usr/lib/postgresql/8.2/bin/initdb
> > /usr/lib/postgresql/8.2/bin/initdb: symbol lookup error: /usr/lib/postgresql/8.2/bin/initdb: undefined symbol:
pqsignal
> 
> Well, 8.2 is *very* long out of support, and there are plenty of
> nasty bugs you're at risk of if you keep using it.  So I don't
> find this to be a good argument for contorting what we do in v12.

I try to keep the old versions alive on apt.pg.o for Debian unstable (only)
so I have something to grab when customers ask questions about the old
versions. We are probably lucky that 8.2 broke only now, and leaving
it broken is a fair thing. I was just mentioning for completeness, I
didn't mean to insist on fixing it.

> If you really want to keep using 8.2 (and even make new installations
> with it!?), you could back-patch the 8.3 patch that made sure that
> initdb didn't absorb pqsignal, pg_encoding_to_char, etc from libpq.
> It looks like what you'd need is a portion of the Makefile changes
> from 8468146b03c87f86162cee62e0de25f6e2d24b56.

I might give that a shot, but if it's too hard, dropping 8.2 "support"
is not a problem. Thanks for digging up the commit hash.

Christoph


Re: pgsql: Build src/port files as a library with -fPIC, and use thatin li

From
Robert Haas
Date:
On Tue, Jan 29, 2019 at 10:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Well, 8.2 is *very* long out of support, and there are plenty of
> nasty bugs you're at risk of if you keep using it.  So I don't
> find this to be a good argument for contorting what we do in v12.

Isn't the issue more a matter of whether it's OK to break
compatibility without changing SO_*_VERSION?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jan 29, 2019 at 10:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Well, 8.2 is *very* long out of support, and there are plenty of
>> nasty bugs you're at risk of if you keep using it.  So I don't
>> find this to be a good argument for contorting what we do in v12.

> Isn't the issue more a matter of whether it's OK to break
> compatibility without changing SO_*_VERSION?

We're breaking compatibility only if you regard pqsignal() as
part of the advertised API, which I don't.

            regards, tom lane


Re: pgsql: Build src/port files as a library with -fPIC, and use thatin li

From
Robert Haas
Date:
On Thu, Jan 31, 2019 at 2:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > Isn't the issue more a matter of whether it's OK to break
> > compatibility without changing SO_*_VERSION?
>
> We're breaking compatibility only if you regard pqsignal() as
> part of the advertised API, which I don't.

Well, I would agree with that if it weren't the case that binaries we
ship apparently use that API.  If we say that the libraries are
API-compatible, but our own binaries don't work, our veracity is
questionable.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Jan 31, 2019 at 2:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> We're breaking compatibility only if you regard pqsignal() as
>> part of the advertised API, which I don't.

> Well, I would agree with that if it weren't the case that binaries we
> ship apparently use that API.  If we say that the libraries are
> API-compatible, but our own binaries don't work, our veracity is
> questionable.

initdb hasn't depended on those libpq exports since 8.2, and it was
a bug that it did so even then.

            regards, tom lane


Re: pgsql: Build src/port files as a library with -fPIC, and usethat in li

From
Christoph Berg
Date:
Re: Tom Lane 2019-01-31 <12792.1548965044@sss.pgh.pa.us>
> initdb hasn't depended on those libpq exports since 8.2, and it was
> a bug that it did so even then.

9.2 psql (and createuser, ...) is also broken:

/usr/lib/postgresql/9.2/bin/psql: symbol lookup error: /usr/lib/postgresql/9.2/bin/psql: undefined symbol: pqsignal

Could we at least put a compat symbol back?

Christoph