Skip to content

REPL shows source code from _pyrepl.__main__.py #129098

Description

@WolframAlph

Bug report

Bug description:

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.

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

Activity

  1. added
    stdlibStandard Library Python modules in the Lib/ directory
    topic-replRelated to the interactive shell
    on Jan 20, 2025
  2. picnixz commented on Jan 20, 2025

    @picnixz
    Member
  3. WolframAlph commented on Jan 20, 2025

    @WolframAlph
    ContributorAuthor

    @picnixz

    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:

    cpython/Lib/traceback.py

    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.

  4. picnixz commented on Jan 20, 2025

    @picnixz
    Member

    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...

  5. WolframAlph commented on Jan 23, 2025

    @WolframAlph
    ContributorAuthor

    I will try to get more context and see how it can be fixed better if that's okay.

  6. ambv commented on Jan 23, 2025

    @ambv
    Contributor

    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.

  7. picnixz commented on Jan 23, 2025

    @picnixz
    Member

    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).

  8. ambv commented on Jan 23, 2025

    @ambv
    Contributor

    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.

  9. picnixz commented on Jan 24, 2025

    @picnixz
    Member

    By adding __spec__ = __loader__ = None to _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 linecache this 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
  10. WolframAlph commented on Jan 24, 2025

    @WolframAlph
    ContributorAuthor

    Doesn't it break other things somewhere?

  11. picnixz commented on Jan 24, 2025

    @picnixz
    Member

    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)

  12. StanFromIreland commented on Feb 26, 2025

    @StanFromIreland
    Member

    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'?
    
  13. picnixz commented on Feb 28, 2025

    @picnixz
    Member

    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.

  14. picnixz commented on Mar 1, 2025

    @picnixz
    Member

    Ok: the reason why <...> placeholders don't exhibit the bug is because they are treated specially by linecache:

    cpython/Lib/linecache.py

    Lines 98 to 99 in ddc27f9

    if not filename or (filename.startswith('<') and filename.endswith('>')):
    return []

    cpython/Lib/linecache.py

    Lines 179 to 180 in ddc27f9

    if not filename or (filename.startswith('<') and filename.endswith('>')):
    return False

  15. StanFromIreland commented on Mar 1, 2025

    @StanFromIreland
    Member

    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?

  16. picnixz commented on Mar 1, 2025

    @picnixz
    Member

    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.

  17. StanFromIreland commented on Mar 1, 2025

    @StanFromIreland
    Member

    I see, you should open a PR with your fix then :-)

  18. WolframAlph commented on Mar 3, 2025

    @WolframAlph
    ContributorAuthor

    @picnixz thanks for writing the fix as I did not have time to do it myself. We can close my PR, right?

  19. picnixz commented on Mar 3, 2025

    @picnixz
    Member

    I think we can, because I don't think we'll go that way at least.

  20. added a commit that references this issue on Apr 20, 2025
  21. added a commit that references this issue on Apr 20, 2025
  22. added a commit that references this issue on Apr 20, 2025
  23. added
    3.13only security fixes
    3.14bugs and security fixes
    on Apr 24, 2025
  24. added a commit that references this issue on Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.13only security fixes3.14bugs and security fixesstdlibStandard Library Python modules in the Lib/ directorytopic-replRelated to the interactive shelltype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions