-
Notifications
You must be signed in to change notification settings - Fork 9
Add --assert-config option #89
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
Conversation
|
I am testing this in https://copr.fedorainfracloud.org/coprs/churchyard/pyproject-buildrequires-no-tox-error/builds/ with an updated version of pyproject-rpm-macros. Also, I should add a test that tests if tox configuration in a different file works (as the current tests only test cases of tox.ini vs no config). |
Everything seems to work as expected.
Done. Rwady fro review. |
|
Cross-referenceing https://src.fedoraproject.org/rpms/pyproject-rpm-macros/pull-request/512 |
frenzymadness
left a comment
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.
I like its simplicity and it's well tested. Good job. The only one subjective issue I have with this is the help text which I find a little confusing.
What about this instead:
In tox 4, this option ensures that tox fails (raises an exception) if no configuration is found. By default, tox 4 does not terminate when no configuration exists.
In tox 3, this option has no effect, but it can still be specified without causing errors.
This option can be used alongside other options.
If it does not make sense to you, feel free to ignore it and merge it.
|
You mean this for the README or for argument |
|
I was also thinking about extending the error message a bit. |
Readme.
Good idea given that users packaging Python packages might not know tox much. |
|
See the two fixups. |
Motivation from Fedora: When we updated tox from version 3 to 4, it no longer fails when here is no suitable tox configuration found. This was a deliberate upstream choice. Unfortunately, it means that packages that use %pyproject_buildrequires with -t or -e now silently succeed if there is no tox configuration found. I identified 95 packages that are affected by this. Details: https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org/thread/ZSHSHZKVA4XJQBJD7FMMCALKN4UP5SAJ/
…x configuration exists Since tox 4, tox does not fail without configuration (tox.ini, or tox section in setup.cfg/pyproject.toml). As a result, packages that use %pyproject_buildrequires with -t or -e without having a tox confuration only generate additional BuildRequires on tox & tox-current-env itself. More dangerously, %tox without tox configuration does nothing (and succeeds). This behavior is dangerous and warrants an announced breakage. Packagers of ~100 affected Fedora packages were informed about the problem earlier in https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org/thread/ZSHSHZKVA4XJQBJD7FMMCALKN4UP5SAJ/ There will be a further announcement and warning about this change. EPEL 9 packages are not affected, EPEL 9 has tox 3 which fails without config by default. The change used a newly added option for tox-current-env: --assert-config. This was added in tox-current-env 0.0.16: fedora-python/tox-current-env#89
Motivation from Fedora:
When we updated tox from version 3 to 4, it no longer fails when here is no suitable tox configuration found. This was a deliberate upstream choice.
Unfortunately, it means that packages that use %pyproject_buildrequires with -t or -e now silently succeed if there is no tox configuration found.
I identified 95 packages that are affected by this.
Details: https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org/thread/ZSHSHZKVA4XJQBJD7FMMCALKN4UP5SAJ/