storaged-project / libbytesize

A tiny library providing a C "class" for working with arbitrary big sizes in bytes
GNU Lesser General Public License v2.1
20 stars 21 forks source link

Python path detection doesn't work on Debian #111

Closed vojtechtrefny closed 1 year ago

vojtechtrefny commented 2 years ago

Somehow this change broke the Debian build.

               libbytesize 2.7
             ====================

        prefix:                     /usr
        libdir:                     ${prefix}/lib/x86_64-linux-gnu
        libexecdir:                 ${exec_prefix}/libexec
        bindir:                     ${exec_prefix}/bin
        sbindir:                    ${exec_prefix}/sbin
        datadir:                    ${datarootdir}
        sysconfdir:                 /etc
        localstatedir:              /var
        docdir:                     ${datarootdir}/doc/${PACKAGE_TARNAME}

        compiler:                   gcc
        cflags:                     -g -O2 -ffile-prefix-map=/home/michael/debian/build-area/libbytesize-2.7=. -fstack-protector-strong -Wformat -Werror=format-security -DENABLE_NLS
        cppflags:                   -Wdate-time -D_FORTIFY_SOURCE=2
        ldflags:                    -Wl,-z,relro -Wl,-z,now

        Python 3 bindings:          yes
        tools:                      no
...

 /bin/mkdir -p '/home/michael/debian/build-area/libbytesize-2.7/debian/tmp/usr/local/lib/python3.10/dist-packages/bytesize'
 /usr/bin/install -c -m 644 bytesize.py __init__.py '/home/michael/debian/build-area/libbytesize-2.7/debian/tmp/usr/local/lib/python3.10/dist-packages/bytesize'

Notice how it installs the files in /usr/local despite using --prefix=/usr

Reverting this commit, I get:

<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
<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
<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
<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
<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
<string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead
 /bin/mkdir -p '/home/michael/debian/build-area/libbytesize-2.7/debian/tmp/usr/lib/python3/dist-packages/bytesize'
 /usr/bin/install -c -m 644 bytesize.py __init__.py '/home/michael/debian/build-area/libbytesize-2.7/debian/tmp/usr/lib/python3/dist-packages/bytesize'

A few warnings but installed into the proper path

Originally posted by @mbiebl in https://github.com/storaged-project/libbytesize/issues/98#issuecomment-1230233006

vojtechtrefny commented 2 years ago

Looks like something changed in Python 3.10, I originally tested the change with 3.9 where it works as expected:

$ python3.9 -c "import sysconfig; print(sysconfig.get_path('platlib', vars={'platbase': '/usr'}))"
/usr/lib64/python3.9/site-packages

but not so much with 3.10

$ python3.10 -c "import sysconfig; print(sysconfig.get_path('platlib', vars={'platbase': '/usr'}))"
/usr/local/lib64/python3.10/site-packages

so it looks like we need a different approach.

Btw. it's interesting that we don't see this in Fedora, we should have the same problem with RPM packaging on Fedora 36 and newer.

vojtechtrefny commented 2 years ago

I am not going to pretend I fully understand what happened in Python 3.10, but the root of the problem is that both Debian and Fedora change the sysconfig module to always return /usr/local path (or to be more precise {platbase}/local/{platlibdir}) so even if we specify our own platbase we'll get /usr/local anyway (Fedora downstream patch, Debian has a similar one).

We probably need a different approach when getting the Python path than this, I'll try to ask someone from our Python team for help. Reverting back to the distutils module is probably a "good enough" solution for now.

mbiebl commented 2 years ago

Nod, I'll probably revert this particular commit downstream for now until a proper solution has been found.

mbiebl commented 2 years ago

btw, this will affect libblockdev as well once https://github.com/storaged-project/libblockdev/commit/feaec37ae5f32b6ae1ff638ceffcef762374df10 is merged into 2.x-branch

vojtechtrefny commented 2 years ago

Fedora decided to change the sysconfig behaviour, because it broke other things too (rhbz2026979 and rhbz2097183) and the code from my comment above now works with Python 3.10 too. So I think the behaviour in Debian should also probably be considered to be a bug. I might be able to come up with some fix in libbytesize (we still have some time before distutils removal in 3.12) but so far I didn't find anything useful. (I guess I could hardcode the site-packages because getting platstdlib seems to be working everywhere and returns the /usr/lib64/python3.10 path with the correct prefix, but I don't like this solution.)

stefanor commented 1 year ago

Fedora's change does look somewhat related, but definitely not the same.

Without looking at your code in detail, I'd suggest explicitly selecting the posix_prefix scheme, with the supplied prefix.

vojtechtrefny commented 1 year ago

Without looking at your code in detail, I'd suggest explicitly selecting the posix_prefix scheme, with the supplied prefix.

Using the posix_prefix scheme looks promising, it uses the correct prefix, but now I get /usr/lib/python3.11/site-packages and (correct me if I am wrong) we need /usr/lib/python3.11/dist-packages on Debian.

In [1]: import sysconfig

In [2]: sysconfig.get_path('platlib', vars={'platbase': '/usr'})
Out[2]: '/usr/local/lib/python3.11/dist-packages'

In [3]: sysconfig.get_path('platlib', vars={'platbase': '/usr'}, scheme='posix_prefix')
Out[3]: '/usr/lib/python3.11/site-packages'
stefanor commented 1 year ago

That's true. Within Debian's build system, that wouldn't be an issue, because we'd move things around to work, but you want to work outside that, too.

So, yeah, if you want to select dist-packages, you'll need to detect the presense of deb_system and use it instead of posix_prefix.

vojtechtrefny commented 1 year ago

I looked at the Debian patch again and if I understand it correctly we might not need to change anything:

+        # default to /usr for package builds, /usr/local otherwise
+        deb_build = os.environ.get('DEB_PYTHON_INSTALL_LAYOUT', 'posix_local')
+        if deb_build in ('deb', 'deb_system'):
+            prefix_scheme = 'deb_system'

so inside the Debian build system the scheme should default to deb_system which is what we want:

$ DEB_PYTHON_INSTALL_LAYOUT="deb" python3 -c "import sysconfig; print(sysconfig.get_path('platlib', vars={'platbase': '/usr'}))"
/usr/lib/python3/dist-packages
mbiebl commented 1 year ago

@vojtechtrefny I somehow missed your last comment but on #debian-devel it was also mentioned that DEB_PYTHON_INSTALL_LAYOUT="deb" can be used to influence the behaviour of python on Debian. I'll do that via debian/rules. So, feel free to close this issue.

See also https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54412

vojtechtrefny commented 1 year ago

I wish there was a way to make this work in our code, but if the DEB_PYTHON_INSTALL_LAYOUT="deb" works I think we can close this.