use python_DISTLIB #1023

huangminghuang · 2023-04-12T21:37:10Z

This PR

uses ${Python_DISTLIB} as the location for installing python modules,
add CI package installation test for TestHarness.

Resolves #839

oschwaldp-oci

In light of comment: #818 (comment), the use of FindPackage(Python) at build time verses the discovery and setting of Python_SITELIB at installation time - The reason for the decision to opt for the installation time choice was to more accurately reflect and support the use case where build machine and install machine could be different and the location on the installation machine is what is important here.

ScottBailey · 2023-04-14T19:28:03Z

scripts/postinst

@@ -1,13 +1,11 @@
 #!/bin/sh

 set -e
+Python_SITELIB=$(python3 -c "from distutils import sysconfig;print(sysconfig.get_python_lib(plat_specific=False,standard_lib=False))")


As of python 3.10 (in Ubuntu 22), distutils is deprecated. It will be removed in 3.12. Additionall pain: distutils must be installed before use!

I'm not 100% certain this suggestion works in non Debian derived environments, but it works in my Debain Python 3.11 environment as well as a mostly vanilla Ubuntu 18 environment. Here is the documentation on Debian dist-packages.

Suggested change

Python_SITELIB=$(python3 -c "from distutils import sysconfig;print(sysconfig.get_python_lib(plat_specific=False,standard_lib=False))")

Python_SITELIB=$(python3 -c "import sysconfig; print(sysconfig.get_path('stdlib'))")/dist-packages

If this is not the solution, we should find a better solution than distutils.

I couldn't get that to work in Ubuntu nor Debian

# ls $(python3 -c "import sysconfig; print(sysconfig.get_path('stdlib'))")/dist-packages ls: cannot access '/usr/lib/python3.8/dist-packages': No such file or directory

I think you want platlib

# python3 -c "import sysconfig; print(sysconfig.get_path('platlib'))" /usr/lib/python3.9/site-packages

Currently, it is implemented with python3 -c "import sysconfig; print(sysconfig.get_path('purelib'))" as described in https://cmake.org/cmake/help/latest/module/FindPython3.html

According to that link, dist-packages seem to be the correct path. I wonder if we need to mkdir -p to ensure it exists. @spoonincode This worked for me in Debian 11/12 and Ubuntu 18.

purelib seems to be the correct answer. :-) Approved.

Are we sure we should be installing into site-packages?

dist-packages instead of site-packages. Third party Python software installed from Debian packages goes into dist-packages, not site-packages. This is to reduce conflict between the system Python, and any from-source Python build you might install manually.
https://wiki.debian.org/Python

ScottBailey · 2023-04-14T19:30:09Z

scripts/prerm

-# Also may have already existed, so it is acceptable to leave them in place if the rmdir fails
-rmdir --ignore-fail-on-non-empty ${CMAKE_INSTALL_FULL_LIBDIR}/python3/dist-packages
-rmdir --ignore-fail-on-non-empty ${CMAKE_INSTALL_FULL_LIBDIR}/python3
+Python_SITELIB=$(python3 -c "from distutils import sysconfig;print(sysconfig.get_python_lib(plat_specific=False,standard_lib=False))")


Same issue with distutils as noted in scripts/postinst.

ScottBailey

See comments: distutils requires install and is deprecated.

spoonincode

holding

spoonincode · 2023-04-17T22:56:42Z

When I try to install the package, I get

ln: failed to create symbolic link '/usr/lib/python3.8/site-packages/TestHarness': No such file or directory

I'm not sure if we're expected to create site-packages or what...

…-sitelib

spoonincode · 2023-04-18T16:33:58Z

It installs now but I don't think this location is searched by default...

# python3 -c 'from TestHarness import Cluster, TestHelper'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named 'TestHarness'

# ls /usr/lib/python3.8/site-packages/
TestHarness

# python3 -c 'import sys; print(sys.path)'
['', '/usr/lib/python38.zip', '/usr/lib/python3.8', '/usr/lib/python3.8/lib-dynload', '/usr/local/lib/python3.8/dist-packages', '/usr/lib/python3/dist-packages']

Don't we want this installed in to the default module search path?

ScottBailey

I think we need to look at how official Ubuntu and Debian packages set distlib.

ScottBailey · 2023-04-19T21:39:40Z

scripts/prerm

-# Also may have already existed, so it is acceptable to leave them in place if the rmdir fails
-rmdir --ignore-fail-on-non-empty ${CMAKE_INSTALL_FULL_LIBDIR}/python3/dist-packages
-rmdir --ignore-fail-on-non-empty ${CMAKE_INSTALL_FULL_LIBDIR}/python3
+Python_DISTLIB=$(python3 -c "import sysconfig; print(sysconfig.get_path('purelib'))" | sed 's/site-packages/dist-packages/')


I really don't think this is the right answer.

I agree. Something feels wrong about the trouble we're having here and that's usually a reasonable indicator we're on the wrong path.

This implementation is based on the description of https://wiki.debian.org/Python, first point of the section Deviations from upstream. In debian/Ubuntu, using the deprecated distutils.sysconfig.get_python_lib(plat_specific=False,standard_lib=False) does return the right answer /usr/lib/python3/dist-packages while using sysconfig does not. Based on the StackOverflow post

The root cause of the discrepancy is because Debian patch the distutils install layout, to correctly reflect their changes to the site, but they fail to patch the sysconfig module.

I am not sure how to move forward from here. Here are the list of solutions I can think of (I am welcome any other solutions)

just use /usr/lib/python3/dist-packages,

use distutils (at least it's still available in our supported distributions),

keep the current implementation.

I think my vote is to just use distutils and sort out the problem with python 3.12 + ubuntu when that actually arrives.

As much as I dislike using distutils, it seems like the least painful solution.

spoonincode · 2023-04-20T18:32:45Z

.github/workflows/build.yaml

+          export DEBIAN_FRONTEND=noninteractive
+          export TZ="America/New_York"
+          apt install -y ./build/leap_*.deb
+          apt install -y ./build/leap-dev*.deb


I want to keep the workflow as concise and clutter free as possible, so two changes I'd like here:

move the DEBIAN_FRONTEND and TZ to the Dockerfile

combine the two installs, either as ./build/leap_*.deb ./build/leap-dev*.deb or ./build/leap*.deb or whatever

spoonincode · 2023-04-20T18:35:05Z

.github/workflows/build.yaml

@@ -103,12 +103,21 @@ jobs:
          zstdcat build.tar.zst | tar x
          cd build
          cpack
+      - name: Install dev package
+        run: |
+          apt update


Should be update && upgrade here? I'm not sure how problematic it will be on Ubuntu to have "mixed age" packages installed: the base image is cached and could have many month old packages installed alongside new packages installed here.

use python_SITELIB

e358330

huangminghuang requested review from oschwaldp-oci and spoonincode April 12, 2023 21:37

heifner added the OCI Work exclusive to OCI team label Apr 12, 2023

oschwaldp-oci approved these changes Apr 13, 2023

View reviewed changes

heifner added this to the Leap v5.0.0-rc1 milestone Apr 13, 2023

heifner requested a review from ScottBailey April 13, 2023 21:14

update Python_SITELIB

16c4375

ScottBailey reviewed Apr 14, 2023

View reviewed changes

ScottBailey suggested changes Apr 14, 2023

View reviewed changes

huangminghuang requested a review from ScottBailey April 14, 2023 20:35

ScottBailey approved these changes Apr 17, 2023

View reviewed changes

spoonincode requested changes Apr 17, 2023

View reviewed changes

huangminghuang added 2 commits April 18, 2023 09:30

make sure Python_SITELIB exists

4b10426

Merge remote-tracking branch 'origin/main' into huangminghuang/python…

de82f95

…-sitelib

huangminghuang added 2 commits April 18, 2023 13:03

use dist-package

f74763b

add test

306e914

huangminghuang changed the title ~~use python_SITELIB~~ use python_DISTLIB Apr 18, 2023

huangminghuang requested a review from spoonincode April 18, 2023 19:09

Merge branch 'main' into huangminghuang/python-sitelib

f394de8

ScottBailey suggested changes Apr 19, 2023

View reviewed changes

spoonincode reviewed Apr 20, 2023

View reviewed changes

revert to use distutils

c81ae9a

spoonincode approved these changes Apr 25, 2023

View reviewed changes

huangminghuang requested a review from ScottBailey April 25, 2023 16:10

ScottBailey approved these changes Apr 25, 2023

View reviewed changes

huangminghuang merged commit 101f11f into main Apr 25, 2023

huangminghuang deleted the huangminghuang/python-sitelib branch April 25, 2023 16:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use python_DISTLIB #1023

use python_DISTLIB #1023

huangminghuang commented Apr 12, 2023 •

edited

Loading

oschwaldp-oci left a comment

ScottBailey Apr 14, 2023

spoonincode Apr 17, 2023

spoonincode Apr 17, 2023

huangminghuang Apr 17, 2023

ScottBailey Apr 17, 2023

ScottBailey Apr 17, 2023

oschwaldp-oci Apr 18, 2023

ScottBailey Apr 14, 2023

ScottBailey left a comment

spoonincode left a comment

spoonincode commented Apr 17, 2023

spoonincode commented Apr 18, 2023

ScottBailey left a comment

ScottBailey Apr 19, 2023

spoonincode Apr 20, 2023

huangminghuang Apr 21, 2023

spoonincode Apr 21, 2023

ScottBailey Apr 21, 2023

spoonincode Apr 20, 2023

spoonincode Apr 20, 2023

	Python_SITELIB=$(python3 -c "from distutils import sysconfig;print(sysconfig.get_python_lib(plat_specific=False,standard_lib=False))")
	Python_SITELIB=$(python3 -c "import sysconfig; print(sysconfig.get_path('stdlib'))")/dist-packages

use python_DISTLIB #1023

use python_DISTLIB #1023

Conversation

huangminghuang commented Apr 12, 2023 • edited Loading

oschwaldp-oci left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ScottBailey left a comment

Choose a reason for hiding this comment

spoonincode left a comment

Choose a reason for hiding this comment

spoonincode commented Apr 17, 2023

spoonincode commented Apr 18, 2023

ScottBailey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huangminghuang commented Apr 12, 2023 •

edited

Loading