python3.buildEnv: support extending NIX_PYTHONPATH

[yajo]

Allow devshells to extend $NIX_PYTHONPATH, to effectively support nested virtualenvs.

@moduon MT-1075

Description of changes

I'm trying to set up a devshell where:

• Most python dependencies are pure.
• Some python modules I'm developing in a meta-repo are impure.

In this specific scenario, some pure dependencies depend on impure ones.

For example, I'm developing odoo and some addons, but while doing so I want to have click-odoo-contrib at my fingertips while developing. Thus, I add it to my devshell. Sounds nice, but when I try to execute it, I get an ImportError:

➤ click-odoo-listdb --help
Traceback (most recent call last):
  File "/nix/store/p8ivjzvbbkm6jqc9vyvhh6r6p4qvv2qj-python3.10-click-odoo-contrib-1.17.0/bin/.click-odoo-listdb-wrapped", line 6, in <module>
    from click_odoo_contrib.listdb import main
  File "/nix/store/r6djzp9sdnmgpaz9k413y1jdhn9aizr5-odoo-dev-16.0-python3-3.10.12-env/lib/python3.10/site-packages/click_odoo_contrib/listdb.py", line 6, in <module>
    import click_odoo
  File "/nix/store/r6djzp9sdnmgpaz9k413y1jdhn9aizr5-odoo-dev-16.0-python3-3.10.12-env/lib/python3.10/site-packages/click_odoo/__init__.py", line 4, in <module>
    from .compat import odoo  # noqa
  File "/nix/store/r6djzp9sdnmgpaz9k413y1jdhn9aizr5-odoo-dev-16.0-python3-3.10.12-env/lib/python3.10/site-packages/click_odoo/compat.py", line 11, in <module>
    import odoo
ModuleNotFoundError: No module named 'odoo'

That's expected. Odoo is installed as an editable dependency in an impure venv, because I'm developing it.

How could one pure package, packaged by nix, such as click-odoo-contrib, import an impure one installed in a venv in editable mode? The solution is to customize the site. It turns out that's already handled by Nix in these lines:

nixpkgs/pkgs/development/interpreters/python/sitecustomize.py

Lines 20 to 22 in 8fbb1f3

	paths = os.environ.pop('NIX_PYTHONPATH', None)
	if paths:
	functools.reduce(lambda k, p: site.addsitedir(p, k), paths.split(':'), site._init_pathinfo())

As you can see in that code, it supports multiple paths separated by colon.

However, without this patch, any binary produced from a Nix python environment will ignore any NIX_PYTHONPATH variable and replace it with what it wants.

Now, after this is done, I can just add to my devshell an environment variable in the shape of NIX_PYTHONPATH = "$PRJ_ROOT_DIR/.venv/${python.sitePackages}", and magically click-odoo-listdb will be able to import odoo if it is installed into that venv.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[yajo]

Fixed in #297628 using a different strategy.

[yajo]

I see that #297628 was reverted in #302385, and there's a new attempt to fix it in #326094. Therefore, I think this alternate proposal still is relevant and I reopened and rebased.

[yajo]

Answering to #326094 (comment) here to avoid polluting that other thread too much:

well, I am not sure

Fair point 😆

Well, let's be more accurate and say it fixed the problem for me while not presenting any incompatibilities, and I tested the fix in depth.

Of course, if you happen to be exporting a NIX_PYTHONPATH variable in your devshell, then yes, this will alter the behavior. But if you were doing that, then it's maybe because you were hoping it fixed this problem, so the side effect would still probably be that we'd fix something, which should be a good thing too.

FWIW, in my case I can export that variable in a devshell pointing to python dependencies from nix, and still create a venv, install editable packages on it, and be able to work, mixing all nix and impure dependencies in the venv.

OTOH I can't find a reason why NIX_PYTHONPATH should be set instead of prefixed. 🤔

[FRidh]

To not break with nested environments. When you use prefixing or suffixing you can mix packages causing breakage.

Check the sitecustomize.py. It also unsets the variables because they should not leak further.

[yajo]

Well, but that's the issue I'm fixing here. Basically I'm adding support for nested environments. El jue, 8 ago 2024, 16:51, Frederik Rietdijk ***@***.***> escribió:

…

To not break with nested environments. When you use prefixing or suffixing you can mix packages causing breakage. Check the sitecustomize.py. It also unsets the variables because they should not leak further. — Reply to this email directly, view it on GitHub <#261801 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHNXDMGRLAWX2AXEPOJ3XTZQOAW3AVCNFSM6AAAAAA6FJFV32VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZWGAZDINRTGM> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[FRidh]

That's why this cannot go in. It is set on purpose.

The NIX_PYTHONPATH is an internal implementation, it is not meant to be exposed. Users can just set the regular PYTHONPATH if they wish to do so.

[yajo]

Sorry I don't understand that last comment. Do you mean that not supporting nested python environments is a feature? Could you please explain the benefits of not being able to do this?

{python3, ...}: let
  py3 = python3.withPackages (ps: [ps.ansible])
in
  py3.withPackages (ps: [ps.werkzeug])

[phanirithvij]

This should target staging branch as it is causing a mass rebuild.

Read the links I provided here #334825 (comment)
I suggest going through https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md fully.

[yajo]

Thanks, done.

[yajo]

Conflicts solved. This is still relevant IMHO.