Add optional manylinux1 (PEP 513) support for Python #55812

thomasjm · 2019-02-15T09:37:10Z

Motivation for this change

PEP 513 tries to make the task of distributing Python wheels easier by defining a list of "core" shared libraries that should be available on any Linux system. If your system contains these libraries, then you are considered manylinux1 compliant and you can install these wheels.

Until now, Nix's Python distributions have not been manylinux1 compliant, but it would be nice if it were possible. This is primarily useful if you are using Nix to install pip and then using pip to install additional packages outside of Nix. (cf. #51641)

This PR isn't entirely complete -- I'm not sure the right way to expose this option and looking for feedback. Currently the idea is that you can make a manylinux1-compliant python.withPackages like this:

myWithPackages = f: let packages = f pythonPackages; in
  python.buildEnv.override {
    extraLibs = packages;
    manylinux1 =true;    
};

There would also need to be a way to re-implement the restriction that prevents attempting to install manylinux1 wheels when it isn't enabled; we can't just bake it into the Python build.

There's some discussion about this restriction in #18484.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- tested this idea on Ubuntu
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[N/A] Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
[] Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Assured whether relevant documentation is up to date
Fits CONTRIBUTING.md.

thomasjm · 2019-02-15T09:38:28Z

Figured I'd send you another one @FRidh :)

FRidh · 2019-02-15T09:51:22Z

Related issue #18484

FRidh

I think it's important to better define the goal. Is it to be able to run manylinux1 packages, or is it to build manylinux1 packages?

If you want to run them, I suggest instead of extending the buildPythonPackage to add the autoPatchelfHook as setupHook to your manylinux environment. That way, you can use buildPythonPackage with format = "wheel"; and just add your manylinux env to the buildInputs and everything will work.

If you want to build them, well, then we need to create a function to create a wheel (split it out of buildPythonPackage) and disable patchelf.

pkgs/development/interpreters/python/manylinux1.nix

FRidh · 2019-02-15T09:47:42Z

pkgs/development/interpreters/python/manylinux1.nix

+  # The desired libraries can be collectively found in these packages
+  libs = [glib libGL ncurses5 xorg.libX11 xorg.libXrender xorg.libXext xorg.libICE xorg.libSM glibc gcc7.cc];
+
+  package = stdenv.mkDerivation {


consider using runCommand

Done, please check that it's reasonable to pass in buildInputs this way

pkgs/development/interpreters/python/mk-python-derivation.nix

thomasjm · 2019-02-15T10:27:43Z

The goal is to create a Python environment that has pip and use it to install manylinux1 wheels. Then when these packages are imported within that Python environment, they should find the libraries they need.

I would say building such wheels is out of scope, it's a different problem. (Not sure if anyone would want it.)

Yeah, maybe adding this to mk-python-derivation.nix isn't the right thing. What I'm hoping to find is a good way to extend python.withPackages. What I'd like to be able to express is

myPython = python.withPackagesAndManyLinuxSupport (ps: with ps; [ pip etc ])

but currently python.withPackages doesn't offer a way to add parameters.

thomasjm · 2019-02-15T10:29:48Z

BTW if there were a modified version like python.withPackagesArgs that accepts parameters, it would also be convenient to pass the permitUserSite arg from the previous PR that way :). Since python.withPackages is just a shallow wrapper around the builder it seems convenient to expose more of the builder's args through it.

Anyway, thanks for the review so far! I'll look into your suggestions tomorrow.

FRidh · 2019-02-15T10:32:34Z

version like python.withPackagesArgs that accepts parameters,

that's python.buildEnv

thomasjm · 2019-02-15T10:35:21Z

Right exactly, see my edit above haha. Curious about the idea you mentioned to not add this to the builder.

thomasjm · 2019-02-22T12:51:47Z

Okay @FRidh , just responded to your review.

I also implemented an idea for how manylinux1 can be selectively disabled. According to PEP 513 you signal whether it's supported by creating a module called _manylinux as part of the Python environment. I moved the place where this happens from being baked into the Python builds (in cpython/2.7/default.nix etc.) and instead happens in the builder (specifically in wrapper.nix). Thus manylinux1 support is a property of the Python environment, which I think makes more sense anyway.

FRidh

I had a look at it, and it's going in the right direction, but there's still changes to be made (more than what I commented on). As branch-off is happening soon there are more important things that require my attention, so I'll probably won't be able to give further feedback in the coming week (or two).

pkgs/development/interpreters/python/manylinux1.nix

pkgs/development/interpreters/python/mk-python-derivation.nix

thomasjm · 2019-04-19T10:04:38Z

@FRidh just responded to your latest two comments. Now that 19.03 is released, any chance you have time to help me push this over the finish line? :)

FRidh · 2019-04-28T08:00:12Z

See https://github.com/FRidh/nixpkgs/tree/manylinux1. I split it into two commits and removed the buildEnv part. The Python package might actually need to be moved back to a normal non-Python package because....well, it's not a Python package.

The question is now, how do you want to use it? If you open e.g. a nix-shell with Python packages and this metapackage with libraries, then when pip installing a wheel it won't find those libraries during runtime. More is needed for that.

thomasjm · 2019-05-10T03:03:38Z

Sorry for the delay, I had to regain context on this.

I think it was wrong to remove the makeWrapperArgs stuff in my latest response to your review. (I removed it here.) As a reminder, I had added this code to mk-python-derivation.nix:

makeWrapperArgs = makeWrapperArgs ++ (lib.optionals manylinux1 [	
  "--set"	
  "LD_LIBRARY_PATH"	
  (lib.makeLibraryPath [(lib.callPackage ./manylinux1.nix {}).package])	
]);

The idea is to set LD_LIBRARY_PATH in the resulting Python environment so that it contains the manylinux1 libraries, so that things work at runtime. The way I've been using this is like this:

myWithPackages = f: let packages = f pythonPackages; in
  python.buildEnv.override {
    extraLibs = packages;
    manylinux1 = true;
  };

myPythonEnvironment = myWithPackages (ps: [ps.pip])

So, in the resulting myPythonEnvironment, I can use pip to install manylinux1 wheels.

Thus, I think the missing piece is the change to mk-python-derivation.nix. Plus the change to selectively disable manylinux1 in Python environments. What do you think? I'll clean up this PR to show what I think the final version should be.

FRidh · 2019-05-12T18:14:28Z

No, nothing needs to be added to buildEnv or buildPythonPackage. If you take my branch, and add the autopatchelfhook there as setupHook, then it should work fine.

thomasjm · 2019-05-13T10:12:15Z

Confused, I'm looking at https://github.com/FRidh/nixpkgs/tree/manylinux1 and not seeing any autoPatchelfHook in the 2 commits there. Could you explain further?

thomasjm · 2019-05-18T01:06:01Z

Friendly ping @FRidh , I still need some clarification on what you mean before I can proceed.

Quarub · 2019-05-29T20:36:28Z

Confused, I'm looking at https://github.com/FRidh/nixpkgs/tree/manylinux1 and not seeing any autoPatchelfHook in the 2 commits there. Could you explain further?

@thomasjm I guess FRidh wants you to add something like setupHook = ''autoPatchelfHook''; into the [...]/manylinux1/default.nix file in FRidh's branch. Then use the branch for the merge request (?)

Anyway, using the branch with the modification I was able to build some derivations I was previously not able to. Thanks to both of you.

thomasjm · 2019-05-30T04:33:58Z

Glad to help @Quarub. Is your use-case building manylinux1-compatible wheels? I'm wondering if you have any opinions on how the final version of this should look--for example, if we add the setupHook you mentioned and land that branch, would that be enough for you to use it?

Quarub · 2019-05-30T13:31:31Z

I wanted to install a library from Pypi and ran into X is not a supported wheel on this platform and ended up here (I only saw manylinux1 wheels available to download). I'm afraid @thomasjm I'm not familiar with building wheels.

thomasjm · 2019-05-31T01:02:03Z

No worries @Quarub -- I'm just trying to be clear what exactly you did to make things work so I can make sure this gets packaged up in a nice way for your use-case. Any details you can provide to spell out how you made it work for you would be helpful.

Quarub · 2019-06-01T20:20:05Z

Sorry @thomasjm , I had to deal with some tasks.
Now to what I did:

Run git clone https://github.com/FRidh/nixpkgs.git -b manylinux1
Add setupHook = ''autoPatchelfHook''; to /to/where/you/cloned/nixpkgs/pkgs/development/python-modules/manylinux1/default.nix (what FRidh suggested to do).
Run nix-shell -I nixpkgs=/to/where/you/cloned/nixpkgs in the directory with the following files:
pack.nix
nes-py.nix
gymmario.nix
opencv4.nix
shell.nix
It will take some time due to the rebuild needed (as shown in the labels of this request).
In the nix-shell run the first example of the readme from the gym-super-mario-bros repo
See Mario jump around randomly in a small window.

Btw. if you spot something I could improve in the expressions, please tell me. For example I just figured that I could replace <nixpkgs> in pack.nix with /to/where/you/cloned/nixpkgs so I don't need to use the -I option.

thomasjm · 2019-07-14T08:21:09Z

Thanks for the details @Quarub . Maybe I'm missing something obvious but I'm still a little confused by how that worked for you, since none of the files you posted seem to actually mention manylinux1/default.nix. Or maybe I just really don't understand setup hooks :P. Any clarification would be appreciated.

@FRidh , I'm thinking about closing this PR since we haven't been able to come up with a story for how to package it up. For my own purposes I can simply keep a copy of the manylinux1 file in my own code and create a manylinux1-capable environment like this:

    python.buildEnv.override {
      extraLibs = [my packages];
      makeWrapperArgs = ["--set" "LD_LIBRARY_PATH" (makeLibraryPath manylinux1.libs)];
    };

The ability to make an environment like this is all I ever wanted. It allows me to pip install a manylinux1 wheel and then successfully open a Python repl and import it. I still don't fully understand your proposed setup hook/pseudo Python package solution--it doesn't seem like a complete solution to me. So please let me know if you're interested in keeping this PR around, if not I'll close.

disassembler · 2019-09-01T02:38:22Z

this is stale and not looking like it's something we want to do at this time. closing.

thomasjm requested a review from FRidh as a code owner February 15, 2019 09:37

GrahamcOfBorg added the 6.topic: python label Feb 15, 2019

FRidh requested changes Feb 15, 2019

View reviewed changes

thomasjm force-pushed the manylinux1 branch from 71bacbd to be178c1 Compare February 22, 2019 02:46

GrahamcOfBorg requested a review from FRidh February 22, 2019 13:03

GrahamcOfBorg added 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ labels Feb 22, 2019

FRidh requested changes Feb 22, 2019

View reviewed changes

pkgs/development/interpreters/python/manylinux1.nix Outdated Show resolved Hide resolved

pkgs/development/interpreters/python/mk-python-derivation.nix Show resolved Hide resolved

FRidh added the 2.status: work-in-progress label Feb 22, 2019

GrahamcOfBorg added the 8.has: package (new) label Apr 19, 2019

GrahamcOfBorg requested a review from FRidh April 19, 2019 10:11

thomasjm added 3 commits May 9, 2019 20:09

Add a file containing all the manylinux1 libraries

12154b1

Switch from blanket disabling manylinux1 support to selectively enabling

f7e2bb5

Be able to generate Python environments with runtime manylinux1 support

728e17b

thomasjm force-pushed the manylinux1 branch from 1149804 to 728e17b Compare May 10, 2019 03:14

kierdavis mentioned this pull request Jul 26, 2019

Dependencies packaged as manylinux1 wheels cannot be installed on Nix srobo/j5#352

Closed

disassembler closed this Sep 1, 2019

FRidh mentioned this pull request Nov 21, 2019

manylinux packages for Python #73866

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional manylinux1 (PEP 513) support for Python #55812

Add optional manylinux1 (PEP 513) support for Python #55812

thomasjm commented Feb 15, 2019

thomasjm commented Feb 15, 2019

FRidh commented Feb 15, 2019

FRidh left a comment

FRidh Feb 15, 2019

thomasjm Feb 22, 2019

thomasjm commented Feb 15, 2019

thomasjm commented Feb 15, 2019 •

edited

Loading

FRidh commented Feb 15, 2019

thomasjm commented Feb 15, 2019

thomasjm commented Feb 22, 2019

FRidh left a comment

thomasjm commented Apr 19, 2019

FRidh commented Apr 28, 2019

thomasjm commented May 10, 2019 •

edited

Loading

FRidh commented May 12, 2019

thomasjm commented May 13, 2019

thomasjm commented May 18, 2019

Quarub commented May 29, 2019

thomasjm commented May 30, 2019

Quarub commented May 30, 2019

thomasjm commented May 31, 2019

Quarub commented Jun 1, 2019 •

edited

Loading

thomasjm commented Jul 14, 2019

disassembler commented Sep 1, 2019

Add optional manylinux1 (PEP 513) support for Python #55812

Add optional manylinux1 (PEP 513) support for Python #55812

Conversation

thomasjm commented Feb 15, 2019

Motivation for this change

Things done

thomasjm commented Feb 15, 2019

FRidh commented Feb 15, 2019

FRidh left a comment

Choose a reason for hiding this comment

FRidh Feb 15, 2019

Choose a reason for hiding this comment

thomasjm Feb 22, 2019

Choose a reason for hiding this comment

thomasjm commented Feb 15, 2019

thomasjm commented Feb 15, 2019 • edited Loading

FRidh commented Feb 15, 2019

thomasjm commented Feb 15, 2019

thomasjm commented Feb 22, 2019

FRidh left a comment

Choose a reason for hiding this comment

thomasjm commented Apr 19, 2019

FRidh commented Apr 28, 2019

thomasjm commented May 10, 2019 • edited Loading

FRidh commented May 12, 2019

thomasjm commented May 13, 2019

thomasjm commented May 18, 2019

Quarub commented May 29, 2019

thomasjm commented May 30, 2019

Quarub commented May 30, 2019

thomasjm commented May 31, 2019

Quarub commented Jun 1, 2019 • edited Loading

thomasjm commented Jul 14, 2019

disassembler commented Sep 1, 2019

thomasjm commented Feb 15, 2019 •

edited

Loading

thomasjm commented May 10, 2019 •

edited

Loading

Quarub commented Jun 1, 2019 •

edited

Loading