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

manylinux packages for Python #73866

Merged
merged 1 commit into from Dec 5, 2019
Merged

manylinux packages for Python #73866

merged 1 commit into from Dec 5, 2019

Conversation

@FRidh
Copy link
Member

@FRidh FRidh commented Nov 21, 2019

This adds three lists with manylinux dependencies as well as three
packages that include all the manylinux dependencies.

Motivation for this change

Thanks to @thomasjm for their work in #55812.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @jonringer @adisbladis @thomasjm

@jonringer
Copy link
Contributor

@jonringer jonringer commented Nov 21, 2019

I think you need to also edit the interpreters so that they say they are "manylinux" compliant

  echo "manylinux1_compatible=True" >> $out/lib/${libPrefix}/_manylinux.py
@FRidh
Copy link
Member Author

@FRidh FRidh commented Nov 21, 2019

yes, that's a separate step. I'm still thinking about the interface.

@jonringer
Copy link
Contributor

@jonringer jonringer commented Nov 21, 2019

should we mark this as WIP then?

@FRidh
Copy link
Member Author

@FRidh FRidh commented Nov 21, 2019

No, I make that change separate as it would be a mass-rebuild. This is usable without anyway.

@jonringer
Copy link
Contributor

@jonringer jonringer commented Nov 21, 2019

Yea, that's fair

@@ -9181,6 +9181,8 @@ in
pypy27Packages = pypy27.pkgs;
pypy3Packages = pypy3.pkgs;

manylinuxPackages = callPackage ./../development/interpreters/python/manylinux { };

This comment has been minimized.

@jonringer

jonringer Nov 21, 2019
Contributor

This is kind of confusing as a top-level attribute (as it's python specific, but seems like it could be something broader), I think the manylinux* builders should live alongside buildPythonPackages under pythonX.pkgs

I'm also in favor of them being renamed from manylinuxXXXXPackage to buildManylinuxXXXXPackage

This comment has been minimized.

@FRidh

FRidh Nov 21, 2019
Author Member

This is kind of confusing as a top-level attribute (as it's python specific, but seems like it could be something broader)

These are not Python libraries, but just lists of derivations. They are used with Python packages but are not Python-version specific. Having some kind of indicator that they are related to Python is not be a bad idea.

I'm also in favor of them being renamed from manylinuxXXXXPackage to buildManylinuxXXXXPackage

They are not build functions either. The lists are just that, lists, and the other ones are derivations but are not meant to be used; you do not want to patchelf libs via the symlink tree because you would retain too many dependencies that way.

Ideally every package set contains only derivations, not functions or anything else. We're far from there but its important to consider.

This comment has been minimized.

@jonringer

jonringer Nov 21, 2019
Contributor

hmmm, I abandon my renaming stance, but still think it should be moved to python.manylinuxPackages, or python.pkgs.manylinuxPackages

This adds three lists with manylinux dependencies as well as three
packages that include all the manylinux dependencies.
@adisbladis adisbladis force-pushed the FRidh:manylinux branch from b13b644 to 6530535 Dec 5, 2019
@adisbladis
Copy link
Member

@adisbladis adisbladis commented Dec 5, 2019

@FRidh I have renamed manylinuxPackages to pythonManylinuxPackages (as suggested on IRC) and rebased.

@adisbladis adisbladis merged commit 65cc851 into NixOS:master Dec 5, 2019
1 check was pending
1 check was pending
grahamcofborg-eval Checking original out paths
Details
@FRidh FRidh deleted the FRidh:manylinux branch Dec 16, 2019
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Apr 29, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/mach-nix-create-python-environments-quick-and-easy/6858/17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.