-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix #6163: Default to setuptools.build_meta:__legacy__ #6212
Fix #6163: Default to setuptools.build_meta:__legacy__ #6212
Conversation
Note that the CI is expected to fail, as successfully testing this currently requires a manual build step as described in #6210 (comment) However, I wanted to give folks reassurance that the horrible hack in #6210 really is temporary, and just a way to take a bit of the urgency off the setuptools folks, since this approach involves a public API change for them, and hence shouldn't be rushed. |
Note that this approach poses a problem for projects with a non-wildcard
|
The above error was just an artifact of my failing to pass in the local directory with the pre-built wheel file in it. However, solving that still wasn't enough to get a successful build:
|
Ah, that assertion error looks like an unrelated bad interaction with the Testing with cache-dir instead gives a successful install as we'd expect:
|
Turns out there's already an issue filed for the AssertionError I ran into above: #6197 |
Closing in favour of #6229 as a near-term resolution of #6163 (see #6163 (comment) for more details) We'll presumably revisit this in a future release, but that can be a new issue and new PR (with the tests rolled in to #6229, there really won't be a lot of changes needed to update to a new setuptools backend later) |
Just noting that once switching to the updated PEP 517 implementation, the |
It's looking like this is going to be the actual resolution for #6163 in However, it will remain WIP until |
The new version has been released as 40.8.0 🎉
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes all look good to me @ncoghlan!
If it isn't too much pain, could you rewrite the commit history of this PR? If it is more work than you'd like, I'll be happy to do so myself.
e9043b2
to
500c87a
Compare
Squashed everything into one commit and removed the out of date comment. Hopefully CI will agree this is now good to go :) |
Missed a couple of test cases that referenced the exact implicit setuptools version. I also have a couple of markup and docstring fixes to add. |
The main setuptools PEP 517 backend is intended for explicit usage in `pyproject.toml`, when the project authors can ensure that their `setup.py` runs without that directory being implicitly on `sys.path`. For implicit usage, setuptools now offers a separate legacy backend that more closely mimics direct execution of the `setup.py` script.
500c87a
to
682cff7
Compare
@pradyunsg @pfmoore Latest push looks good to go to me - if you spot anything that needs tweaking before merging, please don't feel obliged to wait for me to comment before fixing it (I'm about to head to bed and won't be back online until tomorrow evening my time). |
Great! Thanks @ncoghlan! :) |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
The main setuptools PEP 517 backend is intended for
explicit usage in
pyproject.toml
, when the projectauthors can ensure that their
setup.py
runs withoutthat directory being implicitly on
sys.path
.For implicit usage, setuptools now offers a separate
legacy backend that more closely mimics direct
execution of the
setup.py
script.