Simplify _should_build #13232
Simplify _should_build #13232
Conversation
The only call sites are in pip install in pip wheel, after resolution, so there are never constraint requirements there.
In pip wheel, we always want wheels, of course. should_build_for_wheel_command was only called when is_wheel is False, so the logging statement part of _should_build was never reached. Since this is the only side effect and _should_build otherwise always return true in these circumstances, it can be removed.
The need_wheel argument is now always False
sbidoul
added
skip news
It is a remnant of the `setup.py install` era, and we can safely assume it is always set.
|
I think |
There was a problem hiding this comment.
Skipping %s, due to already being wheel seems to be a relic of the past. Searching Google for exact matches (after the comma) since January 2020 only nets three results, which implies it's only from ancient versions of pip. This was the only change I was worried about since it looked like a potential user-facing logging change.
Constraint requirements are only persisted after resolution with the legacy resolver, and even then, they are filtered out by RequirementSet.
Thanks for simplifying this!
|
Also thank you for making this a breeze to review with the well explained commits! 🍃 |
Ah, I had forgotten to check it. Thanks for spotting it. I'd be surprised if the legacy resolver is not riddled with issues at this point, since we don't test it. |
This is a suite of simple commits to simplify
_should_build, as the circumstances in which it is invoked have evolved over time.Each individual commit explains the reasoning.
towards #11457