Re: Let's stop with the retail rebuilds of src/port/ files already

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Let's stop with the retail rebuilds of src/port/ files already
Date: 2018-09-27 19:48:36
Message-ID: 19581.1538077716@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Here's a partial patch for that: it adds the third build variant
> to src/port/ and teaches libpq to use it. We'd want to likewise
> modify src/common/ and fix up other callers such as ecpg, but this
> seems to be enough to test whether the idea works or not.
> ...
> What I think would make sense is to push this and see what the
> buildfarm thinks of it. If there are unfixable problems then
> we won't have wasted time fleshing out the concept. Otherwise,
> I'll do the remaining pieces.

Well, the buildfarm did turn up one problem: on really old macOS
(cf prairiedog) the libpq link step fails with

ld: symbols names listed in -exported_symbols_list: exports.list not in linked objects
_pqsignal

Apparently, with that linker, the exported symbols list is resolved
against just what is found in the listed *.o files, not anything pulled
in from a library file.

Now, the question that raises in my mind is why is libpq.so exporting
pqsignal() at all? Probably there was once a reason for it, but nowadays
I would think that any client program using pqsignal() should get it
from -lpgport, not from libpq. Having more than one place to get it from
seems more likely to create issues than solve them. And we certainly
do not document it as a function available from libpq.

So my recommendation is to remove pqsignal from libpq's exports.txt.
I've verified that prairiedog builds happily with that change,
confirming my expectation that all consumers of the symbol can get it
from someplace else.

Now, if we go forward with that solution, there will be issues with
some other things that libpq exports without having defined itself:

src/backend/utils/mb/wchar.c:
pg_utf_mblen
src/backend/utils/mb/encnames.c:
pg_encoding_to_char
pg_char_to_encoding
pg_valid_server_encoding
pg_valid_server_encoding_id

Now, I'd already had my eye on those two files, because after applying a
similar fix for src/common/, those two files would be the only ones that
libpq needs to symlink from somewhere else.

What I was thinking of proposing was to move those two files out of the
backend and into src/common/, thereby normalizing their status as
modules available in both frontend and backend, and removing the need
for a special build rule for them in libpq. (initdb could be simplified
too.) Per this discovery, we'd need to also remove these symbols from
libpq's exports list, meaning that clients *must* get them from -lpgcommon
not from libpq.

There's a small chance that this'd break third-party clients that
are using these symbols out of libpq. We've never documented them
as being available, but somebody might be using them anyway.
If that does happen, it could be repaired by linking against -lpgcommon
along with libpq, but it'd possibly still make people unhappy.

Comments?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2018-09-27 20:00:20 \d+ fails on index on partition
Previous Message Andres Freund 2018-09-27 18:42:49 Re: pgsql: Remove absolete function TupleDescGetSlot().