guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: 03/163: build/python: Add a new guix-pythonpath procedure.


From: Hartmut Goebel
Subject: Re: 03/163: build/python: Add a new guix-pythonpath procedure.
Date: Fri, 5 Feb 2021 11:26:20 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1

Hi Maxim,

many thanks for picking up this issue.
Indeed, I thought about the possibility to filter the GUIX_PYTHONPATH
entries based on their version at runtime after I wrote my initial
reply.  It makes life easier.  I've updated the
cu/farewell-to-pythonpath branch with this new way of doing things.

I had a look at the first changes (only) and have some remarks:

1) Did I understand this correctly: `sitecustomize.py` is filtering GUIX_PYTHONPATH for all parts containing "/lib/pythonX.Y/site-packages" (with X.Y being Major.Minor)?

2) This does not remove duplicates and does not honor .pth files in the respective directories - which might still be used. Thus site.addsitedir() should be called for adding the paths. This also takes care about duplicates.

3) Empty part (…::…) are not handled.

4) Since PYTHONPATH is evaluated prior to importing sitecustomize, any sitecustominze.py in the user's path will overwrite our file, thus inhibiting our paths to be added. Not sure this is what we want in Guix.

5) When implementing the logic into site.py, the code could be simplified, Eg. You could patch a "getguixsitepackages" (based on getsitepackages) and a "addguixsitepackages" (based on addguixsitepackages) into site.py. Downside is that maybe we need different patches for different Python versions. Benefit is simpler code - no need to search the correct position in the list.

6) Please add some more comments to the code explaining the idea.


Some nitpicking:

> python_root = os.path.realpath(sys.executable).split('/bin/')[0]

There already is sys.prefix, which is also the base for the entry in sys.path (see top of site.py

> major_minor = '{}.{}'.format(sys.version_info[0], sys.version_info[1])

major_minor = '{}.{}'.format(*sys.version_info)

>sys.path = sys.path[:index] + matching_sites + sys.path[index:]

sys.path[index:index] = matching_sites


I suggest using os.path.join(), os.path.split(), os.pathsep, etc. to be forward-compatible. Imagine we want to port Guix to another platform with different separators.

--
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |




reply via email to

[Prev in Thread] Current Thread [Next in Thread]