Support building via PyPA/build #17
Conversation
|
Excellent! I'm looking forward to this feature. |
|
@ionelmc Does this need the changes in PR 14? |
|
@bennyrowland can you do a rebase, just to kick the CI again |
|
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? |
|
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.
|
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.
Changes in setuptools automatic package discovery meant that building with the build tool was failing because it didn't like the package structure in the generated folders. Adding an empty list as the packages argument to `setup()` solves this problem.
bennyrowland
force-pushed
the
support_build
branch
from
05b0aea
to
6da45c5
Compare
|
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. |
|
@ionelmc Would you be able to approve the workflow? |
|
@ionelmc just a reminder that this PR needs approval to run the tests etc. |
|
Doh. How can I enable this by default? I always forget this. |
|
@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? |
src/tox_wheel/plugin.py
Outdated
| @hookimpl | ||
| def tox_testenv_install_deps(venv, action): | ||
| if venv.envconfig.wheel_pep517 == "build": | ||
| venv.run_install_command(["build[virtualenv]==0.7.0"], action) |
There was a problem hiding this comment.
This shouldn't be a pin as it can downgrade the package. Use a >=?
There was a problem hiding this comment.
I am happy with this change (and have pushed a new version with it made). I think my logic for pinning it was because this is only installed into the specific venv and it is hard to imagine a scenario where someone would want to have a version of build installed in their venv other than to build the package, and we are already doing that for them here. Thus pinning it meant that there was no chance of an updated build changing the behaviour of tox-wheel. However, removing the pin is probably fine, if users decide they need an older version of build they can install it into the venv themselves and this code will now allow that. It is also quite unlikely that build is going to suddenly change its (very minimal) API.
There was a problem hiding this comment.
You never know when someone needs a newer build version. Exact version pinning is a bad practice, you should avoid it in general for specifying library dependencies. More on that: https://caremad.io/posts/2013/07/setup-vs-requirement/
There was a problem hiding this comment.
Interesting article - I have made the change as requested.
|
Released 1.0.0. |
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