Repository navigation
REPL shows source code from _pyrepl.__main__.py #129098
Description
Activity
- addedtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
on Jan 20, 2025 - addedstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytopic-replRelated to the interactive shellRelated to the interactive shell
on Jan 20, 2025 cc @pablogsal @ambv
Or we could check whether the reported traceback is correct or not (or we could hook compile itself?)
Traceback seems like good place to check for it instead of linecache, but I did not find any reference to
_pyrepl.__main__inFrameSummary(maybe it is somewhere else?). Filename there is stillnon-existing-filein this case. And then it passes it tolinecache.getline. And from there it falls back to_py_repl.__main__after failing to find file. FTR:
Lines 341 to 351 in ed6934e
def _set_lines(self): if ( self._lines is None and self.lineno is not None and self.end_lineno is not None ): lines = [] for lineno in range(self.lineno, self.end_lineno + 1): # treat errors (empty string) and empty lines (newline) as the same lines.append(linecache.getline(self.filename, lineno).rstrip()) self._lines = "\n".join(lines) + "\n" So I would need someone to point me in the right direction where it should be handled as I don't have much context here.
I'm sorry, I wasn't explicit enough. What I meant is that we should fix
_pyrepl/*and notlinecacheortracebackitself. Those are general-purpose utilities that should not depend on the REPL itself. The fact that the code for_pyrepl.__main__is shown is because we're essentially executing everything here (not sure anymore). So, we need to patch the REPL files, not the other files (linecacheandtracebackare technically correct; it's the fact that we don't set a filename forcompilethat becomes problematic).Btw, I think we have an existing issue for this one but I don't remember which one it is...
Reacted by Sergey B Kirpichev and Petr ViktorinI will try to get more context and see how it can be fixed better if that's okay.
The current state of things is already due to an attempt to hide _pyrepl internals. I'm not sure there's an easy way to change
_pyrepl/*in a way thatlinecachewon't cache it without a change tolinecacheitself.I disagree that it's somehow invalid to patch
linecachefor this. What you're trying to do here is exactly to makelinecachespecial-case_pyrepl/*, so that would be the easiest thing to do here.I'm not sure there's an easy way to change _pyrepl/* in a way that linecache won't cache it without a change to linecache itself
Wouldn't it be possible with some custom
__loader__in_pyrepl.__main__for whichget_source()returns None? (not sure if it could cause other issues though)I disagree that it's somehow invalid to patch linecache for this. What you're trying to do here is exactly to make linecache special-case _pyrepl/*, so that would be the easiest thing to do here.
What I meant is that I'm more worried about the fact that we have some hardcoded
_pyrepl.__main__guard here. If you however think it's the easiest way to do it maybe this can motivate that choice but... it feels a bit hacky =/ OTOH, if the repl has special cases elsewhere in the stdlib, then maybe the change is fine. But if possible, it would be nicer if we could make it work in a more generic way (though, if this properly fixes the issue, it could be fine).Hm,
__loader__might be a good idea to pursue. I was worried about putting any logic in_pyrepl.__main__, as anything you put there will end up in globals of the interactive shell. But since there is a default__loader__there already, maybe this can work.By adding
__spec__ = __loader__ = Noneto_pyrepl/__main__.py, I have the following output:>>> eval(compile("val", "missing", "eval")) Traceback (most recent call last): File "<python-input-0>", line 1, in <module> eval(compile("val", "missing", "eval")) ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "missing", line 1, in <module> NameError: name 'val' is not defined. Did you mean: 'eval'? >>>
So it appears to work. In
linecachethis would do the following:# Try for a __loader__, if available if module_globals and '__name__' in module_globals: # <- ok spec = module_globals.get('__spec__') # will be explicitly None name = getattr(spec, 'name', None) or module_globals['__name__'] loader = getattr(spec, 'loader', None) # will be None if loader is None: loader = module_globals.get('__loader__') # will be None get_source = getattr(loader, 'get_source', None) # will be None if name and get_source: # will be skipped def get_lines(name=name, *args, **kwargs): return get_source(name, *args, **kwargs) cache[filename] = (get_lines,) return True return False # and we get here
Doesn't it break other things somewhere?
Doesn't it break other things somewhere?
I don't think so because we're only changing the loader and spec of a specific module. But we should investigate (I don't have time now and I'm leaving for 2 weeks tomorrow)
Benedkit's fix does not work for me.
The error does not occur when using a "<foo>" placeholder
>>> eval(compile("val", "<string>", "eval")) Traceback (most recent call last): File "<python-input-0>", line 1, in <module> eval(compile("val", "<string>", "eval")) ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "<string>", line 1, in <module> NameError: name 'val' is not defined. Did you mean: 'eval'?I don't understand the problem. My fix is to avoid jumping to the pyrepl main. In this case
<string>is still not an existing file. Should it output something else?Note: traceback or linecache treats
<...>locations differently but I don't remember where.Ok: the reason why
<...>placeholders don't exhibit the bug is because they are treated specially bylinecache:Lines 98 to 99 in ddc27f9
if not filename or (filename.startswith('<') and filename.endswith('>')): return [] Lines 179 to 180 in ddc27f9
if not filename or (filename.startswith('<') and filename.endswith('>')): return False I don't think this is just a repl issue, this is just that when the file does not exist it returns the first line of the file being run, why on earth is it implemented this way?
That's how tracebacks are constructed. The problem is just that we should actually skip that file when reporting the traceback, namely, making it "inexisting" from a summary PoV. In addition, it's probably because we're actually using
compileandexec. Finally, this issue does not occur in a pure Python script.I see, you should open a PR with your fix then :-)
@picnixz thanks for writing the fix as I did not have time to do it myself. We can close my PR, right?
I think we can, because I don't think we'll go that way at least.
- added a commit that references this issue
on Apr 20, 2025 - added a commit that references this issue
on Apr 20, 2025 - added3.13only security fixesonly security fixes3.14bugs and security fixesbugs and security fixes
on Apr 24, 2025
Bug report
Bug description:
When using
compilebuilt-in function and providing non-existing filename, REPL shows source lines from_pyrepl.__main__.pyif exception occurs. Reproducer:Results in:
traceback with following line:
# Important: don't add things to this module, as they will end up in the REPL's. This is first line fromLib/_pyrepl/__main__.py.CPython versions tested on:
CPython main branch
Operating systems tested on:
No response
Linked PRs
_pyrepl.__main__.pyinside REPL #129099_pyrepl/__main__.pywhen reporting tracebacks #130721_pyrepl/__main__.pywhen reporting tracebacks (GH-130721) #132755