Description
Okay, as a preface, I don't like coming into a project I've just started contributing to and saying everything is wrong. So before unloading a giant structural change to the project, I'd like to explain what I found, explain why its wrong, and how to move forward.
As background, I've been actually hacking really hard at Hy, trying my hand at various open issues, like #1416 and #1482, but keep bumping into issues around Hy's importer. Frustrated, I started really digging into how the system works, as you guys may have noticed opening issues like #1512
So, having dug into depth of how the Hy importer works and I'm very confident to say we've gotten it wrong. This isn't just an issue of using deprecated APIs, but actually implementing the API incorrectly. I'm at the point that I'm not sure now how Hy ever worked to begin with.
The biggest sign that things are wrong is that runpy
doesn't work with Hy modules. runpy
uses the standard import mechanism (PEP 302), and since we hook into that, hy modules should Just Work.
In fact, the hy
command line utility probably should be using runpy
for hy <file>
and hy -m <module>
instead of its home rolled implementations. Once it does (and the various necessary fixes are in place), virtually all the outstanding issues like #1513 and #1466 just disappear. There is actually no need to roll our own.
That said, as a side effect, it also means that hy -m <module>
can launch python code. But I argue this is really desirable. It allows Python and Hy to glue together even better. For example:
hy -m aiohttp.web myproject:app_factory
This should be completely legal, and lets me easily launch an aiohttp server (python, naturally), but with a Hy based app factory. Currently, I'd have to provider a custom shim to make this work.
So, core issues of what's wrong and stuff that should be done:
- MetaLoader is missing expected public APIs:
get_code
andis_package
(even on Python 2). The causesrunpy
to chock - importer puts incorrect values for
__path__
. This causespkgutil.find_loader
to misbehave, which then preventsrunpy
from working as it can't find the appropriate loader inside Hy packages. - Logic of actually setting up
sys.modules
is imperfect. See Fix self imports #1513 for details - Create a separate loader for Python 3. The current loader is sufficient for Python 2 once fixed up, but basically built with deprecated APIs. Building a new loader around
importlib
is actually cleaner. Most of the implementation can be shared.
I'm 98% there of dealing with all these issues and closing a whole pile of open bugs. Its also a much more straightforward implementation than #1085 so should be more maintainable.