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

use python -m build to make wheels via sdists #13

Closed
graingert opened this issue Sep 24, 2021 · 6 comments · Fixed by #17
Closed

use python -m build to make wheels via sdists #13

graingert opened this issue Sep 24, 2021 · 6 comments · Fixed by #17

Comments

@graingert
Copy link
Contributor

graingert commented Sep 24, 2021

eg pypa/build#257

Following Trio's lead, I implemented package builds for towncrier such that first an sdist is built and then a wheel is built from that sdist. Following that, those two files are distributed throughout the CI tests and checks and (upcoming) automated publishing. The latter bit makes sure that the specific wheel to be published is what is being tested. The former bit makes sure that the sdist is able to generate a functional wheel. I don't mean this to test that build and wheel and so on work correctly, but rather to test that the package being built is setup properly such that it will result in a functional sdist. Twisted #10103 was shared as an example issue that this workflow would have caught.

@bennyrowland
Copy link

I am just trying to get started with tox, coming from a build process using python -m build to build my wheels followed by a manual install and test step, and this is something I would be really interested in having, so that the wheel (and sdist) that are created as part of tox are also the files that get uploaded to pypi afterwards. Is this something that anyone is actively working on, otherwise I would be interested in having a crack at it?
I guess the first step would be to create an alternative to wheel_pep517 like wheel_build, that calls build instead of pip or setup.py, that would allow it to be tested without impacting existing behaviour. Are there any other obvious design considerations that I am missing?

@mcarans
Copy link
Contributor

mcarans commented Dec 23, 2021

@bennyrowland I'm not sure what would be the value of having both a pip build and a python -m build. I guess you could just replace pip with python -m build in the following line:
https://github.com/ionelmc/tox-wheel/blob/master/src/tox_wheel/plugin.py#L151

What do you think @ionelmc ?

@mcarans
Copy link
Contributor

mcarans commented Dec 29, 2021

I have created an issue for pip to build via sdist: pypa/pip#10746

@ionelmc
Copy link
Owner

ionelmc commented Dec 29, 2021

I think the best way is to just have an option to chose how you'd like it to be built, perhaps allow these sort of usages:

wheel_pep517 = build # python -mbuild
wheel_pep517 = true # the current way, pip wheel --use-pep517

There could also be another new option but in that case we'd need to bring in some conflict validation, and then we'd have strange error/situations when options come from multiple sources.

I think we shouldn't replace pip wheel --use-pep517 with python -mbuild completely, well, at least not yet. I don't even know why I would use one over the other.

@bennyrowland
Copy link

@ionelmc I think the key difference is that build will generate an sdist and a wheel, thus if there is a problem with the sdist (because e.g. a source file is not being included in the MANIFEST) then the wheel will also fail, and the developer will notice, whereas pip will just make the wheel from the source directory, so doesn't simultaneously validate the sdist. If you want the sdist to upload, build is probably the best way of getting it in a PEP517 compatible manner.

I like your proposal of multiple options to wheel_pep517 and I will have a look at implementing that, I don't think there will be a lot of code to change. build will obviously become a dependency - should this be packaged as an "optional" dependency (i.e. pip install tox-wheel[build]), or a universal one whether people want it or not?

@ionelmc
Copy link
Owner

ionelmc commented Jan 17, 2022

build has no deps but it requires python 3.6+ ... I guess it could be as an optional extra for now.

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 a pull request may close this issue.

4 participants