You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
REPL shows source code from _pyrepl.__main__.py #129098
When using compile built-in function and providing non-existing filename, REPL shows source lines from _pyrepl.__main__.py if exception occurs. Reproducer:
Python 3.14.0a4+ (heads/main-dirty:ed6934e71e5, Jan 20 2025, 18:59:25) [Clang 16.0.0 (clang-1600.0.26.6)] on darwin
Type "help", "copyright", "credits" or "license"for more information.
>>> eval(compile("val", "non-existing-file", "eval"))
Results in:
Traceback (most recent call last):
File "<python-input-0>", line 1, in<module>
eval(compile("val", "non-existing-file", "eval"))
~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "non-existing-file", line 1, in<module># Important: don't add things to this module, as they will end up in the REPL's
NameError: name 'val' is not defined. Did you mean: 'eval'?
>>>
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 from Lib/_pyrepl/__main__.py.
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__ in FrameSummary (maybe it is somewhere else?). Filename there is still non-existing-file in this case. And then it passes it to linecache.getline. And from there it falls back to _py_repl.__main__ after failing to find file. FTR:
I'm sorry, I wasn't explicit enough. What I meant is that we should fix _pyrepl/* and not linecache or traceback itself. 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 (linecache and traceback are technically correct; it's the fact that we don't set a filename for compile that becomes problematic).
Btw, I think we have an existing issue for this one but I don't remember which one it is...
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 that linecache won't cache it without a change to linecache itself.
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.
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 which get_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.
So it appears to work. In linecache this would do the following:
# Try for a __loader__, if availableifmodule_globalsand'__name__'inmodule_globals: # <- ok spec=module_globals.get('__spec__') # will be explicitly Nonename=getattr(spec, 'name', None) ormodule_globals['__name__']
loader=getattr(spec, 'loader', None) # will be NoneifloaderisNone:
loader=module_globals.get('__loader__') # will be Noneget_source=getattr(loader, 'get_source', None) # will be Noneifnameandget_source: # will be skippeddefget_lines(name=name, *args, **kwargs):
returnget_source(name, *args, **kwargs)
cache[filename] = (get_lines,)
returnTruereturnFalse# and we get here
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)
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.
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 compile and exec. Finally, this issue does not occur in a pure Python script.
Activity
picnixz commentedon Jan 20, 2025
cc @pablogsal @ambv
WolframAlph commentedon Jan 20, 2025
@picnixz
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:cpython/Lib/traceback.py
Lines 341 to 351 in ed6934e
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.
picnixz commentedon Jan 20, 2025
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...
WolframAlph commentedon Jan 23, 2025
I will try to get more context and see how it can be fixed better if that's okay.
ambv commentedon Jan 23, 2025
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.picnixz commentedon Jan 23, 2025
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)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).ambv commentedon Jan 23, 2025
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.picnixz commentedon Jan 24, 2025
By adding
__spec__ = __loader__ = Noneto_pyrepl/__main__.py, I have the following output:So it appears to work. In
linecachethis would do the following:WolframAlph commentedon Jan 24, 2025
Doesn't it break other things somewhere?
picnixz commentedon Jan 24, 2025
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)
StanFromIreland commentedon Feb 26, 2025
Benedkit's fix does not work for me.
The error does not occur when using a "<foo>" placeholder
picnixz commentedon Feb 28, 2025
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.picnixz commentedon Mar 1, 2025
Ok: the reason why
<...>placeholders don't exhibit the bug is because they are treated specially bylinecache:cpython/Lib/linecache.py
Lines 98 to 99 in ddc27f9
cpython/Lib/linecache.py
Lines 179 to 180 in ddc27f9
StanFromIreland commentedon Mar 1, 2025
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?
picnixz commentedon Mar 1, 2025
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.StanFromIreland commentedon Mar 1, 2025
I see, you should open a PR with your fix then :-)
WolframAlph commentedon Mar 3, 2025
@picnixz thanks for writing the fix as I did not have time to do it myself. We can close my PR, right?
picnixz commentedon Mar 3, 2025
I think we can, because I don't think we'll go that way at least.
gh-129098: avoid using content of `_pyrepl/__main__.py` when reportin…
pythongh-129098: avoid using content of `_pyrepl/__main__.py` when re…
[3.13] gh-129098: avoid using content of `_pyrepl/__main__.py` when r…
Revert "pythongh-129098: avoid using content of `_pyrepl/__main__.py`…