cmdoptions: Check if format_control exists before accessing it #9459
Conversation
|
@NoahGorny thanks! Could you add a test that surfaces the issue ? This will help make sure this does not crop up again when this part will be refactored. |
@sbidoul Done 😄 |
NoahGorny
force-pushed
the
fix-build-wheel-options
branch
from
fcf0e35
to
e040e0f
Compare
news/9444.bugfix.rst
Outdated
| Check if format_control exists before accessing it. | ||
| Fixes a case when per-setup.py options are specified in a requirement | ||
| file, and we are trying to invoke uninstall. |
There was a problem hiding this comment.
| Check if format_control exists before accessing it. | |
| Fixes a case when per-setup.py options are specified in a requirement | |
| file, and we are trying to invoke uninstall. | |
| Permit ``pip uninstall -r`` to accept requirements files containing ``--install-option`` or ``--global-option``. |
|
Oh, please do rebase this PR on the current master branch, since there have been quite a few linter-related changes and I'd appreciate to get a confirmation that this won't break CI once merged. :) |
NoahGorny
force-pushed
the
fix-build-wheel-options
branch
from
75ae612
to
9945b2d
Compare
hey @pradyunsg , I did what you requested, let me know what you think 😄 |
| if any(map(getname, names)): | ||
| control = options.format_control | ||
|
|
||
| control = getattr(options, "format_control", None) |
There was a problem hiding this comment.
Why is format_control not present on the options object? Is there a way we can make it always available instead?
There was a problem hiding this comment.
I am unsure, as this approach is the one being used with the build/global/install_options
Fixes a case when per-setup.py options are specified in a requirement file, and we are trying to invoke uninstall
NoahGorny
force-pushed
the
fix-build-wheel-options
branch
from
9945b2d
to
43ee207
Compare
github-actions
bot
added
the
needs rebase or merge
|
Going to go ahead and close this, since it has merge conflicts that haven't been resolved in a while. Please feel welcome to file a new PR or to reopen and update this one! FWIW, |
Fixes a case when per-setup.py options are specified in a requirement
file, and we are trying to invoke uninstall
Closes #9444