Обсуждение: Replace uses of deprecated Python module distutils.sysconfig

With Python 3.10, configure spits out warnings about the module 
distutils.sysconfig being deprecated and scheduled for removal in Python 
3.12:

<string>:1: DeprecationWarning: The distutils.sysconfig module is 
deprecated, use sysconfig instead
<string>:1: DeprecationWarning: The distutils package is deprecated and 
slated for removal in Python 3.12. Use setuptools or check PEP 632 for 
potential alternatives

This patch changes the uses in configure to use the module sysconfig 
instead.  The logic stays the same.  (It's basically the same module but 
as its own top-level module.)

Note that sysconfig exists since Python 2.7, so this moves the minimum 
required version up from Python 2.6.

Buildfarm impact:

gaur and prariedog use Python 2.6 and would need to be upgraded.

Possible backpatching:

Backpatching should be considered, since surely someone will otherwise 
complain when Python 3.12 comes around.  But dropping support for Python 
versions in stable branches should be done with some care.

Python 3.10 was released Oct. 4, 2021, so it is quite new.  Python major 
releases are now yearly, so the above-mentioned Python 3.12 can be 
expected in autumn of 2023.

Current PostgreSQL releases support Python versions as follows:

PG10: 2.4+
PG11: 2.4+
PG12: 2.4+  (EOL Nov. 2024)
PG13: 2.6+
PG14: 2.6+

So unfortunately, we won't be able to EOL all versions with Python 2.4 
support before Python 3.12 arrives.

I suggest leaving the backbranches alone for now.  At the moment, we 
don't even know whether additional changes will be required for 3.12 
(and 3.11) support, so the overall impact isn't known yet.  In a few 
months, we will probably know more about this.

In the meantime, the warnings can be silenced using

export PYTHONWARNINGS='ignore::DeprecationWarning'

(It ought to be possible to be more specific, like 
'ignore::DeprecationWarning:distutils.sysconfig', but it doesn't seem to 
work for me.)

(I don't recommend putting that into configure, since then we wouldn't 
be able to learn about issues like this.)

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> With Python 3.10, configure spits out warnings about the module 
> distutils.sysconfig being deprecated and scheduled for removal in Python 
> 3.12:

Bleah.

> This patch changes the uses in configure to use the module sysconfig 
> instead.  The logic stays the same.  (It's basically the same module but 
> as its own top-level module.)
> Note that sysconfig exists since Python 2.7, so this moves the minimum 
> required version up from Python 2.6.

That's surely no problem in HEAD, but as you say, it is an issue for
the older branches.  How difficult would it be to teach configure to
try both ways, or adapt based on its python version check?

> I suggest leaving the backbranches alone for now.  At the moment, we 
> don't even know whether additional changes will be required for 3.12 
> (and 3.11) support, so the overall impact isn't known yet.  In a few 
> months, we will probably know more about this.

Agreed, this is a moving target so we shouldn't be too concerned
about it yet.

            regards, tom lane

On 02.12.21 08:20, Peter Eisentraut wrote:
> Buildfarm impact:
> 
> gaur and prariedog use Python 2.6 and would need to be upgraded.

Tom, are you planning to update the Python version on these build farm 
members?  I realize these are very slow machines and this might take 
some time; I'm just wondering if this had registered.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 02.12.21 08:20, Peter Eisentraut wrote:
>> Buildfarm impact:
>> gaur and prariedog use Python 2.6 and would need to be upgraded.

> Tom, are you planning to update the Python version on these build farm 
> members?  I realize these are very slow machines and this might take 
> some time; I'm just wondering if this had registered.

I can do that when it becomes necessary.  I've got one eye on the meson
conversion discussion, which will kill those two animals altogether;
so it seems possible that updating their Pythons now would just be
wasted effort depending on what lands first.

            regards, tom lane

On 09.12.21 14:31, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> On 02.12.21 08:20, Peter Eisentraut wrote:
>>> Buildfarm impact:
>>> gaur and prariedog use Python 2.6 and would need to be upgraded.
> 
>> Tom, are you planning to update the Python version on these build farm
>> members?  I realize these are very slow machines and this might take
>> some time; I'm just wondering if this had registered.
> 
> I can do that when it becomes necessary.  I've got one eye on the meson
> conversion discussion, which will kill those two animals altogether;
> so it seems possible that updating their Pythons now would just be
> wasted effort depending on what lands first.

