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

Add manylinux support #18

Merged
merged 14 commits into from
Dec 18, 2019
Merged

Add manylinux support #18

merged 14 commits into from
Dec 18, 2019

Conversation

gilligan
Copy link
Contributor

@gilligan gilligan commented Dec 14, 2019

Description

This PR adds manylinux support to poetry2nix. It depends on a nixpkgs revision that has the following changes:

Additional Info

  • selectWheel: The added pep425.nix provides a selectWheel function which picks a preferred .whl based on python version, arch and some heuristics. However selectWheel is only ever used if no source distribution is available.

  • mkPoetryEnv: Provides a python environment with the packages specified by poetry. This is useful in scenarios when you don't want to actually build a python application per se but instead want to provide an python environment to work with.

  • Refactoring The PR also contains quite some refactoring which aims to make the code base easier to use with. On several occasions code was extracted, let bindings reduced or some functions could be replaced with builtin or lib functions.


Some additional work was required to get this over the finish line...

  1. The first thing that needed to be extended was the parsing of file names according to PEP425. Files using py2 or py3 and especially py2.py3 would not be picked up. I fixed all of that and added proper tests for those scenarios.
  2. Furthermore we added mkPoetryEnv where all packages specified in the poetry lock file are readily available. If you use mkPoetryPython without actually forcing/referencing to some poetry dependency you will get a vanilla python package instead.
  3. I noticed that all over the project there was code passing poetryLock to mkPoetryApplication which doesn't even exist. However the combination of a default argument and ... for additional arguments meant this remained unnoticed. I removed the default arguments and also removed src from mkPoetryPython and mkPoetryEnv which both don't need it.
  4. I also updated the README.. (I turned it into markdown..hope that's ok..)

@gilligan
Copy link
Contributor Author

gilligan commented Dec 15, 2019

@zimbatm @adisbladis care to have a quick look already? Especially the refactoring

@andir and I put some effort into it and there is a small part left still. Obviously this will get harder to keep up to date once more stuff gets merged. I think the refactoring should be quite beneficial and make the code easier to work with..

Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a quick pass

default.nix Outdated Show resolved Hide resolved
lib.nix Outdated Show resolved Hide resolved
lib.nix Outdated Show resolved Hide resolved
gilligan and others added 4 commits December 16, 2019 15:02
Provides a `select` function that picks the most appropriate `.whl` file
based on platform, arch, python version and manylinux/OSX - version.

- Adds unit tests for pep425
- Adds test for manylinux support
- Fixes overrides: `passhtru` must be extended, not overwritten
- Don't strip python packages: this screws up manylinux binaries
- Extract utility functions to ./lib.nix
- Extract buildPythonPackage to ./mk-poetry-dep.nix
- Replace some functions with existing builtin functions
- Use lib.partition for python package splitting
- Drop package filtering on the top-level and leave it to pep425.nix
@gilligan gilligan changed the title WIP: manylinux support Add manylinux support Dec 16, 2019
As per NixOS/nixpkgs#75763 this environment
variable is not needed anymore. Instead python interpreters are now
assuming compatibility by default.
The manylinux support works but needs to remain disabled until the
necessary nixpkgs changes have been merged and propagated.
lib.nix Show resolved Hide resolved
@FRidh
Copy link
Contributor

FRidh commented Dec 17, 2019

haven't checked in detail but on a glance this looks good to me.

gilligan and others added 6 commits December 17, 2019 12:38
Drp `poetrylock` and `pyproject` mandatory arguments
without default values and drops `src` from `mkPoetryPyhon`.

Fix test invocations with invalid arguments.
Make sure that whl files using notation such as "py2|py3|py2.py3"
and/or files using "any" in the `abi` tag are accepted as well.
@gilligan gilligan force-pushed the manylinux-support branch 2 times, most recently from 29915be to d8eb359 Compare December 17, 2019 20:07
- Add info on mkPoetryApplication
- Add info on mkPoetryEnv
- Add info about manylinux
@adisbladis adisbladis merged commit 90198af into master Dec 18, 2019
@zimbatm zimbatm deleted the manylinux-support branch December 18, 2019 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants