Feedback-ID: id4a34324:Fastmail
Date: Tue, 24 Jan 2023 17:48:56 -0800
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>, Noah Misch <noah@leadboat.com>
Cc: pgsql-hackers@postgresql.org,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	Andrew Dunstan <andrew@dunslane.net>
Subject: Re: plpython vs _POSIX_C_SOURCE
Message-ID: <20230125014856.j3ndx2cftjxdvqul@awork3.anarazel.de>
References: <20230124165814.2njc7gnvubn2amh6@awork3.anarazel.de>
 <2265665.1674582915@sss.pgh.pa.us>
 <20230124190721.mi7l5c3n2tsrqjno@awork3.anarazel.de>
 <2320614.1674594966@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2320614.1674594966@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/20230125014856.j3ndx2cftjxdvqul%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2023-01-24 16:16:06 -0500, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Python's _POSIX_C_SOURCE value is set to a specific value in their configure
> > script:
>
> > if test $define_xopen_source = yes
> > then
> >   ...
> >   AC_DEFINE(_POSIX_C_SOURCE, 200809L, Define to activate features from IEEE Stds 1003.1-2008)
> > fi
>
> Hm.  I looked into Python 3.2 (the oldest release we still support)
> and it has similar code but
>
>   AC_DEFINE(_POSIX_C_SOURCE, 200112L, Define to activate features from IEEE Stds 1003.1-2001)
>
> So yeah it's fixed (or else not defined) for any particular Python
> release, but could vary across releases.

Looks like it changed in 3.3:

$ git grep -E 'AC_DEFINE.*_POSIX_C_SOURCE' v3.2 v3.3.0
v3.2:configure.in:  AC_DEFINE(_POSIX_C_SOURCE, 200112L, Define to activate features from IEEE Stds 1003.1-2001)
v3.3.0:configure.ac:  AC_DEFINE(_POSIX_C_SOURCE, 200809L, Define to activate features from IEEE Stds 1003.1-2008)

I'm not sure we need to care a lot about a build with python 3.3 triggering a
bunch of warnings.

Personally I'd just bump the python requirements to well above it - the last
3.2 release was Oct. 12, 2014.

Official EOL date:
Ver	Last Release	EOL Date
3.2	2014-10-11	2016-02-20
3.3	2017-09-19	2017-09-29
From 3.4 on there's just an official last release:
3.4	2019-03-18
3.5	2020-09-05
3.6	2021-09-04
3.7	2023-06-??


> > Solaris and AIX are the ones missing.
> > I guess I'll test them manually. It seems promising not to need this stuff
> > anymore?
>
> Given that hoverfly is AIX, I'm betting there's an issue there.

Doesn't look that way.

I found plenty problems on AIX, but all independent of _POSIX_C_SOURCE.


Both autoconf and meson builds seem to need externally specified
-D_LARGE_FILES=1 to build successfully when using plpython, otherwise we end
up with conflicting signatures with lseek. I see that Noah has that in his
buildfarm config.  ISTM that we should just move that into our build specs.


To get 64 bit autoconf to link plpython3.so correctly, I needed to add to
manually add -lpthread:
ld: 0711-317 ERROR: Undefined symbol: .pthread_init
...

I suspect Noah might not hit this, because one of the dependencies he has
enabled already adds it to the backend LDFLAGS.


Also for autoconf, I needed to link
$prefix/lib/python3.11/config-3.11/libpython3.11.a
to
$prefix/lib/libpython3.11.a
That might be a python version difference or be related to building python
with --enable-shared - but I see saw other problems without --enable-shared.


I ran out of energy to test on aix with xlc, I spent way more time on this
than I have already. I'll pick it up later.


I also tested 64bit solaris. No relevant warnings (lots of other warnings
though), tests pass, with both acc and gcc.


> >> 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.
>
> > You think it's not a viable path to just remove the _POSIX_C_SOURCE,
> > _XOPEN_SOURCE undefines?
>
> I think at the least that will result in warnings on some platforms,
> and at the worst in actual build problems.  Maybe there are no more
> of the latter a dozen years after the fact, but ...

I think it might be ok. I tested nearly all OSs that we support, with the
exception of DragonFlyBSD and Illumos, which both are very similar to tested
operating systems.


> Nice idea.  We did not have that option while we were using HAVE_SNPRINTF
> ourselves, but now that we are not I concur that this should work.

Cool.


> (I confirmed that their code looks the same in Python 3.2.)
> Note that you'd better make it
>
> #define HAVE_SNPRINTF 1
>
> or you risk macro-redefinition warnings.

Good point.

I guess I'll push that part first, given that we have agreement how it should
look like.

Greetings,

Andres Freund