| Message ID | cover.1689093931.git.koszko@koszko.org |
|---|---|
| Headers | show |
| Series | guix: build: python-build-system: Have applications by default ignore non-Guix libraries in user site dir | expand |
Hi, > These patches modify the python-build-system's procedure responsible for > wrapping executables. The modified proc defines a PYTHONNOUSERSITE variable > which makes Python applications disregard the user site dir when loading > libraries. if we’re patching applications like pip anyways, what stops us from just setting site.ENABLE_USER_SITE to False globally in Python’s site.py? Note that our python package currently (unfortunately) bundles and exposes pip (through the pip3 command), which would not be affected by your change to the python-pip package. Also note that we have *two* build systems for Python right now (python-build-system and pyproject-build-system) and the new flag disable-user-site? would have to be added to both, even though they share the wrap phase. Cheers, Lars
Hi, thanks for reviewing the series > > These patches modify the python-build-system's procedure responsible for > > wrapping executables. The modified proc defines a PYTHONNOUSERSITE variable > > which makes Python applications disregard the user site dir when loading > > libraries. > > if we’re patching applications like pip anyways, what stops us from > just setting site.ENABLE_USER_SITE to False globally in Python’s > site.py? I think it would need to be set to True, not False, to have the desired effect on Guix-installed pip application. However, we want our change to only affect applications installed with Guix. So that the user could theoretically still do e.g. python3 -m pip install --ignore-installed pip ~/.local/bin/pip install xmldiff Rn I don't see a better way to achieve this than patching python-build-system and applications like pip. > Note that our python package currently (unfortunately) bundles and > exposes pip (through the pip3 command), which would not be affected by > your change to the python-pip package. I haven't been aware of that, thanks. Fortunately, the bundled pip is also unaffected by the change to python-build system. So although this patch series fails to harden it, it doesn't break it either. > Also note that we have *two* build systems for Python right now > (python-build-system and pyproject-build-system) and the new flag > disable-user-site? would have to be added to both, even though they > share the wrap phase. Fair point, thanks. Should I send an updated patch series that also adds this flag to pyproject-build-system? And should I include a patch that modifies the python's bundled pip analogously to how I did with the python-pip package? Best, Wojtek
Hi, > I think it would need to be set to True, not False, to have the desired > effect on Guix-installed pip application. to clarify, the comment in site.py says set it to False to disable the feature or True to force the feature and my impression was that we want to disable the user site dir by default (i.e. disable the feature), right? > However, we want our change to only affect applications installed with > Guix. So that the user could theoretically still do e.g. > > python3 -m pip install --ignore-installed pip > ~/.local/bin/pip install xmldiff > > Rn I don't see a better way to achieve this than patching > python-build-system and applications like pip. I can still `python3 -m pip install` with the explicit `--user` switch, even when the user site dir is disabled globally via ENABLE_USER_SITE=False. The only thing that changes is the default search path. So that library will only be available if I explicitly add .local/lib/pythonX/site-packages to PYTHONPATH. Shouldn’t that also solve the original issue of Guix-installed applications picking up random libraries from the user site dir. Cheers, Lars
Hi again! > > I think it would need to be set to True, not False, to have the desired > > effect on Guix-installed pip application. > > to clarify, the comment in site.py says > > set it to False to disable the feature or True to force the feature > > and my impression was that we want to disable the user site dir by default > (i.e. disable the feature), right? Oh, you were right. For some reason I previously misunderstood what you actually wanted to change. > > However, we want our change to only affect applications installed with > > Guix. So that the user could theoretically still do e.g. > > > > python3 -m pip install --ignore-installed pip > > ~/.local/bin/pip install xmldiff > > > > Rn I don't see a better way to achieve this than patching > > python-build-system and applications like pip. > > I can still `python3 -m pip install` with the explicit `--user` > switch, even when the user site dir is disabled globally via > ENABLE_USER_SITE=False. The only thing that changes is the default > search path. So that library will only be available if I explicitly add > .local/lib/pythonX/site-packages to PYTHONPATH. It's useful to know `--user` does the job here. > Shouldn’t that also solve the original issue of Guix-installed > applications picking up random libraries from the user site dir. Yes, it should. I still see some benefits of using PYTHONNOUSERSITE env var, though. 1. The hardening can be easily disabled for a single application if some not yet known need arises[1]. 2. The change is limited to just applications — people running `python3` shall have it behave just as it used to so far. 3. As a result of 2., there's no need to explicitly add something to PYTHONPATH when using the user site dir. I'm trying to imagine what I'd expect if I were just starting to use Guix. And I believe there'd be least astonishment if both the user site dir were working out-of-the-box and the applications were working independently of what one puts in that dir. During this discussion one more idea came to mind. There might exist a different way of solving the problem. I.e. to keep user site dir enabled, then make - GUIX_PYTHONPATH take precedence over both user site dir and PYTHONPATH whenever a Guix-installed application is launched through its wrapper and - PYTHONPATH with user site dir take precedence over GUIX_PYTHONPATH in all other cases. This probably wouldn't require patching applications like pip. And would also leave the control over the PYTHONNOUSERSITE variable and the option it affects to the user. Should I try doing this? Wojtek [1] Perhaps with ENABLE_USER_SITE=False this can also be achieved by the `-S` flag to Python (although won't this approach be less reliable?). -- (sig_start) website: https://koszko.org/koszko.html fingerprint: E972 7060 E3C5 637C 8A4F 4B42 4BC5 221C 5A79 FD1A follow me on Fediverse: https://friendica.me/profile/koszko/profile ♥ R29kIGlzIHRoZXJlIGFuZCBsb3ZlcyBtZQ== | ÷ c2luIHNlcGFyYXRlZCBtZSBmcm9tIEhpbQ== ✝ YnV0IEplc3VzIGRpZWQgdG8gc2F2ZSBtZQ== | ? U2hhbGwgSSBiZWNvbWUgSGlzIGZyaWVuZD8= -- (sig_end) On Tue, 18 Jul 2023 11:41:48 +0200 Lars-Dominik Braun <lars@6xq.net> wrote: > Hi, > > > I think it would need to be set to True, not False, to have the desired > > effect on Guix-installed pip application. > > to clarify, the comment in site.py says > > set it to False to disable the feature or True to force the feature > > and my impression was that we want to disable the user site dir by default > (i.e. disable the feature), right? > > > However, we want our change to only affect applications installed with > > Guix. So that the user could theoretically still do e.g. > > > > python3 -m pip install --ignore-installed pip > > ~/.local/bin/pip install xmldiff > > > > Rn I don't see a better way to achieve this than patching > > python-build-system and applications like pip. > > I can still `python3 -m pip install` with the explicit `--user` > switch, even when the user site dir is disabled globally via > ENABLE_USER_SITE=False. The only thing that changes is the default > search path. So that library will only be available if I explicitly add > .local/lib/pythonX/site-packages to PYTHONPATH. > > Shouldn’t that also solve the original issue of Guix-installed > applications picking up random libraries from the user site dir. > > Cheers, > Lars >
Wojtek Kosior <koszko@koszko.org> writes: > Python applications used to prioritize loading their libraries from so-called > "user site dir" (usually in ~/.local/lib/python<VERSION>/site-packages). The > libraries would only be loaded from /gnu/store when not found in the user site > dir. This used to cause hard-to-diagnose bugs like [1] when a user happened to > have a similar but incompatible version of a library installed via pip. > > These patches modify the python-build-system's procedure responsible for > wrapping executables. The modified proc defines a PYTHONNOUSERSITE variable > which makes Python applications disregard the user site dir when loading > libraries. > > While this solution does harden most Python applications, it can also break a > few ones like pip that operate on the user site dir itself. To work around > that, the second patch introduces a change to pip to allow installing to the > user site directory even when PYTHONNOUSERSITE is set by the Guix-created > wrapper script. Hello, I think we can let pip just break as other distros (eg: ArchLinux and Debian) with PEP-668. https://gitlab.archlinux.org/archlinux/packaging/packages/python/-/blob/main/EXTERNALLY-MANAGED https://pythonspeed.com/articles/externally-managed-environment-pep-668/ https://peps.python.org/pep-0668/#recommendations-for-distros With usage guide towards virtual environments, guix shell, or pipx (not packaged yet). Consider other distros does the same thing, this should be safer. What do you think? 🤔
> Hello, I think we can let pip just break as other distros (eg: ArchLinux > and Debian) with PEP-668. > > https://gitlab.archlinux.org/archlinux/packaging/packages/python/-/blob/main/EXTERNALLY-MANAGED > https://pythonspeed.com/articles/externally-managed-environment-pep-668/ > https://peps.python.org/pep-0668/#recommendations-for-distros > > With usage guide towards virtual environments, guix shell, or pipx > (not packaged yet). > > Consider other distros does the same thing, this should be safer. > > What do you think? 🤔 You're right, making pip break and recommend pipx seems like the right thing to do. I opened a new issue with patches that add python-pipx (haven't done anything related to the 'EXTERNALLY-MANAGED' file yet, tho). Thanks, Wojtek -- (sig_start) website: https://koszko.org/koszko.html fingerprint: E972 7060 E3C5 637C 8A4F 4B42 4BC5 221C 5A79 FD1A follow me on Fediverse: https://friendica.me/profile/koszko/profile ♥ R29kIGlzIHRoZXJlIGFuZCBsb3ZlcyBtZQ== | ÷ c2luIHNlcGFyYXRlZCBtZSBmcm9tIEhpbQ== ✝ YnV0IEplc3VzIGRpZWQgdG8gc2F2ZSBtZQ== | ? U2hhbGwgSSBiZWNvbWUgSGlzIGZyaWVuZD8= -- (sig_end) On Sat, 22 Jul 2023 08:30:04 +0800 宋文武 <iyzsong@envs.net> wrote: > Wojtek Kosior <koszko@koszko.org> writes: > > > Python applications used to prioritize loading their libraries from so-called > > "user site dir" (usually in ~/.local/lib/python<VERSION>/site-packages). The > > libraries would only be loaded from /gnu/store when not found in the user site > > dir. This used to cause hard-to-diagnose bugs like [1] when a user happened to > > have a similar but incompatible version of a library installed via pip. > > > > These patches modify the python-build-system's procedure responsible for > > wrapping executables. The modified proc defines a PYTHONNOUSERSITE variable > > which makes Python applications disregard the user site dir when loading > > libraries. > > > > While this solution does harden most Python applications, it can also break a > > few ones like pip that operate on the user site dir itself. To work around > > that, the second patch introduces a change to pip to allow installing to the > > user site directory even when PYTHONNOUSERSITE is set by the Guix-created > > wrapper script. > > Hello, I think we can let pip just break as other distros (eg: ArchLinux > and Debian) with PEP-668. > > https://gitlab.archlinux.org/archlinux/packaging/packages/python/-/blob/main/EXTERNALLY-MANAGED > https://pythonspeed.com/articles/externally-managed-environment-pep-668/ > https://peps.python.org/pep-0668/#recommendations-for-distros > > With usage guide towards virtual environments, guix shell, or pipx > (not packaged yet). > > Consider other distros does the same thing, this should be safer. > > What do you think? 🤔
Python applications used to prioritize loading their libraries from so-called "user site dir" (usually in ~/.local/lib/python<VERSION>/site-packages). The libraries would only be loaded from /gnu/store when not found in the user site dir. This used to cause hard-to-diagnose bugs like [1] when a user happened to have a similar but incompatible version of a library installed via pip. These patches modify the python-build-system's procedure responsible for wrapping executables. The modified proc defines a PYTHONNOUSERSITE variable which makes Python applications disregard the user site dir when loading libraries. While this solution does harden most Python applications, it can also break a few ones like pip that operate on the user site dir itself. To work around that, the second patch introduces a change to pip to allow installing to the user site directory even when PYTHONNOUSERSITE is set by the Guix-created wrapper script. The third patch adds a boolean argument called disable-user-site? to python-build-system. Packagers can set this argument to #f on per-package basis to disable the hardening behavior in case it breaks some application. Note that in the long run, it might be beneficial (although more time-consuming) to leave disable-user-site? as #t everywhere and instead modify the problematic applications — as done here with python-pip. It might even be practical to only merge the first 2 patches from this series. Please note that virtualenvs and packages that operate on them are likely unaffected by this change. The initial bug doesn't even occur with virtualenvs. I tested the changes with ./pre-inst-env guix shell -C --network --no-cwd python-xmldiff coreutils python-pip pip install xmldiff==2.4 echo > ~/.local/lib/python3.10/site-packages/xmldiff/main.py xmldiff --help Without any patches, the 4th line fails. With the patches applied, the 4th line succeeds and prints xmldiff's usage info [1] https://issues.guix.gnu.org/63912 Wojtek Kosior (3): guix: build: python-build-system: Don't process user site dir gnu: python-pip: Enable user site even with PYTHONNOUSERSITE guix: build: python-build-system: Honor disable-user-site? argument gnu/packages/python-build.scm | 10 +++++++++- guix/build-system/python.scm | 2 ++ guix/build/python-build-system.scm | 27 ++++++++++++++++++--------- 3 files changed, 29 insertions(+), 10 deletions(-) base-commit: 67e22584faaa558c2a5834a5013d77660ec45e85