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

[manylinux2014] arm64 patchelf alignment fix #739

Merged

Conversation

geoffreyblake
Copy link
Contributor

Bug reports for Manylinux2014_aarch64 wheels show that they contain bad alignment for Arm distros that use 64k pagesize. This updates patchelf 0.11 to include recent patches that force Arm64 page alignment to 64k regardless of the base system used for building.

Tested locally with the cffi project, resulting wheel and binary after performing audit wheel uses 64k alignment. Tested wheel on Centos8 on Arm64 and imported cffi with no issue.

Solves Issue: #735

@geoffreyblake
Copy link
Contributor Author

Huh, Travis is failing on a curl call:
curl: (7) Failed to connect to 2a04:4e42:50::223: Cannot assign requested address

Seems to be happening in some of the other PRs here too. Additionally the PPC builds are being detected as stalled, and that seems to be happening for the past week or so:

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received
The build has been terminated

@geoffreyblake
Copy link
Contributor Author

@reaperhulk , @mattip , could I get a review on this?

@mattip
Copy link
Contributor

mattip commented Aug 21, 2020

It looks good to me, but is a bit hard to verify with a failing arm64 build. I also see problems with ppc64le on another repo.

@geoffreyblake
Copy link
Contributor Author

@mattip, can we kick the build again? That curl error is related to ephemeral port starvation, so may be intermittent, not sure if this is a more widespread travis problem or just got unlucky.

@geoffreyblake
Copy link
Contributor Author

@mattip, just force pushing restarted travis, looks like ppcle is having odd timeout issues, no clue as to why, but Arm64, x86_64, and s390x are passing

@reaperhulk
Copy link
Contributor

That's quite a patch -- I take it the change we want was squashed up in all those other changes? That's unfortunate...

@geoffreyblake
Copy link
Contributor Author

@reaperhulk, I'm happy to iterate on it, I can try cherry-picking just the alignment fix to see if it works too. Though my experience is not great with that going cleanly.

@reaperhulk
Copy link
Contributor

It'd be good to try I think. If it proves unworkable we can always change our minds.

@geoffreyblake
Copy link
Contributor Author

@reaperhulk , @mattip , cherry-picking worked with just a minor merge issue. The resulting binary appears to be generating the proper alignments. Re-building the container to do a full test on a wheel.

@geoffreyblake
Copy link
Contributor Author

Internal testing on the pyca/cryptography shows it is generating binaries with the proper alignments on Arm. Still seeing ppcle failing with timeouts, though all other builds are working.

@mattip
Copy link
Contributor

mattip commented Aug 24, 2020

The goal of this PR is to finx the ARM64 images, so maybe it can just go in as-is, without the ppc64le builds. They have been failing for the past week.

@reaperhulk
Copy link
Contributor

Much easier to review! Could you add some text to make it clear that this patch can be backed out when patchelf is updated? Just in case someone else ends up doing that work. Otherwise this LGTM (although I am not a manylinux contributor so take my opinion with whatever sized grain of salt you desire)

…4 machines with 64k pages.

- Based on NixOS/patchelf@0470d69

Can be removed once a new version of patchelf is released containing the above commit.
@geoffreyblake
Copy link
Contributor Author

Ok, comments added. I posted a question to the TravisCI mailing list about this PPC issue, seems odd it happens during a yum install. This fix is primarily applicable to Arm64, which has multiple page sizes in use in the wild.

@geoffreyblake
Copy link
Contributor Author

@mattip, anything else needed for this? I do not believe that the PPCLE build is necessary for a fix related to Arm64.

@mattip
Copy link
Contributor

mattip commented Aug 25, 2020

@geoffreyblake you need to ping someone who can merge.

@reaperhulk
Copy link
Contributor

@auvipy might be a good candidate here 😄

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.

4 participants