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

Fix binary wheel build step #39

Merged
merged 13 commits into from
Aug 9, 2020
Merged

Fix binary wheel build step #39

merged 13 commits into from
Aug 9, 2020

Conversation

moralrecordings
Copy link

PyPI expects Linux binaries to be built to the manylinux specification. The build toolchain for V8 uses a new-ish glibc, so this process produces manylinux2014-compatible packages.

I've tested the cibuildwheel build process, but the changes to the Travis build script are speculative based on the manual process I used. With luck opening a PR will fire off a proper test.

@tbodt
Copy link
Owner

tbodt commented Jul 31, 2020

Please mark this as ready for review when it's done

@scottp-dpaw
Copy link

Notes:

  • I gave up on cibuildwheel, as it was a bit flaky, and there isn't an official Docker image for manylinux2014 with Python 2.7 support. The v8 toolchain is manylinux2014 symbol compatible, so we build everything with that and then confirm the result with auditwheel.
  • For some reason, the build scripts for the v8 unit testing framework generate Clang commands that exclude the compiler's default paths for includes etc, leading to the build failing. Given that the tests are unused by v8py, and there is no obvious way to exclude it, I made a change to setup.py to strip out the test targets from v8.
  • greenstack isn't compatible with Python 3.7 or 3.8, so the Travis job checks the version before attempting to install.

@moralrecordings moralrecordings marked this pull request as ready for review July 31, 2020 06:00
@tbodt
Copy link
Owner

tbodt commented Aug 1, 2020

Thanks for the PR. It'll be good to finally have wheels, 3 years later. There are a few areas of future work I've noted, otherwise looks good to me (as long as it actually works).

If you don't mind me asking, what are you using v8py for?

setup.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
@tbodt tbodt merged commit 4fe3eea into tbodt:master Aug 9, 2020
@scottp-dpaw scottp-dpaw mentioned this pull request Aug 10, 2020
@armudgal
Copy link

This is great. Thanks for the PR

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