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

Hy will treat a Hy file as Python if it doesn't have a ".hy" file extension #1677

Closed
Kodiologist opened this issue Sep 4, 2018 · 9 comments

Comments

@Kodiologist
Copy link
Member

$ echo '(print 1 2)' >testcode
$ hy testcode
Traceback (most recent call last):
  File "/usr/local/bin/hy", line 12, in <module>
    sys.exit(hy_main())
  File "/usr/local/lib/python3.7/site-packages/hy/cmdline.py", line 369, in hy_main
    sys.exit(cmdline_handler("hy", sys.argv))
  File "/usr/local/lib/python3.7/site-packages/hy/cmdline.py", line 355, in cmdline_handler
    runpy.run_path(filename, run_name='__main__')
  File "/usr/local/lib/python3.7/runpy.py", line 261, in run_path
    code, fname = _get_code_from_file(run_name, path_name)
  File "/usr/local/lib/python3.7/site-packages/hy/importer.py", line 195, in _get_code_from_file
    code = compile(source, fname, 'exec')
  File "testcode", line 1
    (print 1 2)
           ^
SyntaxError: invalid syntax

@brandonwillard, you might want to take a look at this one.

@brandonwillard
Copy link
Member

brandonwillard commented Sep 4, 2018

Ah, yes, all the Hy-specific loader logic from #1672 is based on file extensions and falls back to Python builtins otherwise. In other words, it only enforces a choice between files recognized as Hy and Python.

Do we want to assume that extension-less files and/or unrecognized extensions are Hy? Should we raise an exception (from the cmdline tool) when we don't find/recognize an extension? Should we fail silently in those cases?

We can make some fairly straightforward changes to produce most of those conditions, but we probably want to be more laissez-faire than not. Python's choice to interpret every file as Python source seems sorta reasonable if only because it handles the entire import process — in which we are simply injecting Hy parsing capabilities. If we attempt to take over default behaviours, we might need to do a lot more just to preserve expected functionality elsewhere (e.g. make sure we don't block other importlib finders and loaders).

@refi64
Copy link
Contributor

refi64 commented Sep 4, 2018

I think anything passed via the command line should be automatically assumed to be a Hy module. Other than that, the import rules could probably stay intact.

@Kodiologist
Copy link
Member Author

Yeah, I think it's reasonable to expect a .hy file extension of Hy files that are imported, but hy SOMETHING should always interpret SOMETHING as Hy—unless, perhaps, it both has a file extension and that extension is .py or .pyc.

@Kodiologist
Copy link
Member Author

We also presumably want shebang lines to work:

$ printf '#!/usr/bin/env hy\n(print 1 2)\n' > myprog
$ chmod 755 myprog                                  
$ ./myprog
Traceback (most recent call last):
  …
  File "./myprog", line 2
    (print 1 2)
           ^
SyntaxError: invalid syntax

@brandonwillard
Copy link
Member

@kirbyfan64, that's the approach I was initially leaning toward, and it would require us to either stop using runpy in cmdline.py or force the runpy patch to use our loader. Both of those approaches avoid changing the importer/finder we put on sys.path_hooks and negatively affecting the basic import process (e.g. making it greedy or blocking downstream importers/finders); however, they also effectively impose their own greedy import process just for the cmdline and/or runpy.

@Kodiologist, handling a shebang is much more in-line with the approach I'm currently leaning toward, which attempts to determine whether or not a file is Hy source through extension and (simple) non-extension means. Such a change could be introduced in the way described above, or more generally through changes to the importer/finder. Since we implement the Python 2.7 importer/finder, it's clear where/how we should do this, but, with our minimally-invasive Python 3.x patch, we might need to do some research if a simple change to this line in _hy_source_to_code isn't enough.

Also, it's quite possible I'm over-thinking this, and that simply changing the extension conditions in the 2.7 loader, importer/finder and 3.x loader patch will work just fine. Still, I think it's safer to define a clear and constrained — albeit potentially non-standard (e.g. exclusively extension, shebang, and perhaps light syntax inference) — means of determining when a file is Hy source.

@refi64
Copy link
Contributor

refi64 commented Sep 4, 2018

Well IMO if someone's using Hy to run a non-Hy file, they're doing something very wrong...

@brandonwillard
Copy link
Member

Even so, we don't want to break valid Hy code that triggers imports or runpy invocations of other file types. That's my main concern.

@Kodiologist
Copy link
Member Author

We should probably avoid trying to interpret shebang lines (that's the shell's job) or detect Hy syntax. I think that hy FILENAME will need an exception from the usual import rules, and perhaps it shouldn't use runpy.

@brandonwillard
Copy link
Member

All right, I'll start a PR for changes to that effect.

brandonwillard added a commit to brandonwillard/hy that referenced this issue Sep 5, 2018
Adds a Hy-parsing fallback to `runpy`, for files with unrecognized source
extensions.

Closes hylang#1677.
brandonwillard added a commit to brandonwillard/hy that referenced this issue Sep 5, 2018
This change a Hy-preferring `runhy` that is used by cmdline Hy.  Standard
`runpy` is still patched so that it can run `.hy` files, but the default
behaviour for unknown filetypes is preserved (i.e. assume they are Python
source).

Closes hylang#1677.
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

No branches or pull requests

3 participants