Skip to content

Conversation

@jdemeyer
Copy link
Contributor

@jdemeyer jdemeyer commented Apr 30, 2018

@jdemeyer
Copy link
Contributor Author

@embray
@pfmoore

@pfmoore
Copy link
Member

pfmoore commented Apr 30, 2018

Looks good to me in relation to my comments on python-list, that this is what the docs say. But I haven't reviewed bpo-32797, so I'm not going to approve this without @embray's feedback.

Further discussion should really be on the issue tracker, so I'll add any other comments I have there.

@embray
Copy link
Contributor

embray commented May 2, 2018

LGTM. I don't know exactly what would be impacted by this if the source code search falls through for normal extension modules, but regardless the current behavior is a regression.

sageb0t pushed a commit to sagemath/sage-archive-2023-02-01 that referenced this pull request Jun 17, 2018
I've been getting to the bottom of why tracebacks aren't displaying
Cython sources on Python 3.  The problem turns out to be multi-faceted,
and fixing it right would also mean fixing some things in general, since
the way it works now is a hack.

Basically, the only reason it works at all on Python 2, is that the
`linecache` module on Python 2 has some code which, given a relative
filename (with an extension unrecognized by the import system) like
`sage/rings/integer.pyx`, it will search for this file under all
`sys.path` entries and, if found, read the source lines from that file.

This, in turn, only works for Sage because we actually install the
`.pyx` sources in the sage package.

This does not work in Python 3.  In `linecache.updatecache`, before it
tries the `sys.path` search, it checks if the module object has a
`__loader__`, and calls its `get_source()` method if it exists.  On
Python 2 this isn't a problem since modules don't necessarily have a
`__loader__`, and in particular extension modules don't.  But on the
reworked import system in Python 3, pretty much every module has a
`__loader__`--in the case of extension modules an `ExtensionFileLoader`.
But the built-in `ExtensionFileLoader` of course knows nothing about
Cython so its `get_source()` method just returns `None`.
`linecache.updatecache` assumes this is correct (why would the loader
lie?) and returns.

The simplest way to fix this is to remove the `get_source()` method from
the `ExtensionFileLoader` class. This way, Python 3 works the same way
as Python 2.

'''Upstream''':

- https://bugs.python.org/issue32797
- python/cpython#6653

URL: https://trac.sagemath.org/24681
Reported by: embray
Ticket author(s): Jeroen Demeyer, Erik Bray
Reviewer(s): Frédéric Chapoton
@jdemeyer
Copy link
Contributor Author

jdemeyer commented Jul 5, 2018

@pfmoore: Erik Bray commented positively on this PR, so can this be approved then?

@encukou
Copy link
Member

encukou commented Oct 24, 2018

This was auto-closed by merging #9540. @jdemeyer, please re-open if that wasn't intentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants