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

[Maxim Cournoyer]

Hi,

I've cherry-picked this commit to the version-1.4.0 branch.  I'll amass
some fixes there and then later have Cuirass build it.

Closing.

Thanks for the review!

Maxim

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

[Maxim Cournoyer]

Hello!

Lars-Dominik Braun  writes:

> Hi Maxim,
>
>> You can test it by placing the new sitecustomize.py file in the current
>> directory, and then:
> that works, thanks!
>
>> 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).
> Sure.
>
>> +# 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])
> One more nitpick: list.index() will raise a ValueError if the requested
> value does not exist. I believe setting GUIX_PYTHONPATH=/nonexistent
> will trigger this.

It doesn't break when I try it here:

$ PYTHONPATH=. GUIX_PYTHONPATH=/nonexistent python sample.py

Also, messing with GUIX_PYTHONPATH is something users shouldn't do
unless they really know what they are doing, in my opinion.  It's
intended as Guix's own mechanism to discover Python packages.  Users can
and should still use PYTHONPATH if they want to mess with Python's
module search path.

Thank you!

Maxim

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

[Lars-Dominik Braun]

Hi Maxim,

> You can test it by placing the new sitecustomize.py file in the current
> directory, and then:
that works, thanks!

> 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).
Sure.

> +# 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])
One more nitpick: list.index() will raise a ValueError if the requested
value does not exist. I believe setting GUIX_PYTHONPATH=/nonexistent
will trigger this.

Cheers,
Lars

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

[Maxim Cournoyer]

Hello,

Lars-Dominik Braun  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 
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 

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)()
-> 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 
Date: Fri, 3 Dec 2021 22:36:26 -0500
Subject: [PATCH] sitecustomize.py: Honor .pth files.

Fixes .

* 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 .
 
 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 

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

[Maxim Cournoyer]

Hi Ludovic,

Ludovic Courtès  writes:

> Hello Maxim,
>
> Maxim Cournoyer  skribis:
>
>>>From 762357609270ab016236d22999ae5cfc3fe4ff28 Mon Sep 17 00:00:00 2001
>> From: Maxim Cournoyer 
>> Date: Fri, 3 Dec 2021 22:36:26 -0500
>> Subject: [PATCH] sitecustomize.py: Honor .pth files.
>>
>> Fixes .
>>
>> * 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.
>
> I had completely overlooked this patch.
>
> Lars had useful comments about it.
>
> Do we need to address this before we merge ‘core-updates-frozen’ into
> ‘master’?

The only reason I'm on the fence about it is that it causes a big
rebuild.  But rebuilding aside, I believe it'd be nice to have it in.
I've only spotted one package affected so far (python-pdbpp), but there
may be others.

> If so, what changes need to be made to the patch before it can be
> applied?

I'll try having a look today.

Thanks,

Maxim

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

[Ludovic Courtès]

Hello Maxim,

Maxim Cournoyer  skribis:

>>From 762357609270ab016236d22999ae5cfc3fe4ff28 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer 
> Date: Fri, 3 Dec 2021 22:36:26 -0500
> Subject: [PATCH] sitecustomize.py: Honor .pth files.
>
> Fixes .
>
> * 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.

I had completely overlooked this patch.

Lars had useful comments about it.

Do we need to address this before we merge ‘core-updates-frozen’ into
‘master’?

If so, what changes need to be made to the patch before it can be
applied?

TIA!

Ludo’.