Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New importer #1671

Closed
wants to merge 9 commits into from
Closed

Conversation

brandonwillard
Copy link
Member

This is an update (rebase to current master) of #1518, along with implemented hyc byte-compilation (copy of py_compile), removed Python 2.x compatibility (Hy no longer supports it), and importer tests converted to the new importlib.

(I couldn't find a good way to contribute changes to @vodik's existing PR; that's why this exists)

Also, a lot of utility code was removed in 5960c90, because it came directly from importlib._bootstrap and/or importlib._bootstrap_external and, since we were already importing from importlib._bootstrap, it seemed just as reasonable to use the necessary functions from their original source; however, these modules are not public-facing. At the very least, we can have everything in working order by simply accounting for discrepancies (e.g. importlib._bootstrap_external appearing after 3.5) across versions.

vodik and others added 9 commits August 12, 2018 13:44
Add the necessary bits to provide a Python 3.4+ compliant import
machinery. This means we now have our own PathFinder, which we register
as in sys.meta_path, our own FileFinder, and our own Loader.

Add a compatability later that builds on the Python 3 machinery to make
it best-effort backwards compatible with Python 2.
We can't reply on hy_eval here as it doesn't properly associate the
appropriate file information.

Fixes hylang#1395
There are conflicts between certain combinations of `pytest` and `py` that cause
recursion issues (see pytest-dev/pytest#2501 and
pytest-dev/pytest#2485).  The same errors were showing
up in Travis builds when pip pulled the most recent versions of these two
packages.
@Kodiologist
Copy link
Member

I don't think we're ready to drop Python 2 support, sorry. The Python folks are still supporting it till 1 Jan 2020.

@Kodiologist
Copy link
Member

I should add that even if you want to drop Python 2, it's a big enough change that it should get its own PR.

@brandonwillard
Copy link
Member Author

Agh, yeah; well, I'm in the process of reducing the import machinery to the bare essentials in 3.x, so, once I've figured out exactly what that should look like, I'll re-add 2.7 support.

@brandonwillard
Copy link
Member Author

brandonwillard commented Aug 15, 2018

In the search for minimal integration with Python >= 3.4 importlib, I found this small monkey-patch sufficient for at least the basics (e.g. regular imports of .hy files) and even direct use of standard py_compile:

import importlib

from hy.compiler import hy_compile
from hy.lex import tokenize
from hy.models import HyObject, HyExpression, HySymbol

importlib.machinery.SOURCE_SUFFIXES += ['.hy']

# Keep a copy of the original method
_source_to_code = importlib.machinery.SourceFileLoader.source_to_code

def hy_parse(source):
    source = re.sub(r'\A#!.*', '', source)
    return HyExpression([HySymbol("do")] + tokenize(source + "\n"))

# New, patched method that considers .hy files
def _hy_source_to_code(self, data, path, *, _optimize=-1):
    if os.path.isfile(path) and path.rpartition('.')[-1] == 'hy':
            data = hy_compile(hy_parse(data.decode("utf-8")), self.name)

    return _source_to_code(self, data, path, _optimize=_optimize)

# Patch the standard file loader
importlib.machinery.SourceFileLoader.source_to_code = _hy_source_to_code

If this approach works across the board, we could remove the custom copy of py_compile and almost all existing importlib code in this branch. Patches like this aren't usually ideal, but at least it's patching a method specified in the importlib API/spec without blatantly compromising that spec. As well. the addition to SOURCE_SUFFIXES doesn't seem too out of place, since importlib itself updates it "dynamically".

The current approach is a non-trivial work-around for FileFinder's inflexibilities (see here) that effectively supplants the standard import mechanisms, so it's probably worth seriously considering an extremely terse patch that provides the same functionality and more.

@Kodiologist
Copy link
Member

Okay, that sounds like a decent thing to try.

@brandonwillard brandonwillard deleted the new-importer branch August 20, 2018 04:59
@vodik
Copy link
Contributor

vodik commented Aug 20, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants