-
-
Notifications
You must be signed in to change notification settings - Fork 30.4k
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
gh-120678: pyrepl: Include globals from modules passed with -i
#120904
Conversation
Cc. @eryksun if you're interested! |
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.
Thank you. LGTM 🚀
The reason why >>> __package__
'_pyrepl'
>>> __name__
'__main__'
>>> __spec__
ModuleSpec(name='_pyrepl.__main__', loader=<_frozen_importlib_external.SourceFileLoader object at 0x103a020c0>, origin='/Volumes/RAMDisk/cpython/Lib/_pyrepl/__main__.py')
>>> __file__
'/Volumes/RAMDisk/cpython/Lib/_pyrepl/__main__.py' We'd need to work around that. |
@@ -487,6 +489,18 @@ def prepare_reader(self, events): | |||
reader.can_colorize = False | |||
return reader | |||
|
|||
def test_stdin_is_tty(self): |
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.
This was helpful to me when debugging weird behavior so I decided to keep it.
@@ -549,7 +597,7 @@ pymain_repl(PyConfig *config, int *exitcode) | |||
*exitcode = (run != 0); | |||
return; | |||
} | |||
int run = pymain_run_module(L"_pyrepl", 0); | |||
int run = pymain_start_pyrepl_no_main(); |
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.
My first intuition was to simply PyImport_ImportModule("_pyrepl.__main__")
but then the entire shell session is a side-effect of an on-going import, which was ugly and caused some funny edge cases (a non-zero exit was technically an ImportError).
So instead we do the full dance of running _pyrepl.main.interactive_console()
. While doing that I'm smuggling PYTHONSTARTUP support because:
- you can always remove PYTHONSTARTUP from your environment to not have it;
- but previously you could not get PYTHONSTARTUP to be executed even if you wanted.
Thanks for picking this up @ambv! |
It's team work: I push the shoddy changes, you get the |
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 can't give an approving review (since this is strictly speaking still "my PR" 😄), but from what I can tell this all LGTM! (With the caveat that I'm far from the world's greatest C programmer, so I could easily have missed something)
Thanks @AlexWaygood for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
pythonGH-120904) (cherry picked from commit ac07451) Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: Łukasz Langa <[email protected]>
GH-121916 is a backport of this pull request to the 3.13 branch. |
|
|
|
|
…i` (GH-120904) (#121916) (cherry picked from commit ac07451) Co-authored-by: Alex Waygood <[email protected]> Co-authored-by: Łukasz Langa <[email protected]>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
In order to fix the regression caused by #119547, this PR goes back to the original fix that was proposed in the early commits of #119547.
The REPL must use the globals from
sys.modules["__main__"]
in case a different module has been run as__main__
using-i
; in order to prevent globals from_pyrepl.__main__
from leaking into the global namespace, we move the vast majority of the globals out of__main__.py
instead of running the REPL with an entirely fresh set of globals.Some small filtering of globals is additionally done in
Lib/_pyrepl/simple_interact.py
, so that we're able to import functions and classes in_pyrepl.__main__
without these globals leaking into the REPL environment. (This filtering only works for things with__module__
attributes, though, so it's still best to keep_pyrepl/__main__.py
as minimal as possible.)This still doesn't exactly match the behaviour of the old REPL for some dunders:
I'm not really sure why that is!
-i
#120678