I saw that the Python installations on gaur and prairiedog had been 
updated, so I committed this patch.  As the buildfarm shows, various 
platforms have problems with this, in particular because they point to 
the wrong place for the include directory.  AFAICT, in most cases this 
appears to have been fixed in more recent editions of those platforms 
(e.g., Debian unstable members pass but older releases don't), so at 
least the approach was apparently not wrong in principle.  But 
obviously, this leaves us in a mess.  I will revert this patch in a bit, 
after gathering a few more hours of data.

Also, considering the failure on prairiedog, I do see now on 
<https://docs.python.org/3/library/sysconfig.html> that the sysconfig 
module is "New in version 3.2".  I had interpreted the fact that it 
exists in version 2.7 that that includes all higher versions, but 
obviously there were multiple branches involved, so that was a mistaken 
assumption.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> Also, considering the failure on prairiedog, I do see now on 
> <https://docs.python.org/3/library/sysconfig.html> that the sysconfig 
> module is "New in version 3.2".  I had interpreted the fact that it 
> exists in version 2.7 that that includes all higher versions, but 
> obviously there were multiple branches involved, so that was a mistaken 
> assumption.

Hm.  I installed 3.1 because we claim support for that.  I don't mind
updating to 3.2 (as long as we adjust the docs to match), but it seems
kinda moot unless you figure out a solution for the include-path
issue.  I see that platforms as recent as Debian 10 are failing,
so I don't think we can dismiss that as not needing fixing.

            regards, tom lane

On 18.01.22 16:24, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> Also, considering the failure on prairiedog, I do see now on
>> <https://docs.python.org/3/library/sysconfig.html> that the sysconfig
>> module is "New in version 3.2".  I had interpreted the fact that it
>> exists in version 2.7 that that includes all higher versions, but
>> obviously there were multiple branches involved, so that was a mistaken
>> assumption.
> 
> Hm.  I installed 3.1 because we claim support for that.  I don't mind
> updating to 3.2 (as long as we adjust the docs to match), but it seems
> kinda moot unless you figure out a solution for the include-path
> issue.  I see that platforms as recent as Debian 10 are failing,
> so I don't think we can dismiss that as not needing fixing.

I have reverted this for now.

I don't have a clear idea how to fix this in the long run.  We would 
perhaps need to determine at which points the various platforms had 
fixed this issue in their Python installations and select between the 
old and the new approach based on that.  Seems messy.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> I don't have a clear idea how to fix this in the long run.  We would 
> perhaps need to determine at which points the various platforms had 
> fixed this issue in their Python installations and select between the 
> old and the new approach based on that.  Seems messy.

Are we sure it's an issue within Python, rather than something we
could dodge by invoking sysconfig differently?  It's hard to believe
that sysconfig could be totally unfit for the purpose of finding out
the include path and would remain so for multiple years.

            regards, tom lane

I wrote:
> Are we sure it's an issue within Python, rather than something we
> could dodge by invoking sysconfig differently?  It's hard to believe
> that sysconfig could be totally unfit for the purpose of finding out
> the include path and would remain so for multiple years.

I dug up a Debian 9 image and found that I could reproduce the problem
against its python2 (2.7.13) installation, but not its python3 (3.5.3):

$ python2 -m sysconfig | grep include
        include = "/usr/local/include/python2.7"
        platinclude = "/usr/local/include/python2.7"
...
$ python3 -m sysconfig | grep include
        include = "/usr/include/python3.5m"
        platinclude = "/usr/include/python3.5m"
...

Looking at the buildfarm animals that failed this way, 10 out of 11
are using python 2.x.  The lone exception is Andrew's prion.  I wonder
if there is something unusual about its python3 installation.

Anyway, based on these results, we might have better luck switching to
sysconfig after we start forcing python3.  I'm tempted to resurrect the
idea of changing configure's probe order to "python3 python python2"
in the meantime, just so we can see how much of the buildfarm is ready
for that.

            regards, tom lane

I wrote:
> Anyway, based on these results, we might have better luck switching to
> sysconfig after we start forcing python3.

On the other hand, that answer is not back-patchable, and we surely
need a back-patchable fix, because people will try to build the
back branches against newer pythons.

Based on the buildfarm results so far, the problem can be described
as "some installations say /usr/local when they should have said /usr".
I experimented with the attached delta patch and it fixes the problem
on my Debian 9 image.  (I don't know Python, so there may be a better
way to do this.)  We'd have to also bump the minimum 3.x version to
3.2, but that seems very unlikely to bother anyone.

            regards, tom lane

diff --git a/config/python.m4 b/config/python.m4
index 8ca1eaa64b..c65356c6ac 100644
--- a/config/python.m4
+++ b/config/python.m4
@@ -56,13 +56,20 @@ AC_MSG_RESULT([$python_configdir])

 AC_MSG_CHECKING([Python include directories])
 python_includespec=`${PYTHON} -c "
-import sysconfig
-a = '-I' + sysconfig.get_path('include')
-b = '-I' + sysconfig.get_path('platinclude')
+import sysconfig, os.path
+a = sysconfig.get_path('include')
+b = sysconfig.get_path('platinclude')
+# Some versions of sysconfig report '/usr/local/include'
+# when they should have said '/usr/include'
+if not os.path.exists(a + '/Python.h'):
+    aalt = a.replace('/usr/local/', '/usr/', 1)
+    if os.path.exists(aalt + '/Python.h'):
+        a = aalt
+        b = b.replace('/usr/local/', '/usr/', 1)
 if a == b:
-    print(a)
+    print('-I' + a)
 else:
-    print(a + ' ' + b)"`
+    print('-I' + a + ' -I' + b)"`
 if test "$PORTNAME" = win32 ; then
     python_includespec=`echo $python_includespec | sed 's,[[\]],/,g'`
 fi
diff --git a/configure b/configure
index 9c856cb1d5..f88db9467d 100755
--- a/configure
+++ b/configure
@@ -10370,13 +10370,20 @@ $as_echo "$python_configdir" >&6; }
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking Python include directories" >&5
 $as_echo_n "checking Python include directories... " >&6; }
 python_includespec=`${PYTHON} -c "
-import sysconfig
-a = '-I' + sysconfig.get_path('include')
-b = '-I' + sysconfig.get_path('platinclude')
+import sysconfig, os.path
+a = sysconfig.get_path('include')
+b = sysconfig.get_path('platinclude')
+# Some versions of sysconfig report '/usr/local/include'
+# when they should have said '/usr/include'
+if not os.path.exists(a + '/Python.h'):
+    aalt = a.replace('/usr/local/', '/usr/', 1)
+    if os.path.exists(aalt + '/Python.h'):
+        a = aalt
+        b = b.replace('/usr/local/', '/usr/', 1)
 if a == b:
-    print(a)
+    print('-I' + a)
 else:
-    print(a + ' ' + b)"`
+    print('-I' + a + ' -I' + b)"`
 if test "$PORTNAME" = win32 ; then
     python_includespec=`echo $python_includespec | sed 's,[\],/,g'`
 fi

On 19.01.22 01:18, Tom Lane wrote:
> Anyway, based on these results, we might have better luck switching to
> sysconfig after we start forcing python3.  I'm tempted to resurrect the
> idea of changing configure's probe order to "python3 python python2"
> in the meantime, just so we can see how much of the buildfarm is ready
> for that.

This seems sensible in any case, given that we have quasi-committed to 
enforcing Python 3 soon.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 19.01.22 01:18, Tom Lane wrote:
>> ... I'm tempted to resurrect the
>> idea of changing configure's probe order to "python3 python python2"
>> in the meantime, just so we can see how much of the buildfarm is ready
>> for that.

> This seems sensible in any case, given that we have quasi-committed to 
> enforcing Python 3 soon.

Done.  (I couldn't find any equivalent logic in the MSVC build scripts
though; is there something I missed?)

            regards, tom lane

I wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> On 19.01.22 01:18, Tom Lane wrote:
>>> ... I'm tempted to resurrect the
>>> idea of changing configure's probe order to "python3 python python2"
>>> in the meantime, just so we can see how much of the buildfarm is ready
>>> for that.

>> This seems sensible in any case, given that we have quasi-committed to 
>> enforcing Python 3 soon.

> Done.

The early returns are not great: we have about half a dozen machines
so far that are finding python3, and reporting sane-looking Python
include paths, but not finding Python.h.  They're all Linux-oid
machines, so I suppose what is going on is that they have the base
python3 package installed but not python3-dev or local equivalent.

I want to leave that patch in place long enough so we can get a
fairly full survey of which machines are OK and which are not,
but I suppose I'll have to revert it tomorrow or so.  We did
promise the owners a month to adjust their configurations.

            regards, tom lane

=?UTF-8?Q?Juan_Jos=C3=A9_Santamar=C3=ADa_Flecha?= <juanjo.santamaria@gmail.com> writes:
> On Wed, Jan 19, 2022 at 9:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Done.  (I couldn't find any equivalent logic in the MSVC build scripts
>> though; is there something I missed?)

> MSVC will use the path configured in src\tools\msvc\config.pl $config->{"python"},
> there is no ambiguity.

Ah, right.  We have only three active Windows animals that are building
with python, and all three are using 3.something, so that side of the
house seems to be ready to go.

            regards, tom lane

On 1/18/22 19:18, Tom Lane wrote:
> I wrote:
>> Are we sure it's an issue within Python, rather than something we
>> could dodge by invoking sysconfig differently?  It's hard to believe
>> that sysconfig could be totally unfit for the purpose of finding out
>> the include path and would remain so for multiple years.
> I dug up a Debian 9 image and found that I could reproduce the problem
> against its python2 (2.7.13) installation, but not its python3 (3.5.3):
>
> $ python2 -m sysconfig | grep include
>         include = "/usr/local/include/python2.7"
>         platinclude = "/usr/local/include/python2.7"
> ...
> $ python3 -m sysconfig | grep include
>         include = "/usr/include/python3.5m"
>         platinclude = "/usr/include/python3.5m"
> ...
>
> Looking at the buildfarm animals that failed this way, 10 out of 11
> are using python 2.x.  The lone exception is Andrew's prion.  I wonder
> if there is something unusual about its python3 installation.



