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

Publish sdist and bdist wheel #359

Closed
groodt opened this issue Dec 21, 2019 · 14 comments
Closed

Publish sdist and bdist wheel #359

groodt opened this issue Dec 21, 2019 · 14 comments

Comments

@groodt
Copy link

groodt commented Dec 21, 2019

The benefits of wheels are well documented. See: https://pythonwheels.com/
This package is pure Python and publishing it as both source and as a wheel is simple.

Would you accept a contribution to add a Makefile to this repo that would allow you to build both source distribution (sdist) and built distribution (wheel)?

@eliben
Copy link
Owner

eliben commented Dec 21, 2019

I've had serious issues with publishing wheels in the past (eg. see #288 and other linked issues).

What concrete problem is this lack of wheels creating for users of pycparser today?

@groodt
Copy link
Author

groodt commented Dec 21, 2019

I've had serious issues with publishing wheels in the past (eg. see #288 and other linked issues).

I'm sorry you had negative experiences in the past. "Once bitten, twice shy" is entirely understandable. The root cause of that issue was that the sdist and the wheel appear to have not been released at the same time or the same commit, not a problem with the wheel format itself. This is perhaps solvable with a repeatable release process or we can create a Makefile to make sure everything is clean before upload. It normally is python setup.py sdist bdist_wheel upload and you're done, but I don't disagree that you had really bad luck with things the last time you tried. I'm sure with contributors such as @graingert, myself and yourself, we can make it something that you are comfortable with.

What concrete problem is this lack of wheels creating for users of pycparser today?

Faster, more repeatable and more easily cacheable installs. It is the future of Python package distribution and the vast majority of the popular packages are all publishing wheels. pycparser is certainly very prominent on Python Wheels You'll probably get more people coming along to ask about wheels next time if it wasn't me. :) More seriously though, if you haven't already, the PEP-427 is a light and clear read that may convince you that it's better.

@eliben
Copy link
Owner

eliben commented Dec 22, 2019

I did read that PEP in the past, and it's not very convincing for pycparser. pycparser is pure Python, so installing it is quick and easy with or without wheels. It doesn't require anything except Python itself on the target machine. Wheels just don't provide a killer feature for pycparser that would motivate spending work on it, to be honest.

Can you show me a concrete proposal as to what would change? Like a PR? Is this only about adding bdist_wheel when calling setup.py? I can try to do that when the next version of pycparser is released

@eliben
Copy link
Owner

eliben commented Dec 22, 2019

@graingert you mean the extra line in setup.cfg? It's still there, that PR (#100) wasn't rolled back.

My question is - what else do I have to do -- just publish the wheel? Meaning, the next time I release a new pycparser version, I should use:

python setup.py sdist bdist_wheel upload

Instead of:

python setup.py sdist upload

Correct?

If yes, I can try to do this again when I'm releasing the next version (2.20). Then presumably both the source distribution and the wheel will be pushed from the same commit tag.

@graingert
Copy link
Contributor

You can also checkout any previous tag (that doesn't currently or previously have a whl) and run checkout origin/master -- setup.cfg && python3 setup.py bdist_wheel upload

@groodt
Copy link
Author

groodt commented Dec 23, 2019

python setup.py sdist bdist_wheel upload
Correct?

Yes, this is really all that is necessary.

If yes, I can try to do this again when I'm releasing the next version (2.20). Then presumably both the source distribution and the wheel will be pushed from the same commit tag.

This would be fantastic, and really all that I can ask for. Thank you!

@graingert I'm not sure we need to ask @eliben to retrospectively fix things. Moving forward is good enough I think and it is probably the lowest risk and least work for all concerned.

@eliben It sounds like you know exactly how to upload a new release of both sdist and bdist_wheel. If you'd like some repeatability, perhaps @graingert and I can contribute a Makefile perhaps using twine that can help you to build and upload distributions. Of course, you may not need or want this if you're happy doing things manually, since it really is a single command.

@mattip
Copy link

mattip commented Jan 7, 2020

xref numpy/numpy#15108 for an example where shipping a wheel would save end users problems.

@eliben
Copy link
Owner

eliben commented Jan 7, 2020

@mattip I'm curious to understand how that would help.
That issue seems to be related to pycparser's requirement of running in an unoptimized python build (no -O) because PLY requires docstrings to be present. Are you saying that if we had the wheel then downstream libraries wouldn't have to worry about which build mode they need for pycparser? If a client depends on pycparser it still wouldn't work with -O, regardless of how pycparser was distributed. Am I missing something here?

@graingert
Copy link
Contributor

@mattip I'm curious to understand how that would help.
That issue seems to be related to pycparser's requirement of running in an unoptimized python build (no -O) because PLY requires docstrings to be present. Are you saying that if we had the wheel then downstream libraries wouldn't have to worry about which build mode they need for pycparser? If a client depends on pycparser it still wouldn't work with -O, regardless of how pycparser was distributed. Am I missing something here?

I'm pretty sure a wheel won't fix their bug, but it will make their build process slightly faster

@mattip
Copy link

mattip commented Jan 7, 2020

There are two stages: installing and then using.

You are correct that we cannot use pycparser when PYTHONOPTIMIZE=2, so we skip the tests that actually invoke it when running the test suite.

But this issue is about installing it as part of one out of many CI test runs. We don't want to special-case to avoid installing pycparser in that specific run (and in fact are pulling in pycparser as a dependency of another package), so it would be nice if we could just install it and not use it. Installing from a wheel is just unzipping the wheel and succeeds with PYTHONOPTIMIZE=2, where installing from a source tarball does all kinds of things and breaks with PYTHONOPTIMIZE=2.

@zacbir
Copy link

zacbir commented Feb 21, 2020

Bit me in the bottom today trying to build multi-platform pex binaries. Specifying even one --platform argument makes pex look for wheels for the application's dependencies, even when a pure python package exists. So, I need to make and vend my own wheel for pycparser. ($0.02)

@eliben
Copy link
Owner

eliben commented Mar 3, 2020

I've opened #365 to track releasing version 2.20 in the next few days. I will try to push the wheel for this version. Please subscribe to that issue to see updates - I might ask for help if the wheels cause issues for pip installs.

@groodt
Copy link
Author

groodt commented Mar 3, 2020

Thanks @eliben

I will be standing by to assist and hopefully some of the others in this issue will be too.

@eliben
Copy link
Owner

eliben commented Mar 4, 2020

2.20 pushed with a wheel, please see update on #365 and comment there if you see issues. I'm closing this one.

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

No branches or pull requests

5 participants