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

python3Packages.sip: ./fix-manylinux-version.patch powerpc64le->ppc64le #168053

Closed
wants to merge 1 commit into from
Closed

python3Packages.sip: ./fix-manylinux-version.patch powerpc64le->ppc64le #168053

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 9, 2022

Description of changes

The upgrade of python3Packages.sip from 6.1.1 -> 6.5.0 in 2c9237 broke
many dependent packages (notably, PyQt5) on most non-x86_64 platforms.
This was reported in #163589 and fixed in #165992 at 0bcbce26.

However the fix is not quite complete; Python seems to have several
competing naming schemes for platforms (none of which aligns with the
standard gcc/autoconf platform tuples). The py_platform identifier
for 64-bit PowerPC machines is "powerpc64le", but the wheel tag for
these machines uses the identifier ppc64le. Since the code in
sipbuild/project.py branches based on one but updates the other, we
need a rosetta stone here. Let's update ./fix-manylinux-version.patch
from 0bcbce26 to add it.

Tested on powerpc64le-linux Raptor Computing Systems Talos II.

CC: @tpwrules @FRidh @r-burns @CrystalGamma

Things done

Marked as draft while I verify with a rebuild.

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
    • powerpc64le-linux (building...)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Fits CONTRIBUTING.md.

The upgrade of python3Packages.sip from 6.1.1 -> 6.5.0 in 2c9237 broke
many dependent packages (notably, PyQt5) on most non-x86_64 platforms.
This was reported in #163589 and fixed in #165992 at 0bcbce26.

However the fix is not quite complete; Python seems to have several
competing naming schemes for platforms (none of which aligns with the
standard gcc/autoconf platform tuples).  The `py_platform` identifier
for 64-bit PowerPC machines is "powerpc64le", but the `wheel` tag for
these machines uses the identifier `ppc64le`.  Since the code in
`sipbuild/project.py` branches based on one but updates the other, we
need a rosetta stone here.  Let's update ./fix-manylinux-version.patch
from 0bcbce26 to add it.

Tested on powerpc64le-linux Raptor Computing Systems Talos II.
@tpwrules
Copy link
Contributor

tpwrules commented Apr 9, 2022

For the record, I grabbed the list of tags and supported versions from PEP 600, which does not mention powerpc64le at all. I found an issue which suggests that ppc64le vs. powerpc64le depends on what the distro decides. I'm not sure if the community has come to a consensus on if powerpc64le is a valid tag or not for the Python wheel specifically. A comment there suggests that the valid wheels should in fact be called powerpc64le and ppc64le be deprecated.

In any case, does a replacement for ppc64 -> powerpc64 also need to be created?

@ghost
Copy link
Author

ghost commented Apr 9, 2022

Yeah, this situation is a total mess. I'm digging into it further.

It looks like the problem is actually further upstream, the s_powerpc64le_ppc64le_ needs to happen in pythonHostPlatform from pkgs/development/interpreters/python/cpython/default.nix, because _PYTHON_HOST_PLATFORM=linux-ppc64le is the only way to get pip debug --verbose to admit support for anything manylinux-ish.

I've got a rebuild testing that right now, but it'll take a few hours.

Why, oh why can't everybody just use the binutils/gcc/autoconf tuples instead of cooking up their own taxonomy...

Someday I am going to submit a PR for lib/systems/rosetta-stone.nix to centralize all of these 5-tuple-to-weird-taxonomy mappings in one place.

@ghost
Copy link
Author

ghost commented Apr 9, 2022

For the record, I grabbed the list of tags and supported versions from PEP 600, which does not mention powerpc64le at all.

Yes, I believe that was the correct thing to do.

I found an issue which suggests that ppc64le vs. powerpc64le depends on what the distro decides.

My reading is that the PEP600 standard adopted the architecture names that Fedora was using in 2014.

Since 2014, Fedora may or may not have switched to the binutils/gcc/autoconf name powerpc64el (which is stable and does not change over time). I don't think this is relevant unless PEP600 is updated or superseded. The fact that PEP600 copied Fedora at the time it was written shouldn't imply that whenever Fedora changes their policy it retroactively changes the PEP600 standard, should it?

@tpwrules
Copy link
Contributor

tpwrules commented Apr 9, 2022

I found an issue which suggests that ppc64le vs. powerpc64le depends on what the distro decides.

My reading is that the PEP600 standard adopted the architecture names that Fedora was using in 2014.

Since 2014, Fedora may or may not have switched to the binutils/gcc/autoconf name powerpc64el (which is stable and does not change over time). I don't think this is relevant unless PEP600 is updated or superseded. The fact that PEP600 copied Fedora at the time it was written shouldn't imply that whenever Fedora changes their policy it retroactively changes the PEP600 standard, should it?

The thing is, that comment seems to be a Fedora maintainer suggesting that they regret the choice of ppc64le and will change their Python to use powerpc64le, with the steps outlined at the end. I am not sure if that ever took place. I agree that it seems weird to change the interpretation of PEP600, but that seems to be what they want to do. I can't find any indication that it was ever revised.

To be clear, what exactly is the problem you see on your system? Theoretically, if your Python reports itself as powerpc64le, then sip with that patch should create a plain manylinux wheel with that tag instead of trying to create a manylinux2014 wheel, which pip should be happy to install and Python itself should be happy to find the .so for. But you said in some way your Python still calls itself ppc64le?

I wonder if it would best to simply patch sip's manylinux option to be False by default, or in all cases, as presumably people are not using NixOS's sip to build wheels for other Pythons and it seems they are fraught with uncertainty. In approximately 100% of cases, NixOS users will build wheels for the exact same Python and ambient libraries they will install them in. Is it even valid to call a NixOS-built wheel manylinux, as I think it would link against libraries that are too new, and in fact won't be available on any other distro at all (by path at least)?

@ghost
Copy link
Author

ghost commented Apr 10, 2022

Superseded by #168083.

@tpwrules, sorry for bothering you; the root cause here had nothing to do with #165992 even though the problem manifest itself with a nearly identical error message.

@ghost ghost closed this Apr 10, 2022
@ghost ghost deleted the fix-fix-manylinux branch April 10, 2022 02:39
This pull request was closed.
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.

1 participant