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

[19.09] python: remove _manylinux.py #75815

Closed
wants to merge 1 commit into from

Conversation

adisbladis
Copy link
Member

@adisbladis adisbladis commented Dec 17, 2019

Backport of #75763.

This will turn manylinux support back on by default.

PIP will now do runtime checks against the compatible glibc version to
determine if the current interpreter is compatible with a given
manylinux specification. However it will not check if any of the
required libraries are present.

The motivation here is that we want to support building python packages
with wheels that require manylinux support. There is no real change for
users of source builds as they are still buildings packages from source.

The real noticeable(?) change is that impure usages (e.g. running pip install package) will install manylinux packages that previously
refused to install.
Previously we did claim that we were not compatible with manylinux and
thus they wouldn't be installed at all.

Now impure users will have basically the same situation as before: If
you require some wheel only package it didn't work before and will not
properly work now. Now the program will fail during runtime vs during
installation time.

I think it is a reasonable trade-off since it allows us to install
manylinux packages with nix expressions and enables tools like
poetry2nix.

This should be a net win for users as it allows wheels, that we
previously couldn't really support, to be used.

(cherry picked from commit e9f522e)

Motivation for this change
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 @andir

This will turn manylinux support back on by default.

PIP will now do runtime checks against the compatible glibc version to
determine if the current interpreter is compatible with a given
manylinux specification. However it will not check if any of the
required libraries are present.

The motivation here is that we want to support building python packages
with wheels that require manylinux support. There is no real change for
users of source builds as they are still buildings packages from source.

The real noticeable(?) change is that impure usages (e.g. running `pip
install package`) will install manylinux packages that previously
refused to install.
Previously we did claim that we were not compatible with manylinux and
thus they wouldn't be installed at all.

Now impure users will have basically the same situation as before: If
you require some wheel only package it didn't work before and will not
properly work now. Now the program will fail during runtime vs during
installation time.

I think it is a reasonable trade-off since it allows us to install
manylinux packages with nix expressions and enables tools like
poetry2nix.

This should be a net win for users as it allows wheels, that we
previously couldn't really support, to be used.

(cherry picked from commit e9f522e)
@FRidh
Copy link
Member

FRidh commented Dec 17, 2019

I am against backporting this as it changes behaviour on a stable release, which is against the concept of a stable release.

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

Successfully merging this pull request may close these issues.

None yet

3 participants