Subject: Re: bug#52269: [core-updates-frozen] sitecustomize.py does not honor .pth files

From: Maxim Cournoyer <[email-masked]>
Date: Mon, 13 Dec 2021 14:04:49 -0500

Hello,

Lars-Dominik Braun <[email-masked]> writes:

> Hi Maxim,
>
>> +if not matching_sites:
>> +    exit(0)
> are you sure about using `exit()` here? sitecustomize.py is imported
> during startup and this would simply quit the Python interpreter if
> GUIX_PYTHONPATH is not set, wouldn’t it? (Can’t test the change
> unfortunately, because it’s a massive rebuild.)

You can test it by placing the new sitecustomize.py file in the current
directory, and then:

$ guix shell python-wrapper python-pdbpp

[env]$ $ PYTHONPATH=. GUIX_PYTHONPATH= python sample.py

where sample.py contains something like:

--8<---------------cut here---------------start------------->8---
__import__("pdb").set_trace()

print('hello')
--8<---------------cut here---------------end--------------->8---

Indeed, when GUIX_PYTHONPATH is unset or matching_sites is empty, it
exit with 0 as you expected:

--8<---------------cut here---------------start------------->8---
$ PYTHONPATH=. GUIX_PYTHONPATH= python sample.py
Fatal Python error: init_import_site: Failed to import the site module
Python runtime state: initialized
Traceback (most recent call last):
  File "/gnu/store/p5fgysbcnnp8b1d91mrvjvababmczga0-python-3.9.6/lib/python3.9/site.py", line 589, in <module>
    main()
  File "/gnu/store/p5fgysbcnnp8b1d91mrvjvababmczga0-python-3.9.6/lib/python3.9/site.py", line 582, in main
    execsitecustomize()
  File "/gnu/store/p5fgysbcnnp8b1d91mrvjvababmczga0-python-3.9.6/lib/python3.9/site.py", line 521, in execsitecustomize
    import sitecustomize
  File "/home/maxim/proj/kinova/kts_robot/sitecustomize.py", line 52, in <module>
    exit(0)
  File "/gnu/store/p5fgysbcnnp8b1d91mrvjvababmczga0-python-3.9.6/lib/python3.9/_sitebuiltins.py", line 26, in __call__
    raise SystemExit(code)
SystemExit: 0
--8<---------------cut here---------------end--------------->8---

After the proposed change:

--8<---------------cut here---------------start------------->8---
[env]$ PYTHONPATH=. GUIX_PYTHONPATH= python sample.py
> /home/maxim/proj/kinova/kts_robot/sample.py(5)<module>()
-> print('hello')
--8<---------------cut here---------------end--------------->8---

There's no longer pdbpp because of clearing GUIX_PYTHONPATH but at least
it doesn't crash :-).

>> +# Move the entries that were appended to sys.path in front of Python's own
>> +# site-packages directory.  This enables Guix packages to override Python's
>> +# bundled packages, such as 'pip'.
>> +python_site_index = sys.path.index(python_site)
>> +new_site_start_index = sys.path.index(matching_sites[0])
>> +if python_site_index < new_site_start_index:
>> +    sys.path = (sys.path[:python_site_index]
>> +                + sys.path[new_site_start_index:]
>> +                + sys.path[python_site_index:new_site_start_index])
> This is unrelated to the pdb issue, right? I see that it’s necessary
> right now, but as suggested in #46848 I’d prefer unbundling
> setuptools/pip from python. (I’ll send a v3 of the patchset at some
> point.)

Previously the Guix-provided paths were directly spliced at the right
location; now using 'site.addsitedir' simply appends them, which
requires manual fiddling afterward.

I agree that after it's un-bundled it shouldn't be necessary anymore, but
let's keep this change for core-updates along work on the 517
python-build-system (I'll try having a look to it after the next release
it out -- ping me otherwise).

Thank you,

Maxim
>From 49f0d2a493b868b9414ea10c7a676cf8404e1bca Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <[email-masked]>
Date: Fri, 3 Dec 2021 22:36:26 -0500
Subject: [PATCH] sitecustomize.py: Honor .pth files.

Fixes <https://issues.guix.gnu.org/52269>.

* gnu/packages/aux-files/python/sitecustomize.py: Use site.addsitedirs to add
the site directories; this takes care of the .pth files.  Make sure the added
items still appear before Python's own 'site-packages' directory.
---
 .../aux-files/python/sitecustomize.py         | 22 ++++++++++++++-----
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/aux-files/python/sitecustomize.py b/gnu/packages/aux-files/python/sitecustomize.py
index 71e328b9ac..e2348e0356 100644
--- a/gnu/packages/aux-files/python/sitecustomize.py
+++ b/gnu/packages/aux-files/python/sitecustomize.py
@@ -18,6 +18,7 @@
 # along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+import site
 import sys
 
 # Commentary:
@@ -47,9 +48,18 @@ all_sites_norm = [os.path.normpath(p) for p in all_sites_raw]
 matching_sites = [p for p in all_sites_norm
                   if p.endswith(site_packages_prefix)]
 
-# Insert sites matching the current version into sys.path, right before
-# Python's own site.  This way, the user can override the libraries provided
-# by Python itself.
-sys_path_absolute = [os.path.realpath(p) for p in sys.path]
-index = sys_path_absolute.index(python_site)
-sys.path[index:index] = matching_sites
+if matching_sites:
+    # Deduplicate the entries, append them to sys.path, and handle any
+    # .pth files they contain.
+    for s in matching_sites:
+        site.addsitedir(s)
+
+    # Move the entries that were appended to sys.path in front of
+    # Python's own site-packages directory.  This enables Guix
+    # packages to override Python's bundled packages, such as 'pip'.
+    python_site_index = sys.path.index(python_site)
+    new_site_start_index = sys.path.index(matching_sites[0])
+    if python_site_index < new_site_start_index:
+        sys.path = (sys.path[:python_site_index]
+                    + sys.path[new_site_start_index:]
+                    + sys.path[python_site_index:new_site_start_index])
-- 
2.34.0