-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-57684: Add the -p command line option #92361
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
Let me try to explain what problem is being solved by adding this new Let's say that we have a Python module called
By default, Python 3.11 can import it:
Good. Using the new
Problems arise when the
Oh no! It's no longer possible to load On Python 3.10, well, the environment variable is just ignored and it just works:
So the problem is that programs which rely on Python 3.10 default sys.path behavior no longer works with Well, the first work around is to use
It works again! The problem is that the The purpose of the
The assumption is that only a minority of Python programs will need the A new feature of this option is the ability to use the isolated mode but still load code from the current working directory (CWD).
By default, PYTHONWARNINGS is read and import from CWD works:
The isolated mode excludes CWD from sys.path and so the import fails:
Using the new -p option, isolated mode features are still in place (ignore PYTHONWARNINGS env var), but importing from CWD is now possible:
|
Without this |
@gpshead: Here is a concrete implementation add the |
(copy of my comment on #31542)
Flipping the default can now be done by setting The problem is that right now, if IMO command line options must have the priority over environment variables. Python does not provide command line options to disable the effect of all Python environment variables. For example, The Python UTF-8 Mode is way more likely to cause compatibility issues and so We don't have to add |
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.
Thanks, @vstinner.
For whatever its worth, I'm not a highly experienced core dev like yourself and others, but the rationale for including -p
as a counterpart to -P
outlined here and in the previous PR makes eminent sense to me.
I do have a handful of grammar/punctuation fixes and phrasing clarifications for the docs and UI text, if that's alright.
Doc/using/cmdline.rst
Outdated
The :option:`-p` option takes precedence over the :option:`-P` and | ||
:option:`-I` (isolated mode) options and the :envvar:`PYTHONSAFEPATH` | ||
environment variable. |
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'd be concerned this might not be entirely clear that this doesn't override all of -I
, just the -P
it implies. What about something like this?
The :option:`-p` option takes precedence over the :option:`-P` and | |
:option:`-I` (isolated mode) options and the :envvar:`PYTHONSAFEPATH` | |
environment variable. | |
The :option:`-p` option takes precedence over the :option:`-P` option | |
(either passed explicitly or implied by :option:`-I`, isolated mode) | |
and the :envvar:`PYTHONSAFEPATH` environment variable. |
Misc/NEWS.d/next/Core and Builtins/2022-05-06-03-53-47.gh-issue-57684.ITjOhD.rst
Outdated
Show resolved
Hide resolved
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.
Looks good, but agree with @CAM-Gerlach's wording suggestion. We may want to merge early so we get this in before the feature freeze.
@CAM-Gerlach: It was really hard for me to explain the subtle differences between the default Python behavior and the usage of this option which is a corner case. Thanks for your review, I enhanced the documentation to take your review in account. |
PR rebased to get the Docs CI fix: #92395 |
3.11 is closed to features at this point so you'd need a release manager exception. I think the nicer world is one in which python doesn't put the the main.py path in sys.path (what PYTHONSAFEPATH now provides to those who opt-in). I'm not suggesting we ever get to flip to that default. But I think adding an opt-out flag to prevent cases where someone may wants to explicitly avoid that nicer world isn't super compelling. At least not any more so than someone who is capable of updating their
which is basically the same thing and works regardless of Python version. (adjust as needed based on if they wanted $CWD or script directory or "" to be added - the point being the logic that |
Yes, but many potential use cases don't necessarily involve running scripts that the user wrote themself, as opposed to, for example, invocations of a test runner or other tools, or modules, scripts or other entrypoints the user does not directly control. Furthermore, they may wish to run the same script/tool/module in different contexts, or for testing, with and without the CWD on the path without manually editing its code or adding a whole CLI of its own to trigger hacking sys.path. It would seem to me that this opt-out option is, perhaps counterintuitively at first, actually a path closer to that better world of a safer default, because it allows users/sysadmins to just set |
About "case sensitive": it's a deliberate choice to use
|
The problem is when you simply cannot modify the source code of an application impacted by The current workaround is to manually unset PYTHONSAFEPATH to run these applications. Replace:
with:
I propose instead:
The -p option also gives the ability to put the option on the shebang. IMO it's simpler and more reliable than having to modify sys.path manually. |
Add the -p option to ignore the -P option and the PYTHONSAFEPATH environment variable. The -p option takes precedence over -P and -I options and the PYTHONSAFEPATH environment variable. test_cmd_line, test_cmd_line_script, test_pdb, test_runpy and test_tools now use the -p option to ignore the PYTHONSAFEPATH environment variable.
The purpose of the -p option is similar to the -R option which ignores the PYTHONHASHSEED environment variable: the -p option ignores PYTHONSAFEPATH environment variable. |
I rebased my PR and changed it to target Python 3.12. |
For Fedora, @hroncok and me plan to add -P to Python program shebangs: see https://fedoraproject.org/wiki/Changes/PythonSafePath For the Fedora use case, the PYTHONSAFEPATH environment variable is not needed. I would prefer to have PYTHONSAFEPATH env var with the -p option. Either have both in Python 3.11 (add -p option), or have none in Python 3.11 (remove PYTHONSAFEPATH env var). @hroncok @pitrou @gpshead: What do you think? Is it better to postpone PYTHONSAFEPATH to Python 3.12 and also add -p to Python 3.12, or request Python 3.11 release manager to get an exception to add the -p option? IMO having PYTHONSAFEPATH=1 without -p makes PYTHONSAFEPATH=1 barely usable. They are still too many programs which rely on Python 3.10 default behavior: add unsafe paths to sys.path. |
Are you proposing reverting just the environment variable or I get the idea that scripts that would be broken in an environment with So, no, I don't think that not having As a reference, I think there are many ways scripts can get broken by |
I'm happy with -P in Python 3.11, I don't want to change it :-)
The world is complicated :-) There are use cases where you want to accept most PYTHON env vars, but not be affected by PYTHONSAFEPATH.
Sometimes, you cannot modify the source code. For example, to run a /usr/bin/ application as a regular user. The existing workaround is to run a program in its own environment and drop PYTHONSAFEPATH from this environment. I dislike modifying As an user, I would like to have a very simple way to opt-in for Python 3.10 sys.path behavior: get the "unsafe" path. |
If you cannot modify the source, you cannot change the shebang either. What is it that you except the user will do exactly if the the
Like this?
That however doesn't work, PYTHONSAFEPATH can be set to anything non-empty apparently, which is a tad confusing. This works (but is not very intuitive):
|
See the PR, especially changes in tests. The -p is added to the command line to run programs implemented in Python. For example, I have no idea how the FREEZE program works and I really don't want to modify it. All I need is to add -p to its command line. No need to change its shebang. For Tools/freeze/test/freeze.py, a shebang wouldn't help since the program is run directly with "python (...)".
Yeah, it's annoying but I chose that to mimick the design of all other PYTHON env vars. I agree that it's annoying to define an environment variable to an empty value, and sometimes it's not possible. |
It seems like there is no strong requirement for adding a -p option if the Python default sys.path behavior doesn't change. We can reconsider adding -p option if the default changes tomorrow. |
Add the -p option to ignore the -P option and the PYTHONSAFEPATH
environment variable. The -p option takes precedence over -P and -I
options and the PYTHONSAFEPATH environment variable.
test_cmd_line, test_cmd_line_script, test_pdb, test_runpy and
test_tools now use the -p option to ignore the PYTHONSAFEPATH
environment variable.