Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-46927: Prevent readline from overriding environment#133585
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
python-cla-botbot commentedMay 7, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Readline library will set the LINES and COLUMNS environment variablesduring initialization. One might expect these variables to be updatedlater on SIGWINCH, but this is not the case with cpython.As a consequence, when the readline module is imported, any processlaunched from cpython with the default environment will have LINES andCOLUMNS variables set, potentially to a wrong value.Use the rl_change_environment global variable to disable this initialsetup of the environment variables.
| @@ -1323,6 +1323,13 @@ setup_readline(readlinestate *mod_state) | |||
| /* The name must be defined before initialization */ | |||
| rl_readline_name = "python"; | |||
| #if !defined(__APPLE__) | |||
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'm a bit surprised I had to rely on the__APPLE__ define here. As I understand apple OS is using libedit, so I would have thought relying onWITH_EDITLINE would be enough. But the macOS pipeline proved me wrong...
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.
Please add a test case. The file you're looking for istest_readline.
Misc/NEWS.d/next/Core_and_Builtins/2025-05-07-18-31-31.gh-issue-46927.sF02gj.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| @@ -405,6 +407,12 @@ def test_write_read_limited_history(self): | |||
| # So, we've only tested that the read did not fail. | |||
| # See TestHistoryManipulation for the full test. | |||
| @requires_subprocess() | |||
| def test_environment_is_not_modified(self): | |||
| original_env = dict(os.environ) | |||
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.
Did you forget to add the use ofsubprocess here? This test always passes.
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.
Err sorry, I reworked the test and forgot to remove the subprocess requirement.
But I do confirm this test fails if I revert the initial commit of the PR.
Uh oh!
There was an error while loading.Please reload this page.
Readline library will set the LINES and COLUMNS environment variables during initialization. One might expect these variables to be updated later on SIGWINCH, but this is not the case with cpython.
As a consequence, when the readline module is imported, any process launched from cpython with the default environment will have LINES and COLUMNS variables set, potentially to a wrong value.
Use the rl_change_environment global variable to disable this initial setup of the environment variables.