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

Build and upload wheels to PyPI #439

Merged
merged 1 commit into from
Oct 3, 2019
Merged

Build and upload wheels to PyPI #439

merged 1 commit into from
Oct 3, 2019

Conversation

jcugat
Copy link
Member

@jcugat jcugat commented Oct 3, 2019

Closes #432

For now both sdist and wheel are built and uploaded, but as @graingert commented #432 (comment) I can drop the sdist if you want.

Tested locally and the wheel is built correctly, the only step I could not test is the upload with twine to PyPI.

@sethmlarson
Copy link
Contributor

My 2 cents is we should be shipping universal wheels only as a testament to ensuring no C extensions. :)

@jcugat
Copy link
Member Author

jcugat commented Oct 3, 2019

Happy to change it. Initially I left both types because I thought it would be a problem for old pip versions, but I've looked and wheel support was added in pip v1.4 released on 2013, so unless somebody is using an ancient version it should be fine.

I'm not sure if there are any other side-effects, but contrary to what @graingert mentioned in the original issue, flit actually ships both the wheel and the sdist: https://pypi.org/project/flit/#files

@sethmlarson
Copy link
Contributor

Oh then let's do what flit does and ship both. I think I'm good to review this then! :D

Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting this! 🎉

@sethmlarson sethmlarson merged commit 9e1cc26 into encode:master Oct 3, 2019
@jcugat jcugat deleted the build_wheels branch October 3, 2019 21:35
@graingert
Copy link
Member

https://github.com/takluyver/flit/blame/fbd7a0d20daffad158d3a80bd8fae183f20114f7/doc/cmdline.rst#L59

Yeah ok it looks like it does ship them both. I wonder why

@sethmlarson
Copy link
Contributor

Probably because pip always favors wheels over sdist so it doesn't hurt if you're pushing a universal wheel? Then you're guaranteed compatibility if you do end up getting installed by an ancient pip version.

@@ -17,6 +17,7 @@ pytest-cov
trio
trustme
uvicorn
wheel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a requirement for this, right?

(We could choose to include a pip install twine wheel in the ./scripts/publish script tho)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, for some reason I thought twine was already in test-requirements.txt. I'll open a PR to remove it and will check inside ./scripts/publish if wheel is installed or not (similar to the other required tools).

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.

Releases to PyPI should also contain wheels
4 participants