Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python buildout is broken after change to use wheels internally #12141

Closed
datakurre opened this issue Jan 4, 2016 · 29 comments
Closed

Python buildout is broken after change to use wheels internally #12141

datakurre opened this issue Jan 4, 2016 · 29 comments
Assignees
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python

Comments

@datakurre
Copy link
Contributor

I'm still struggling with namespace packages after this change #11086 to use wheels internally.

In short, why does namespaces work when package with namespace packages (e.g. zope.deprecation) is in PYTHONPATH, but fail when the same path is added into sys.path? /cc @domenkozar @cillianderoiste

Particularly, all zc.buildout generated scripts with /nix/store paths are broken because of this.

Is this different from the known setuptools issue? As a workaround, using wrapProgram for buildout generated scripts would make namespaced packages directly importable (because wrapProgram would generated PYTHONPATH and sys.path in scripts have no longer any effect). Yet, because setuptools is still broken, e.g. zope.configuration fails to find packages.

@datakurre
Copy link
Contributor Author

An another issue. For some reason I'm not able to override propagated build inputs for a Python package. My use-case is adding extra run-time dependencies for zc_buildout_nix so that it can use those instead of installing new packages (zc_buildout_nix is patched version of buildout so that it prefers existing local packages).

For example,

pythonPackages.zc_buildout_nix.overrideDerivation(args: {                  
  propagatedBuildInputs = [ pythonPackages.pyramid ];
})

would not add pyramid-packages into scripts generated from zc_buildout_nix, but adding that propagatedBuildInputs directly into the expression (pkgs/development/python-modules/buildout-nix/default.nix) would work.

@cillianderoiste Would you be able to reproduce this? You have been using buildout-nix.

@domenkozar
Copy link
Member

@datakurre for namespaces, I'll take a look as soon as I find some time. We should really fix this issue.

For overrideDerivation see http://lists.science.uu.nl/pipermail/nix-dev/2015-March/016452.html and #4855

@datakurre
Copy link
Contributor Author

@domenkozar Thanks. I got overrideDerivation working and zc.buildout-nix works now as expected.

About namespaces. For me it seems that there is no solution in sight for setuptools. Using PYTHONPATH instead of sys.path would work some use-case, but still everything using setuptools API would probably stay broken.

Do you think it would be possible to prevent wheel from excluding namespace init.py-files in bdist_wheel? (And is there still more code in PIP for preventing to install them?)

@domenkozar
Copy link
Member

That was my feeling - we'll have to dig into how namespaces work and patch setuptools (in a way hopefully that can be submitted upstream).

@datakurre
Copy link
Contributor Author

Well, if I only knew what's the real issue. So far, I've found the following related setuptools issues

I can add my proposal for 462 as a patch to fix zc.recipe.egg installation datakurre@6d863bb

I can also patch find_packages to interpret all directories containing modules under some sub directory as a namespace package: datakurre@eea32ab (well, should add a test that such directory is empty...)

Yet, I cannot make implicit namespace packages magically importable in Python 2.x. Even the proposed fixes in setuptools have only been for Python >= 3.3. Should I try importlib2? And I must be missing something, because import works for namespace packages in PYTHONPATH (even it doesn't for packages added into sys.path).

@datakurre
Copy link
Contributor Author

Possibly the difference between PYTHONPATH working and sys.path not is that PYTHONPATH processes .pth-files, which do include information about namespace packages. I'll, try to populate sys.path using site.addsitedir next (it should process pth-files).

Update: Simply replacing sys.path mutation with site.addsitedir in buildout generated scripts did not fix buildout generated Plone. Initially all packages modules are found (similarly to when using PYTHONPATH), but zope.configuration / z3c.autoinclude fails to find packages – something I initially expected to be setuptools issue, but haven't find an existing issue about it yet.

@datakurre
Copy link
Contributor Author

I think, I stop trying to patch setuptools and try to learn if wheel / pip could be set to leave init.pys into packages.

My current understanding is that there are two ways to make namespace packages work with Python < 3.3 (PEP420): 1) either each namespace package should have that special init.py declaring a namespace or 2) there should be .pth-file defining those packages. Implicit PEP420 namespace packages only work on Python >= 3.3, so therefore broken support for implicit namespace packages in setuptools is not a relevat issue for Python 2.7.

Possibly those two ways for declaring namespace packages cannot be mixed in the same package and therefore wheel / pip have opted to use .pth-files to support Python < 3.3 (which also makes it easier to manage those packages with RPM or other legacy package managers, because there are not conflictin namespace init.pys in packages).

Currently, non-buildout based Python software with namespace packages in nixpkgs work, because rely on PYTHONPATH, which does read .pth files to register namespace packages on Python 2.7. Buildout created scripts fail, because they rely on updating sys.path, which does not trigger reading .pth-files. And because Python 2.7 does not support implicit namespace packages, those packages cannot be imported. Using site.addsitedir instead of updating sys.path in buildout generated scripts could fix this, because site.addsitedir does read .pth-files.

@domenkozar Did I miss something in above?

I still don't know, why zope.configuration / z3c.autoinclude fails to locate packages (e.g. zcml in plone.app.contenttypes fails to load module from plone.app.contenttypes...). I may still to try to look into that, but probably I move to figuring out, if somehow namespace init.pys could still be used instead of .pth-files on Python 2.7.

@datakurre
Copy link
Contributor Author

Just found that z3c.autoinclude has copy of setuptools' find_packages (it reasoned that it was safer to copy the method than try to import it from setuptools) , which knew nothing about implicit namespace packages. Fixing that made nix-built Plone start.

@datakurre
Copy link
Contributor Author

@domenkozar So, finally, I see the the following options:

a) Somehow fix wheel and pip to preserve namespace markers init.pys and rely on them instead of .pth files when building and installing package for Python 2.7. This should fix everything to work as before.

b) Do nothing, but recommend wrapping buildout generated files with makeWrapper.

Obviously, z3c.autoinclude should either be fixed in upstream or patched in nixpkgs. I already have a Python 3 fixed version of that so, maybe I should try to get that merged with find_packages supporting implicit namespace packages.

Fixing find_packages in setuptools to find implicit namespace packages may not be trivial, because upstream would probably want to fix it only for Python >= 3.3. find_packages use-case in z3c.autoinclude is a bit different and affects only Python 2.7.

@domenkozar
Copy link
Member

@datakurre not sure I understand option a), can you elaborate?

@datakurre
Copy link
Contributor Author

@domenkozar About a). At least my issue seem to be that packages with namespace marker init.pys get packed and installed so that those markers get removed and replaced with "implicit" namespace packages and .pth file (to support Python < 3.3, I'd guess). Preventing that (preserving and relying namespace init.py files) should fix these issues. But I don't know yet, why wheel does that (adds .pth file and removes namespace packages), is that configurable or otherwise fixable.

@datakurre
Copy link
Contributor Author

@datakurre
Copy link
Contributor Author

In short, I'm interested, which code removes namespace markers and why. Probably for a good reason, but I'd like to know. Some setuptools command seems to have code for clearing them ("install_lib"), but haven't managed to test yet if it's it.

@domenkozar
Copy link
Member

I'll give this a deeper look in Feb.

@datakurre
Copy link
Contributor Author

@domenkozar Have you got any new ideas for this?

To summarize:

  • buildout-generated scripts are broken, because of setuptools' namespace package __init__.py's being removed by wheel packaging; workaround would be to not rely on buildout-generated scripts or wrap them to have all packages in PYTHONPATH
  • Yet, because of z3c.autoinclude using (copied) setuptools' find_packages, Plone will remain broken until __init__.py's reappear or z3c.autoinclude is fixed to find packages without namespace __init__.py's (but because that's not fixed even in setuptools itself, it's hard to guess the "right" solution without side-effects).

Yet, one workaround, you proposed, was to design something for installing Plone-requirements as a bundle (to them into a single nix package). I think, also @garbas had some similiar idea for installing a bunch of packages from requirements.txt.

I could arrange some time to work around this in the next week (the 1st week of March), but I'm not yet familiar enough with Nixpkgs' python packaging internals to get forward alone.

@domenkozar
Copy link
Member

@datakurre hopefully I'll have some time this weekend :) Thanks for being persistent, this one is important to fix.

@domenkozar domenkozar added this to the 16.03 milestone Feb 25, 2016
@domenkozar
Copy link
Member

@datakurre I'll finally have time to dig into this one, can you provide me a way to reproduce this easily? Just something I can run and debug

@datakurre
Copy link
Contributor Author

Ok. This example is nonsense, but should show the issue. Given

default.nix:

with import <nixpkgs> {};
stdenv.mkDerivation {
  name = "env";
  buildInputs = [
    (pythonPackages.zc_buildout_nix.overrideDerivation(args: {
      postInstall = "";
      propagatedNativeBuildInputs = [
        (buildPythonPackage {
          name = "zc.recipe.egg-2.0.3";
          src = fetchurl {
            url = "https://pypi.python.org/packages/source/z/zc.recipe.egg/zc.recipe.egg-2.0.3.tar.gz";
            md5 = "69a8ce276029390a36008150444aa0b4";
          };
          propagatedBuildInputs = [
            pythonPackages.zc_buildout_nix
            pythonPackages.setuptools
          ];
          installPhase = pythonPackages.setuptools.installPhase;
          doCheck = false;
        })
      ];
    }))
  ];
}

buildout.cfg:

[buildout]
parts = zc.buildout
versions = versions

[zc.buildout]
recipe = zc.recipe.egg
eggs = zc.buildout

[versions]
setuptools = 18.2
zc.recipe.egg = 2.0.3

Result

$ nix-shell --run buildout
$ bin/buildout
Traceback (most recent call last):
  File "bin/buildout", line 9, in <module>
    import zc.buildout.buildout
ImportError: No module named zc.buildout.buildout

The default.nix setup zc_buildout_nix, which is just like buildout, but always prefers /nix/store-installed eggs found in its own PYTHONPATH. The buildout.cfg just creates bin/buildout-script, which only includes paths form nix-store. bin/buildout fails, because zc is a new style namespace package, which is not supported by sys.path-approach of zc.buildout.

@datakurre
Copy link
Contributor Author

@domenkozar The easiest way to fix the above, I think, would be to opt-out from wheels/pip removing setuptools namespace __init__.pys. What I read, that removal is there only to please packaging tools, which install all packages into single tree and break when to packages own same namespace __ini__.py. Although, could be hard to get a such option to upstream.

A good enough alternative would be to provide that bundle install (installing multiple Python packages as a single Nix-package) with option to select between wheel and legacy install. That would also fix issues with circular deps.

@domenkozar
Copy link
Member

I've played our with recursive pth loader, I'm getting closer but it still doesn't work.

diff --git a/pkgs/development/python-modules/generic/default.nix b/pkgs/development/python-modules/generic/default.nix
index 1fdbd4f..fa18a23 100644
--- a/pkgs/development/python-modules/generic/default.nix
+++ b/pkgs/development/python-modules/generic/default.nix
@@ -3,7 +3,7 @@
    (http://pypi.python.org/pypi/setuptools/), which represents a large
    number of Python packages nowadays.  */

-{ python, setuptools, unzip, wrapPython, lib, bootstrapped-pip
+{ python, setuptools, unzip, wrapPython, lib, bootstrapped-pip, recursivePthLoader
 , ensureNewerSourcesHook }:

 { name
@@ -68,7 +68,7 @@ python.stdenv.mkDerivation (builtins.removeAttrs attrs ["disabled" "doCheck"] //
     ++ (lib.optional (lib.hasSuffix "zip" attrs.src.name or "") unzip);

   # propagate python/setuptools to active setup-hook in nix-shell
-  propagatedBuildInputs = propagatedBuildInputs ++ [ python setuptools ];
+  propagatedBuildInputs = [ recursivePthLoader ] ++ propagatedBuildInputs ++ [ python setuptools ];

   pythonPath = pythonPath;

diff --git a/pkgs/development/python-modules/recursive-pth-loader/sitecustomize.py b/pkgs/development/python-modules/recursive-pth-loader/sitecustomize.py
index 057e779..ccaf7f3 100644
--- a/pkgs/development/python-modules/recursive-pth-loader/sitecustomize.py
+++ b/pkgs/development/python-modules/recursive-pth-loader/sitecustomize.py
@@ -38,6 +38,8 @@ for path_idx, sitedir in enumerate(sys.path):
                     continue

                 if line.startswith(("import ", "import\t")):
+                    print line
+                    exec line
                     continue

                 line = line.rstrip()

@datakurre
Copy link
Contributor Author

I already forgot pth files. But I'm missing, how that would help buildout generated scripts.

@domenkozar
Copy link
Member

Because that's where namespace magic happens:

[nix-shell:~/dev/nixpkgs]$ cat /nix/store/7f6rf1dd1k6s6r86r743bdawm8yqjac0-python2.7-logilab-constraint-0.6.0/lib/python2.7/site-packages/logilab_constraint-0.6.0-py2.7-nspkg.pth
import sys, types, os;p = os.path.join(sys._getframe(1).f_locals['sitedir'], *('logilab',));ie = os.path.exists(os.path.join(p,'__init__.py'));m = not ie and sys.modules.setdefault('logilab', types.ModuleType('logilab'));mp = (m or []) and m.__dict__.setdefault('__path__',[]);(p not in mp) and mp.append(p)

@datakurre
Copy link
Contributor Author

And those were already read, if the package was in PYTHONPATH, but not, when it was dynamically added into sys.path. You are able to patch that?

@tadfisher
Copy link
Contributor

(triage) any updates?

@datakurre
Copy link
Contributor Author

I'll retry my example next week and report back (I haven't got any new ideas for fixing this).

@fpletz fpletz removed this from the 16.03 milestone Jan 20, 2017
@mmahut
Copy link
Member

mmahut commented Aug 10, 2019

Are there any updates to this issue, please?

@datakurre
Copy link
Contributor Author

I don’t remember why I did not report back, but I did move to use zc_buildout_nix-package and contributed that to make it work in my use cases.

That said, I still have an extra patch over it to make bin/-scripts have correct interpreter https://github.com/datakurre/nix-files/blob/master/overlays/custom/plone-env/default.nix#L68

@stale
Copy link

stale bot commented Jun 2, 2020

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 2, 2020
@Artturin
Copy link
Member

Assuming fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python
Projects
None yet
Development

No branches or pull requests

6 participants