It's an Amazon Linux instance, and using their packages, which seem a
bit odd (there's nothing in /usr/local/include). Maybe we should be
looking at INCLUEPY?


[ec2-user@ip-172-31-22-42 bf]$ python3 -m sysconfig | grep include
    include = "/usr/local/include/python3.6m"
    platinclude = "/usr/local/include/python3.6m"
    CONFIG_ARGS = "'--build=x86_64-redhat-linux-gnu'
'--host=x86_64-redhat-linux-gnu' '--target=x86_64-amazon-linux-gnu'
'--program-prefix=' '--prefix=/usr' '--exec-prefix=/usr'
'--bindir=/usr/bin' '--sbindir=/usr/sbin' '--sysconfdir=/etc'
'--datadir=/usr/share' '--includedir=/usr/include' '--libdir=/usr/lib64'
'--libexecdir=/usr/libexec' '--localstatedir=/var'
'--sharedstatedir=/var/lib' '--mandir=/usr/share/man'
'--infodir=/usr/share/info' '--enable-ipv6' '--enable-shared'
'--with-computed-gotos=yes' '--with-dbmliborder=gdbm:ndbm:bdb'
'--with-system-expat' '--with-system-ffi'
'--enable-loadable-sqlite-extensions' '--with-dtrace' '--with-valgrind'
'--without-ensurepip' '--enable-optimizations'
'build_alias=x86_64-redhat-linux-gnu'
'host_alias=x86_64-redhat-linux-gnu'
'target_alias=x86_64-amazon-linux-gnu' 'CFLAGS=-O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -D_GNU_SOURCE -fPIC
-fwrapv  ' 'LDFLAGS= -g  ' 'CPPFLAGS= '
'PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'"
    CONFINCLUDEDIR = "/usr/include"
    CONFINCLUDEPY = "/usr/include/python3.6m"
    INCLDIRSTOMAKE = "/usr/include  /usr/include/python3.6m"
    INCLUDEDIR = "/usr/include"
    INCLUDEPY = "/usr/include/python3.6m"


I have upgraded it to python 3.8, but got similar results.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

I wrote:
> The early returns are not great: we have about half a dozen machines
> so far that are finding python3, and reporting sane-looking Python
> include paths, but not finding Python.h.  They're all Linux-oid
> machines, so I suppose what is going on is that they have the base
> python3 package installed but not python3-dev or local equivalent.

> I want to leave that patch in place long enough so we can get a
> fairly full survey of which machines are OK and which are not,
> but I suppose I'll have to revert it tomorrow or so.  We did
> promise the owners a month to adjust their configurations.

I have now reverted that patch, but I think this was a highly
worthwhile bit of reconnaissance.  It identified 18 animals
that had incomplete python3 installations (versus only 13
that definitely or possibly lack python3 altogether).  Their
owners most likely thought they were already good to go for the
changeover, so without this experiment we'd have had a whole lot
of buildfarm red when the real change is made.

I've notified the owners of these results.

            regards, tom lane

I wrote:
> Based on the buildfarm results so far, the problem can be described
> as "some installations say /usr/local when they should have said /usr".
> I experimented with the attached delta patch and it fixes the problem
> on my Debian 9 image.  (I don't know Python, so there may be a better
> way to do this.)  We'd have to also bump the minimum 3.x version to
> 3.2, but that seems very unlikely to bother anyone.

I did a little more digging into this.  The python2 package on
my Deb9 (actually Raspbian) system says it is 2.7.13, but
/usr/lib/python2.7/sysconfig.py is different from what I find in
a virgin Python 2.7.13 tarball, as per attached diff.  I conclude
that somebody at Debian decided that Python should live under
/usr/local, and changed sysconfig.py to match, but then failed
to adjust the actual install scripts to agree, because there is
certainly nothing installed under /usr/local.  (I don't know
enough about Debian packaging to find the smoking gun though;
what apt-get claims is the source package contains no trace of
this diff.)  There's no sign of comparable changes in
/usr/lib/python3.5/sysconfig.py on the same machine, either.

So I think this can fairly be characterized as brain-dead packaging
error, and we should just hack around it as per my previous patch.

In other news, I switched prairiedog and gaur to python 3.2.

            regards, tom lane

--- Python-2.7.13/Lib/sysconfig.py    2016-12-17 15:05:06.000000000 -0500
+++ /usr/lib/python2.7/sysconfig.py    2018-09-26 14:42:22.000000000 -0400
@@ -16,6 +16,26 @@ _INSTALL_SCHEMES = {
         'scripts': '{base}/bin',
         'data': '{base}',
         },
+    'posix_local': {
+        'stdlib': '{base}/lib/python{py_version_short}',
+        'platstdlib': '{platbase}/lib/python{py_version_short}',
+        'purelib': '{base}/local/lib/python{py_version_short}/dist-packages',
+        'platlib': '{platbase}/local/lib/python{py_version_short}/dist-packages',
+        'include': '{base}/local/include/python{py_version_short}',
+        'platinclude': '{platbase}/local/include/python{py_version_short}',
+        'scripts': '{base}/local/bin',
+        'data': '{base}/local',
+        },
+    'deb_system': {
+        'stdlib': '{base}/lib/python{py_version_short}',
+        'platstdlib': '{platbase}/lib/python{py_version_short}',
+        'purelib': '{base}/lib/python{py_version_short}/dist-packages',
+        'platlib': '{platbase}/lib/python{py_version_short}/dist-packages',
+        'include': '{base}/include/python{py_version_short}',
+        'platinclude': '{platbase}/include/python{py_version_short}',
+        'scripts': '{base}/bin',
+        'data': '{base}',
+        },
     'posix_home': {
         'stdlib': '{base}/lib/python',
         'platstdlib': '{base}/lib/python',
@@ -129,7 +149,7 @@ def is_python_build():
 _PYTHON_BUILD = is_python_build()

 if _PYTHON_BUILD:
-    for scheme in ('posix_prefix', 'posix_home'):
+    for scheme in ('posix_prefix', 'posix_local', 'deb_system', 'posix_home'):
         _INSTALL_SCHEMES[scheme]['include'] = '{projectbase}/Include'
         _INSTALL_SCHEMES[scheme]['platinclude'] = '{srcdir}'

@@ -163,8 +183,11 @@ def _expand_vars(scheme, vars):

 def _get_default_scheme():
     if os.name == 'posix':
-        # the default scheme for posix is posix_prefix
-        return 'posix_prefix'
+        # the default scheme for posix on Debian/Ubuntu is posix_local
+        # FIXME: return dist-packages/posix_prefix only for
+        #   is_default_prefix and 'PYTHONUSERBASE' not in os.environ and 'real_prefix' not in sys.__dict__
+        # is_default_prefix = not prefix or os.path.normpath(prefix) in ('/usr', '/usr/local')
+        return 'posix_local'
     return os.name

 def _getuserbase():
@@ -231,11 +254,19 @@ def _parse_makefile(filename, vars=None)
                     done[n] = v

     # do variable interpolation here
-    while notdone:
-        for name in notdone.keys():
+    variables = list(notdone.keys())
+
+    # Variables with a 'PY_' prefix in the makefile. These need to
+    # be made available without that prefix through sysconfig.
+    # Special care is needed to ensure that variable expansion works, even
+    # if the expansion uses the name without a prefix.
+    renamed_variables = ('CFLAGS', 'LDFLAGS', 'CPPFLAGS')
+
+    while len(variables) > 0:
+        for name in tuple(variables):
             value = notdone[name]
             m = _findvar1_rx.search(value) or _findvar2_rx.search(value)
-            if m:
+            if m is not None:
                 n = m.group(1)
                 found = True
                 if n in done:
@@ -246,23 +277,48 @@ def _parse_makefile(filename, vars=None)
                 elif n in os.environ:
                     # do it like make: fall back to environment
                     item = os.environ[n]
+
+                elif n in renamed_variables:
+                    if name.startswith('PY_') and name[3:] in renamed_variables:
+                        item = ""
+
+                    elif 'PY_' + n in notdone:
+                        found = False
+
+                    else:
+                        item = str(done['PY_' + n])
+
                 else:
                     done[n] = item = ""
+
                 if found:
                     after = value[m.end():]
                     value = value[:m.start()] + item + after
                     if "$" in after:
                         notdone[name] = value
                     else:
-                        try: value = int(value)
+                        try:
+                            value = int(value)
                         except ValueError:
                             done[name] = value.strip()
                         else:
                             done[name] = value
-                        del notdone[name]
+                        variables.remove(name)
+
+                        if name.startswith('PY_') \
+                        and name[3:] in renamed_variables:
+
+                            name = name[3:]
+                            if name not in done:
+                                done[name] = value
+
+
             else:
-                # bogus variable reference; just drop it since we can't deal
-                del notdone[name]
+                # bogus variable reference (e.g. "prefix=$/opt/python");
+                # just drop it since we can't deal
+                done[name] = value
+                variables.remove(name)
+
     # strip spurious spaces
     for k, v in done.items():
         if isinstance(v, str):
@@ -277,7 +333,7 @@ def get_makefile_filename():
     """Return the path of the Makefile."""
     if _PYTHON_BUILD:
         return os.path.join(_PROJECT_BASE, "Makefile")
-    return os.path.join(get_path('platstdlib'), "config", "Makefile")
+    return os.path.join(get_config_var('LIBPL'), "Makefile")

 # Issue #22199: retain undocumented private name for compatibility
 _get_makefile_filename = get_makefile_filename
@@ -409,7 +465,7 @@ def get_config_h_filename():
         else:
             inc_dir = _PROJECT_BASE
     else:
-        inc_dir = get_path('platinclude')
+        inc_dir = get_path('platinclude').replace("/usr/local","/usr",1)+(sys.pydebug and "_d" or "")
     return os.path.join(inc_dir, 'pyconfig.h')

 def get_scheme_names():
@@ -476,6 +532,12 @@ def get_config_vars(*args):
         # the init-function.
         _CONFIG_VARS['userbase'] = _getuserbase()

+        multiarch = get_config_var('MULTIARCH')
+        if multiarch:
+            _CONFIG_VARS['multiarchsubdir'] = '/' + multiarch
+        else:
+            _CONFIG_VARS['multiarchsubdir'] = ''
+
         if 'srcdir' not in _CONFIG_VARS:
             _CONFIG_VARS['srcdir'] = _PROJECT_BASE

Andres Freund <andres@anarazel.de> writes:
> On 2022-01-23 16:06:21 -0500, Tom Lane wrote:
>> +    'posix_local': {
>> +        'stdlib': '{base}/lib/python{py_version_short}',
>> +        'platstdlib': '{platbase}/lib/python{py_version_short}',
>> +        'purelib': '{base}/local/lib/python{py_version_short}/dist-packages',
>> +        'platlib': '{platbase}/local/lib/python{py_version_short}/dist-packages',
>> +        'include': '{base}/local/include/python{py_version_short}',
>> +        'platinclude': '{platbase}/local/include/python{py_version_short}',
>> +        'scripts': '{base}/local/bin',
>> +        'data': '{base}/local',
>> +        },
>> +    'deb_system': {
>> +        'stdlib': '{base}/lib/python{py_version_short}',
>> +        'platstdlib': '{platbase}/lib/python{py_version_short}',
>> +        'purelib': '{base}/lib/python{py_version_short}/dist-packages',
>> +        'platlib': '{platbase}/lib/python{py_version_short}/dist-packages',
>> +        'include': '{base}/include/python{py_version_short}',
>> +        'platinclude': '{platbase}/include/python{py_version_short}',
>> +        'scripts': '{base}/bin',
>> +        'data': '{base}',
>> +        },
>> 'posix_home': {

> Hm. It seems the intent of the different paths you show is that we can specify
> which type of path we want. The one to locally installed extensions, or the
> distribution ones. So we'd have to specify the scheme to get the other include
> path?

It may be that one of the other "scheme" values accurately describes
Debian's actual layout of this package.  I didn't check, because the
scheme is defined to be platform-specific.  Specifying a particular
value for it would therefore break other platforms.  Anyway, trying
to figure out whether we're on a Debian package with this mistake
doesn't seem any cleaner than what I proposed.  (In particular,
blindly changing to a different scheme without a check to see
what's really in the filesystem seems doomed to failure.)

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> The relevant part of distutils-install-layout.diff explaining this is:

> +(0)
> +   Starting with Python-2.6 Debian/Ubuntu uses for the Python which comes within
> +   the Linux distribution a non-default name for the installation directory. This
> +   is to avoid overwriting of the python modules which come with the distribution,
> +   which unfortunately is the upstream behaviour of the installation tools.

Yeah, I figured that the explanation was something like that.  Too bad
they didn't get it right.

I stopped to wonder if maybe the problem is that sysconfig.py is from the
"different distribution" that they're worried about here, but it doesn't
look like it:

tgl@rpi3:~$ dpkg -S /usr/lib/python2.7/sysconfig.py
libpython2.7-minimal:armhf: /usr/lib/python2.7/sysconfig.py
tgl@rpi3:~$ dpkg -S /usr/include/python2.7/Python.h
libpython2.7-dev:armhf: /usr/include/python2.7/Python.h

Oh well.  For a moment there I thought maybe this was a "missing
dev package" kind of problem, but it's hard to come to any other
conclusion than "packager screwed up".

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> On 2022-01-23 18:11:41 -0500, Tom Lane wrote:
>> Anyway, trying to figure out whether we're on a Debian package with this
>> mistake doesn't seem any cleaner than what I proposed.  (In particular,
>> blindly changing to a different scheme without a check to see what's really
>> in the filesystem seems doomed to failure.)

> If we make it depend on _get_default_scheme() == 'posix_local' that shouldn't
> be a risk, because that's the debian addition...

Yeah, but we don't know whether there are any versions of the Debian
packaging in which they fixed the file layout, so that 'posix_local'
actually does describe the layout.  I do not think that we are wise
to suppose we know which scheme to use without a check on what's
actually there.

I could go for "if we don't see Python.h where it's claimed to be,
try again with scheme = posix_prefix".  But I'm still not convinced
that that's noticeably cleaner than the hack I suggested.

            regards, tom lane

I wrote:
> Yeah, but we don't know whether there are any versions of the Debian
> packaging in which they fixed the file layout, so that 'posix_local'
> actually does describe the layout.

Actually ... scraping the buildfarm to see what we're currently
finding shows that the following machines are reporting that
/usr/local/include/pythonN.N really is the include directory:

conchuela
curculio
florican
gombessa
jabiru
lapwing
loach
longfin
marabou
morepork
peripatus
plover

Now, most of those are BSD machines --- but lapwing isn't.
It says

checking for python... (cached) /usr/bin/python
configure: using python 3.6.9 (default, Jan 14 2022, 06:45:55)
checking for Python distutils module... yes
checking Python configuration directory... /usr/local/lib/python3.6/config-3.6m-i386-linux-gnu
checking Python include directories... -I/usr/local/include/python3.6m
checking how to link an embedded Python application... -L/usr/local/lib -lpython3.6m -lpthread -ldl  -lutil -lrt -lm

Not sure what to make of that --- maybe that's a handmade build?

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> On 2022-01-23 18:31:44 -0500, Tom Lane wrote:
>> Yeah, but we don't know whether there are any versions of the Debian
>> packaging in which they fixed the file layout, so that 'posix_local'
>> actually does describe the layout.

> I think posix_local try to achieve something different than what you assume it
> does. It's intended to return the location to which "locally" intalled python
> extension install their files (including headers) - after having the problem
> that such local python package installations overwrite (and thus broke) files
> installed via the system mechanism.

Okay, but surely they'd have thought of packages that just want to find
out where the system Python headers are?  Having this be the default
behavior seems like it breaks as much as it fixes.  (Of course, maybe
that's why they gave up on it.)

Anyway, I don't mind trying your second suggestion

scheme = sysconfig._get_default_scheme()
# Work around Debian / Ubuntu returning paths not useful for finding python headers
if scheme == 'posix_local':
    scheme = 'posix_prefix'
sysconfig.get_path('include', scheme = scheme)

If it doesn't work everywhere, we can adjust it later.

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> I think this might be problem on our own end, actually. The distutils.sysconfig
> code did
> a = '-I' + distutils.sysconfig.get_python_inc(False)
> b = '-I' + distutils.sysconfig.get_python_inc(True)
> which the patch upthread  changed to
> +a = '-I' + sysconfig.get_path('include')
> +b = '-I' + sysconfig.get_path('platinclude')
> but I think that's possibly not quite the right translation?

I don't buy it.  The sysconfig documentation says pretty clearly
that get_path('include') and get_path('platinclude') are supposed
to return the directories we want, and there's nothing there
suggesting that we ought to magically know to look in a
non-default scheme.

(I do note that the documentation says there's no direct
equivalent to what get_python_inc does, which is scary.)

> But even so, it seems using sysconfig.get_config_vars('INCLUDEPY') or such
> seems like it might be a better translation than the above
> sysconfig.get_path() stuff?

Can you find ANY documentation suggesting that INCLUDEPY is
meant as a stable API for outside code to use?  That seems
far more fragile than anything else we've discussed, even
if it happens to work today.

I remain of the persuasion that these Debian packages are
broken.  The fact that they've not perpetuated the scheme
into their python3 packages shows that they came to the
same conclusion.  We should not be inventing usage patterns
based on a belief that it's supposed to work like this,
because what we'll mainly get out of that is failures on
other platforms.

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> No, not really. There generally seems to be very little documentation about
> what one is supposed to use when embedding python (rather than building a
> python module). The only thing I really see is:

> https://docs.python.org/3/extending/embedding.html#compiling-and-linking-under-unix-like-systems

> which says to use python-config.

Yeah :-(.  I don't really want to go there, because it will break
existing setups.  An example is that on a few machines I have
pointed the build to non-default Python installations by doing
things like
    ln -s /path/to/desired/python ~/bin/python3
So there's no matching python-config in my PATH at all.
Yeah, I can change that, but that would be a dealbreaker for
back-patching this, I think.

Getting back to the INCLUDEPY solution: I see nothing equivalent
to that for the "platform-dependent include directory".  But maybe
we don't really need that?  Not clear.

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> To avoid too noisy breakages, we could have python.m4 emit INCLUDEPY and then
> search the bf logs in a day or three?

+1, it'd give us some info without breaking the farm.

            regards, tom lane

On 24.01.22 03:53, Andres Freund wrote:
> On 2022-01-23 21:31:52 -0500, Tom Lane wrote:
>> Andres Freund<andres@anarazel.de>  writes:
>>> No, not really. There generally seems to be very little documentation about
>>> what one is supposed to use when embedding python (rather than building a
>>> python module). The only thing I really see is:
>>> https://docs.python.org/3/extending/embedding.html#compiling-and-linking-under-unix-like-systems
>>> which says to use python-config.
>> Yeah :-(.  I don't really want to go there, because it will break
>> existing setups.
> Yea, it seems to introduce a whole set of new complexities (finding python
> from python-config, mismatching python-config and explicitly specified python,
> ...). And it doesn't exist on windows either :(.

Also note that python-config is itself a Python script that uses 
sysconfig and includes code like this:

     elif opt in ('--includes', '--cflags'):
         flags = ['-I' + sysconfig.get_path('include'),
                  '-I' + sysconfig.get_path('platinclude')]

So this would just do the same thing we are already doing anyway.

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> Also note that python-config is itself a Python script that uses 
> sysconfig and includes code like this:

>      elif opt in ('--includes', '--cflags'):
>          flags = ['-I' + sysconfig.get_path('include'),
>                   '-I' + sysconfig.get_path('platinclude')]

> So this would just do the same thing we are already doing anyway.

It used to look like that, but at least in my 3.6.8 installation
on RHEL8, it's been rewritten to be a shell script that doesn't
depend on sysconfig at all.

The result is sufficiently bletcherous that you'd have thought
they'd reconsider getting rid of sysconfig :-(.  Also, it
definitely won't work on Windows.

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> ... But none of the systems report a get_python_inc(False) differing from
> get_python_inc(True), or from the value of INCLUDEPY. So I don't see a reason
> for why it'd not?

Yeah, I was just noticing that.  It looks like the whole business
with checking both get_python_inc(False) and get_python_inc(True)
has been useless from the start: none of the buildfarm animals report
more than one -I switch in "checking Python include directories".

It's a little bit too soon to decide that INCLUDEPY is reliably equal
to that, but if it still looks that way tomorrow, I'll be satisfied.

            regards, tom lane

I wrote:
> Yeah, I was just noticing that.  It looks like the whole business
> with checking both get_python_inc(False) and get_python_inc(True)
> has been useless from the start: none of the buildfarm animals report
> more than one -I switch in "checking Python include directories".

Also, that appears to be true even in the oldest runs that vendikar
still has data for, back in 2015.  So it's not something that they
cleaned up recently.

            regards, tom lane

I wrote:
> It's a little bit too soon to decide that INCLUDEPY is reliably equal
> to that, but if it still looks that way tomorrow, I'll be satisfied.

As of now, 92 buildfarm animals have reported results from f032f63e7.
Every single one of them reports that all the different methods you
tested give the same answer.  So it looks to me like we should just
go with get_config_var('INCLUDEPY') and be happy.

I guess next steps are to revert f032f63e7 and then retry e0e567a10
with that change.  Who's going to do the honors?

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> On 2022-01-25 12:45:15 -0500, Tom Lane wrote:
>> I guess next steps are to revert f032f63e7 and then retry e0e567a10
>> with that change.  Who's going to do the honors?

> I assume Peter is done working for the day. I'm stuck in meetings and stuff
> for another 2-3 hours.  I can give it a go after that, unless you do so
> before.

OK, will do.

            regards, tom lane

Hi,

Sorry I somehow missed that email.

On Mon, Jan 24, 2022 at 7:53 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Now, most of those are BSD machines --- but lapwing isn't.
> It says
>
> checking for python... (cached) /usr/bin/python
> configure: using python 3.6.9 (default, Jan 14 2022, 06:45:55)
> checking for Python distutils module... yes
> checking Python configuration directory... /usr/local/lib/python3.6/config-3.6m-i386-linux-gnu
> checking Python include directories... -I/usr/local/include/python3.6m
> checking how to link an embedded Python application... -L/usr/local/lib -lpython3.6m -lpthread -ldl  -lutil -lrt -lm
>
> Not sure what to make of that --- maybe that's a handmade build?

Yes it is.  Lapwing is a vanilla debian 7, and the backports only
offers python 3.2.  So in anticipiation to the switch to meson I
compiled the latest python 3.6, as it's supposed to be the oldest
supported version.

Andres Freund <andres@anarazel.de> writes:
> Thanks! Looks pretty good so far. Including on machines that were broken in
> take 1...

Just about all of the buildfarm has reported in now, and it's all good.
So now we need to discuss whether we want to back-patch this.

Pros: avoid configure warning now (not worth much); avoid outright
build failure on Python 3.12+ in future.

Cons: breaks compatibility with Python 2.6 and 3.1.

There are probably not many people using current Postgres builds
with 2.6 or 3.1, but we can't rule out that there are some; and
moving the compatibility goalposts in minor releases is generally
not nice.  On the other hand, it's very foreseeable that somebody
will want to build our back branches against 3.12 once it's out.

3.12 is scheduled to start beta in roughly May of 2023 (assuming
they hold to their annual release cadence, which seems like a
good bet).

The compromise I propose is to back-patch into branches that
will still be in-support at that point, which are v11 and up.
v10 will be dead, and it's perhaps a shade more likely than the
later branches to be getting used with hoary Python versions,
so I think the odds favor not changing it.

We could also wait until closer to 2023 before doing anything,
but I fear we'd forget until complaints start to show up.
I'd rather get this done while it's front-of-mind.

            regards, tom lane

[ I was hoping for more opinions, but I guess nobody cares but us ]

Andres Freund <andres@anarazel.de> writes:
> On 2022-01-27 17:53:02 -0500, Tom Lane wrote:
>> So now we need to discuss whether we want to back-patch this.
>> Pros: avoid configure warning now (not worth much); avoid outright
>> build failure on Python 3.12+ in future.
>> Cons: breaks compatibility with Python 2.6 and 3.1.

> How about adding a note about the change to this set of minor releases, and
> backpatch in the next set?

Meh.  Nobody looks at minor release notes to find out what will happen
in some other minor release.  Moreover, the sort of people who might
be adversely affected are probably not absorbing every minor release
right away, so they'd very likely not see the advance warning anyway.

> I don't see much point in worrying somebody still building plpython with 2.6,
> given its age. I feel a tad more compassion with a future self that wants to
> build a by-then EOL version of postgres, and plpython fails to build. We
> didn't commit to keeping plpython building, but it's in my default build
> script, so ...

Hmm, well, we're certainly not making this change in pre-v10 releases,
so I'm not sure that changing v10 will make things much easier for your
future self.  But it's unusual for us to make back-patching decisions
on the sort of basis I proposed here, so I'm okay with just going back
to v10 instead.

> I vote for backpatching all the way either now, or after the next set of minor
> releases is tagged.

If nobody else has weighed in by tomorrow, I'll backpatch to v10.

            regards, tom lane

Noah Misch <noah@leadboat.com> writes:
> On Mon, Jan 31, 2022 at 05:18:47PM -0500, Tom Lane wrote:
>> If nobody else has weighed in by tomorrow, I'll backpatch to v10.

> Works for me.  I agree wanting Python 3.12 w/ PG10.latest is far more likely
> than wanting Python 2.6 or 3.1.  If someone lodges a non-academic complaint,
> we could have back branches fallback to the old way if they detect a Python
> version needing the old way.  I doubt anyone will complain.

I started to do that, but paused when the patch failed on v12, which
I soon realized is because our minimum requirement before v13 was
Python 2.4 not 2.6.  That means we're moving the goalposts a bit
further in the old branches than this discussion was presuming.

I don't think this changes the conclusion any: there's still little
chance that anyone wants to build PG against such old Python versions
in 2022.  So I'm going to go ahead with patching; but does anyone want
to change their vote?  (We can always "git revert".)

            regards, tom lane