Re: Annoying build warnings from latest Apple toolchain

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Noah Misch <noah(at)leadboat(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Annoying build warnings from latest Apple toolchain
Date: 2023-10-03 17:34:33
Message-ID: 2385119.1696354473@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
>> On 2023-09-29 12:14:40 -0400, Tom Lane wrote:
>>> Therefore, I think the prudent thing to do in the back branches is use the
>>> patch I posted before, to suppress the duplicate -l switches only on macOS.
>>> In HEAD, I propose we simplify life by doing it everywhere, as attached.

>> Makes sense.

> Done that way.

So, in the no-good-deed-goes-unpunished department, I see that Noah's
AIX animals are now complaining about a few duplicate symbols, eg
while building initdb:

ld: 0711-224 WARNING: Duplicate symbol: .pg_encoding_to_char
ld: 0711-224 WARNING: Duplicate symbol: .pg_valid_server_encoding
ld: 0711-224 WARNING: Duplicate symbol: .pg_valid_server_encoding_id
ld: 0711-224 WARNING: Duplicate symbol: .pg_char_to_encoding

It's far from clear to me why we see this warning now when we didn't
before, because there are strictly fewer sources of these symbols in
the link than before. They are available from libpgcommon and are
also intentionally exported from libpq. Presumably, initdb is now
linking to these symbols from libpq where before it got them from the
first mention of libpgcommon, but why is that any more worthy of a
warning?

Anyway, I don't have a huge problem with just ignoring these warnings
as such, since AFAICT they're only showing up on AIX.

However, thinking about this made me realize that there's a related
problem. At one time we had intentionally made initdb use its own
copy of these routines rather than letting it get them from libpq.
The reason is explained in commit 8468146b0:

Fix the inadvertent libpq ABI breakage discovered by Martin Pitt: the
renumbering of encoding IDs done between 8.2 and 8.3 turns out to break 8.2
initdb and psql if they are run with an 8.3beta1 libpq.so.

This policy is still memorialized in a comment in initdb/Makefile:

# Note: it's important that we link to encnames.o from libpgcommon, not
# from libpq, else we have risks of version skew if we run with a libpq
# shared library from a different PG version. The libpq_pgport macro
# should ensure that that happens.

and pg_wchar.h has this related comment:

* XXX We must avoid renumbering any backend encoding until libpq's major
* version number is increased beyond 5; it turns out that the backend
* encoding IDs are effectively part of libpq's ABI as far as 8.2 initdb and
* psql are concerned.

Now it's not happening that way. How big a problem is that?

In the case of psql, I think it's actually fixing a latent bug.
8468146b0's changes in psql clearly intend that psql will be
linking to libpq's copies of pg_char_to_encoding and
pg_valid_server_encoding_id, which is appropriate because it's
dealing with libpq's encoding IDs. We broke that when we moved
encnames.c into libpgcommon. We've not made any encoding ID
redefinitions since then, and we'd be unlikely to renumber PG_UTF8
in any case, but clearly linking to libpq's copies is safer.

(Which means that the makefiles are now OK, but the meson
build is not: we need libpq to be linked before libpgcommon.)

However, in the case of initdb, we had better be using the same
encoding IDs as the backend code we are setting up the database for.
If we ever add/renumber any backend-safe encodings again, we'd be
exposed to the same problem that 8.3 had.

Assuming that this problem is restricted to initdb, which I think
is true, probably the best fix is to cause the initdb link *only*
to link libpgcommon before libpq. Every other non-backend program
is interested in libpq's encoding IDs if it cares at all.

Thoughts?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2023-10-03 17:35:57 Re: On login trigger: take three
Previous Message Robert Haas 2023-10-03 16:44:36 Re: pgBufferUsage.blk_{read|write}_time are zero although there are pgBufferUsage.local_blks_{read|written}