-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Fail if an invalid -X option is provided #89608
Comments
We found that using -X ---- can be very confusing and annoying when you make typos, so I think having errors if you pass some unknown stuff will be outweighing any downside of making this more strict. |
I'm opening this again as, this is still not finished as we want to still extend the initialization API to allow a non static string to improve the error message. In general, the protocol is to ask if there is something else to be done before closing (and giving it some time before closing if nobody answers) :) |
Your documentation has stated for YEARS that arbitrary values can be passed via PyConfig.xoptions or -X. 9583cac Quoting your own documentation: There is client software out there relying on this. kovidgoyal/kitty#5223 Please do not randomnly break backwards compatibility, without even a deprecation period, because something "annoys" you. |
@kovidgoyal Thanks for your message. Please, be kind. There are much better ways to communicate your concerns than stating that we "randomly break backwards compatibility because something annoys us" or saying that "Apparently in Python-land its acceptable behavior to break backward This is a bit of a grey area because the Thanks in advance |
CC: @vstinner |
I was simply quoting you, you said you made this change because "We found that using -X ---- can be very confusing and annoying when you make typo". That is really not a good enough reason to break backwards compat. I apologise for my tone, but you guys need to take backwards compat more seriously. As for this issue, I dont care personally anymore, I have already worked around it. If you insist on breaking backwards compat then I suggest you add some loud warnings to the changelog. Just think of some poor dude running a script with python -X some_typo in it and that script now failing because python went from 3.10 to 3.11. |
We take backward compatibility very seriously. In this case, the incompatible change was done on purpose. It was too easy to make typos and Python would not notice them. For example,
For other command line options, Python is strict. For example, I don't see where the -X command line option is documented as it could be used by applications or 3rd party Python modules. It's documented as: Reserved for various implementation-specific options. https://docs.python.org/dev/using/cmdline.html#cmdoption-X IMO the most portable way to pass arbitrary options to Python is to use environment variables. It works on any Python versions and any operating system. By the way, I added sys.orig_argv to Python 3.10: https://docs.python.org/dev/library/sys.html#sys.orig_argv
I'm not comfortable with your tone of your messages. If you are affected and you consider that the change is not properly advertized in https://docs.python.org/dev/whatsnew/3.11.html I suggest you proposing a pull request to better document the change. Python is maintained by volunteers, you're are not a customer of a paid service. |
On Thu, Jun 23, 2022 at 08:20:32AM -0700, Victor Stinner wrote:
> I apologise for my tone, but you guys need to take backwards compat more seriously.
We take backward compatibility very seriously. In this case, the incompatible change was done on purpose.
No, you dont. Every python release breaks something, not to
mention the developer decades you wasted with 2 -> 3.
It was too easy to make typos and Python would not notice them. For example, ``python3.10 -X fautlhandler`` does **not** enable faulthandler and Python 3.10 doesn't say anything about the typo. There are many ways to introduce typos:
And given that it is so easy to make typos and not notice them, there
could well be lots of places where python is invoked with those typos
that will now break. What you should do is output a warning to stderr
for a release or two before causing catastrophic failure.
* pycacheprefix or pycache-prefix instead of pycache_prefix
* UTF8 or utf_8 or utf-8 instead of utf8
* no_debug_range instead of no_debug_ranges
* etc.
For other command line options, Python is strict. For example, ``python -o`` fails with a fatal error, whereas ``python -O`` works as expected. Same for long options like ``--check-hash-based-pyc=always`` (instead of ``--check-hash-based-pycs=always``).
---
I don't see where the -X command line option is documented as it could be used by applications or 3rd party Python modules. It's documented as: **Reserved for various implementation-specific options**.
I quoted *and* linked to the documentation in python 3.11 above.
https://docs.python.org/dev/using/cmdline.html#cmdoption-X
IMO the most portable way to pass arbitrary options to Python is to use environment variables. It works on any Python versions and any operating system.
Right, use environment variables that propagate to all subprocesses
unless you remember to delete them.
By the way, I added sys.orig_argv to Python 3.10: https://docs.python.org/dev/library/sys.html#sys.orig_argv
And?
---
> If you insist on breaking backwards compat then I suggest you add some loud warnings to the changelog.
I'm not comfortable with your tone of your messages.
Christ on a cross. Get off your high horse. You broke backward
compatibility without a second thought. There is not a single mention of
the impact of this backwards compat break in the discussion for this
change. That's obviously par for the course for this project.
If you are affected and you consider that the change is not properly advertized in https://docs.python.org/dev/whatsnew/3.11.html I suggest you proposing a pull request to better document the change. Python is maintained by volunteers, you're are not a customer of a paid service.
Right you are a volunteer so you get to do boneheaded things. Good for
you. I don't care, I already worked around this, and no, not by using
environment variables. I reported it to you solely to *help* you avoid
further pain to your userbase. But, of course, this coven of "volunteers" is
too busy using my tone as an excuse to avoid doing anything useful.
Wasted enough of my life on this. Good Bye.
|
I am fine reverting the change in 3.11 and adding a deprecation period if we all agree that the API is considered public. I think personally I agree with @zooba and the OP, this change should be reverted (at least in 3.11) and a deprecation warning should be emitted. |
What about deprecating un-namespaced options (namespacing to be bikeshedded)? So if it doesn't start with *waves hands* then it has to match one of our options, but otherwise it's passed through. Clearly there are people with uses for this, and I've used it myself to inject arguments that weren't suitable for env variables for whatever reason (typically to avoid it being passed on to subprocesses but without having to control subprocess launches). Offering something to migrate to that isn't OS specific seems the least we can do here. |
If there's a use case for which people have been using -x then we should definitely provide something to meet that need. However, the status quo isn't viable due to the typo problem. A namespace prefix on options could work. |
For the time being, I am opening a revert |
I have merged the revert as the change was technically backwards incompatible and we are super close to RC1: We can discuss bringing it back and how for future versions. |
Thanks for reverting this. I don't think making I understand the typo problem. But people should be testing that their flags are doing what they intended in some form or another. At most I suggest just issuing a warning about an unsupported -X flag name when in PYTHONVERBOSE mode. |
Ok, let's close the issue as "won't fix". |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: