From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: pgsql-hackers@postgresql.org,
        Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
        Andrew Dunstan <andrew@dunslane.net>
Subject: Re: plpython vs _POSIX_C_SOURCE
In-reply-to: <20230124165814.2njc7gnvubn2amh6@awork3.anarazel.de>
References: <20230124165814.2njc7gnvubn2amh6@awork3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Tue, 24 Jan 2023 08:58:14 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2265664.1674582915.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Tue, 24 Jan 2023 12:55:15 -0500
Message-ID: <2265665.1674582915@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/2265665.1674582915%40sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.de> writes:
> The background for the undefines is that _POSIX_C_SOURCE needs to be def=
ined
> the same for the whole compilation, not change in the middle, and Python=
.h
> defines it. To protect "our" parts a11cf433413 instituted the rule that =
all
> postgres headers have to be included first. But then that promptly got b=
roken
> in 147c2482542.

> But apparently the breakage in 147c2482542 was partial enough that we di=
dn't
> run into obvious trouble so far (although I wonder if some of the linkag=
e
> issues we had in the past with plpython could be related).

I found the discussion thread that led up to a11cf433413:

https://www.postgresql.org/message-id/flat/4DB3B546.9080508%40dunslane.net

What we originally set out to fix, AFAICS, was compiler warnings about
_POSIX_C_SOURCE getting redefined with a different value.  I think that'd
only happen if pyconfig.h had originally been constructed on a machine
where _POSIX_C_SOURCE was different from what prevails in a Postgres
build.  On my RHEL8 box, I see that /usr/include/python3.6m/pyconfig-64.h
unconditionally does

#define _POSIX_C_SOURCE 200809L

while /usr/include/features.h can set a few different values, but the
one that would always prevail for us is

#ifdef _GNU_SOURCE
...
# undef  _POSIX_C_SOURCE
# define _POSIX_C_SOURCE	200809L

So I wouldn't see this warning, and I venture that you'd never see
it on any other Linux/glibc platform either.  The 2011 thread started
with concerns about Windows, where it's a lot easier to believe that
there might be mismatched build environments.  But maybe nobody's
actually set up a Windows box with that particular problem since 2011.

Whether inconsistency in _POSIX_C_SOURCE could lead to worse problems
than a compiler warning isn't entirely clear to me, but it certainly
seems possible.

Anyway, I'm still of the opinion that what a11cf433413 tried to do
was the best available fix, and we need to do whatever we have to do
to plpython's headers to reinstate that coding rule.

> The most minimal fix I can see is to institute the rule that no plpy_*.h
> header can include a postgres header other than plpython.h.

Doesn't sound *too* awful.

> Or we could see what breaks if we just don't care about _POSIX_C_SOURCE =
-
> evidently we haven't succeeded in making this work for a long time.

Well, hoverfly is broken right now ...

>  * Sometimes python carefully scribbles on our *printf macros.
>  * So we undefine them here and redefine them after it's done its dirty =
deed.

> I didn't find code in recent-ish python to do that. Perhaps we should tr=
y to
> get away with not doing that?

That would be nice.  This old code was certainly mostly concerned with
python 2, maybe python 3 no longer does that?  (Unfortunately, the
_POSIX_C_SOURCE business is clearly still there in current python.)

			regards, tom lane