Conversation
|
Any feedback on the documentation welcome! |
|
Thanks very much for writing this @joerick! I read the documentation you added and it all looks good to me on a high level. Would you like that I close my PR? I admit that I have been stretched thin for time to take a look at it again after the review comments, and was planning to get to it at some point in April – so if it is the case that you would like to take over, please let me know! |
|
Thanks @agriyakhetarpal. Yes let's close out #2745 and continue work here. Happy for you to contribute too if you want as well! I've added a todo list to the PR description. |
|
Awesome, thank you! Please let me know when you are done with your changes, and I'd be happy to create some PRs against this branch – so as to save ourselves from the trouble of mutual merge conflicts :D I had a fun time writing the tests out in #2745, so I'm happy to continue in that direction. |
|
I really like the fact that running both twine check on all wheels but abi3audit only on abi3 wheels is already being thought of ! |
|
Alright – I think I have made all the changes I wanted to make here, but will self-review a little later once others have a chance to look at them. I'm all done, so I'll unassign myself now :D I've retriggered the CI for the Azure Windows timeout, and all other CI tests look green. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 33 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mhsmith
left a comment
mhsmith
left a comment
There was a problem hiding this comment.
Looks fine as far as Android is concerned. I haven't used abi3audit myself, but if it works for Linux then in principle it should work for Android too.
On win / python 3.13, virtualenv creates a venv where the 'home' points back to the venv that sys.executable was running in, rather than the root install. that seemingly leads to problems with package resolution, where pip.exe couldn't find the pip python package. this appears to fix it!
|
Looks like the CI passed! I guess this is a Windows + CPython bug, in that case? Should we raise it elsewhere? |
|
It might be worth raising with virtualenv, because uv didn't have the bug. But before I do that I'd wanna check it's not a bug with how we activate venvs. The workaround works for now anyway :) |
|
This is ready for a wider review now I think. @woodruffw might also be interested! |
|
@ngoldbaum you might be interested in this for NumPy too :) |
|
@agriyakhetarpal numpy doesn't publish abi3 wheels - how would this impact NumPy's release automation? |
@ngoldbaum, ah, sorry, not NumPy – PyTables, rather. You asked about it in the PyPA Discord but I incorrectly assumed that was NumPy. I just realised when I went to check there again. |
Following on from #2745.
I've only sketched out the functionality in documentation form so far - implementation to come.The idea is that audit commands can specify
{abi3wheel}or{wheel}placeholders, which determine whether they're run for a particular wheel.I think the best approach here would be to run the audit on each wheel as it's built, similar to @agriyakhetarpal's approach. As I've been thinking about it, it turns out there's nothing stopping that as far as I'm aware and it prevents the frustration of leaving the failure to the end of the build.
Todos
audit-requiresinheritto work outside of overrides, making it easier for audit configs to extend the default