gh-126019 Fix inspect.getsource for classes created in PyREPL#126032
gh-126019 Fix inspect.getsource for classes created in PyREPL#126032devdanzin wants to merge 11 commits intopython:mainfrom
Conversation
| # Protect against fetching the wrong source in PyREPL | ||
| if object.__module__ == '__main__': | ||
| raise OSError('source code not available') |
There was a problem hiding this comment.
Hmm, I'm worried about this being a breaking change. __module__ now has more precedent over __file__--maybe people could have been relying on that?
There was a problem hiding this comment.
Yeah I share similar concerns. Also why this only affects pyREPL and not any other main module ?
There was a problem hiding this comment.
Also why this only affects pyREPL and not any other main module ?
I think because for other __main__ modules, __main__.__file__ points to the correct source code.
In basic REPL, sys.modules["__main__"] doesn't have a __file__ attribute, so it raises OSError('source code not available') when inspected.
But with PyREPL, sys.modules["__main__"].__file__ is _pyrepl.__main__, which is where the source code is being pulled from. So we could instead skip loading from __file__ if we detect PyREPL is in use, what do you think?
There was a problem hiding this comment.
Wait, we could just delete __file__ from __main__, right?
diff --git a/Lib/_pyrepl/main.py b/Lib/_pyrepl/main.py
index a6f824dcc4a..17c55eb9c9f 100644
--- a/Lib/_pyrepl/main.py
+++ b/Lib/_pyrepl/main.py
@@ -35,6 +35,7 @@ def interactive_console(mainmodule=None, quiet=False, pythonstartup=False):
import __main__
namespace = __main__.__dict__
namespace.pop("__pyrepl_interactive_console", None)
+ namespace.pop("__file__", None)
# sys._baserepl() above does this internally, we do it here
startup_path = os.getenv("PYTHONSTARTUP")There was a problem hiding this comment.
Yeah, I think that would be much less invasive.
|
I've changed the approach to removing Some changes had to be made to tests that expected Wording of the NEWS entry seems poor to me, would gladly apply any suggested improvements. |
| def test_inspect_getsource(self): | ||
| code = "import inspect\nclass A: pass\n\nprint(inspect.getsource(A))\nexit()\n" | ||
| output, exit_code = self.run_repl(code) | ||
| self.assertEqual(exit_code, 0) | ||
| self.assertIn("OSError('source code not available')", output) |
There was a problem hiding this comment.
I think that testing whether inspect.getsource works on other objects (functions, methods, traceback, frames) is worthwhile, but I'm not sure if it should be done in this PR. So, I'd be fine with doing it in a follow-up
|
This PR is stale because it has been open for 30 days with no activity. |
inspect.getsourcewould fetch sourcecode from_pyrepl.__main__for classes defined in the REPL when using PyREPL becausesys.modules["__main__"]points to that module.This PR makes
inspect.getsourceraiseOSError('source code not available')for classes defined in the REPL by detecting that.__module__is"__main__". Tests pass with this change, but if you know of a corner case that would be broken by it please say so.