ionelmc / tox-wheel

A Tox plugin that builds and installs wheels instead of sdist. Note that this plugin is obsolte as tox 4.0 already has wheel support.
BSD 2-Clause "Simplified" License
23 stars 10 forks source link

Support building via PyPA/build #17

Closed bennyrowland closed 2 years ago

bennyrowland commented 2 years ago

This commit adds support for using the PyPA build backend (https://pypa-build.readthedocs.io/en/latest/index.html) to build the project wheels in an isolated virtualenv as prescribed by PEP517. This is similar to the existing PEP517 support except that build will first create an sdist and then produce the wheel from that, thus validating the sdist as well as the wheel.

This behaviour is controlled by the wheel_pep517 tox testenv attribute - this was a bool but is now a string. An empty string or missing attribute gives the legacy behaviour, the string "build" gives the new behaviour, and any other value gives the previous pip based PEP517 build.

In essence the change is in the command run to build the wheel, switching from "pip wheel --use-pep517" to "python -Im build". Because there is an extra build artefact (the sdist), we filter the distdir for wheels only before returning the first one. Because build is not part of the standard library, it must be installed in the tox virtualenv to be used - this is accomplished by implementing the tox_testenv_install_deps() hook, if the wheel_pep517 parameter is set to "build" then the build package will be installed. Currently it is necessary to install it with the [virtualenv] option specified, build normally uses venv to create its isolated environment but that doesn't work inside tox because venvs currently cannot be created inside virtualenvs. As venv becomes more accepted (and possibly the internal backend for virtualenv) this requirement may go away.

In terms of testing, I have simply added two tests (and a fixture) to match the existing PEP517 behaviour testing.

build has the same requirements of Python 3.6 or above that tox-wheel has so I have not implemented any kind of checks based on different versions. Locally it is passing tests on Python 3.8 and 3.9.

@ionelmc I haven't yet put anything in README.rst or CHANGELOG.rst about this new change, let me know if you are happy with the actual code changes and I can draft something for the documentation. Or feel free to add that part yourself.

Closes: #13

mcarans commented 2 years ago

Excellent! I'm looking forward to this feature.

mcarans commented 2 years ago

@ionelmc Does this need the changes in PR 14?

ionelmc commented 2 years ago

@bennyrowland can you do a rebase, just to kick the CI again

mcarans commented 2 years ago

I had raised an issue with pip to build via sdist but the latest comment on it is "i think its fair to say that this particular one is out of scope for pip, after all pip is a installer, not a builder, and python -m build already exists plus tools like cibuildwheel"

Therefore, it would be good to get this PR into tox-wheel if possible.

@bennyrowland @ionelmc what needs to be done to progress this PR?

bennyrowland commented 2 years ago

Sorry, I have been working on other things for a while and pushing this forward kind of ended up on the back burner. I am currently on holiday until early June, but I will try and get it sorted soon after I get back. I think it just needs a rebase on to the main branch so shouldn’t take too long.

On 23 May 2022, at 00:29, Mike @.***> wrote:

 I had raised an issue with pip to build via sdist but the latest comment on it is "i think its fair to say that this particular one is out of scope for pip, after all pip is a installer, not a builder, and python -m build already exists plus tools like cibuildwheel"

Therefore, it would be good to get this PR into tox-wheel if possible.

@bennyrowland @ionelmc what needs to be done to progress this PR?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

bennyrowland commented 2 years ago

Hi everyone, sorry it took longer than expected to get this done, but I have now rebased this on top of the latest changes. There was an issue that seems to be related to changes in build or setuptools where my builds of test packages were failing because it didn't like the package structure, but I have fixed that now so all tests are passing once again.

mcarans commented 2 years ago

@ionelmc Would you be able to approve the workflow?

bennyrowland commented 2 years ago

@ionelmc just a reminder that this PR needs approval to run the tests etc.

ionelmc commented 2 years ago

Doh. How can I enable this by default? I always forget this.

bennyrowland commented 2 years ago

@ionelmc - I don't know what is going on with the check and docs tests, but they seem to fail inevitably, nothing related to my changes - all other tests seem to pass except a few have timed out. Is there anything else you need from me to get this PR accepted?

ionelmc commented 1 year ago

Released 1.0.0.