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

.pyc files don't get recreated when they use a macro from a file that was changed #1324

Closed
paultag opened this issue Jul 15, 2017 · 30 comments · Fixed by #2308
Closed

.pyc files don't get recreated when they use a macro from a file that was changed #1324

paultag opened this issue Jul 15, 2017 · 30 comments · Fixed by #2308

Comments

@paultag
Copy link
Member

paultag commented Jul 15, 2017

It seems like Hy is using the .pyc cache even if the file we're loading has a newer filesystem mtime than the .pyc file -- Python will default to overwriting the pyc.

This winds up biting people who update a script and wind up very confused about why something isn't being invoked :)

(Just bit me on the version on pypi - anyone know if this is being tracked with another issue?)

@gilch gilch added the bug label Jul 15, 2017
@Kodiologist
Copy link
Member

Kodiologist commented Jul 15, 2017

I can't reproduce this with Hy 0.13.0 and Python 3.6:

$ echo '(print "hello")' > foo.hy
$ hy foo.hy 
hello
$ hy foo.hy
hello
$ echo '(print "bye")' > foo.hy 
$ hy foo.hy                     
bye

@gilch
Copy link
Member

gilch commented Jul 15, 2017

I'd start looking here https://github.com/hylang/hy/blob/master/hy/importer.py#L57

@paultag, a reliable reproduction guide would be helpful. There was a recent automation of hyc #1094 #1269.

@paultag
Copy link
Member Author

paultag commented Jul 15, 2017

Weird. The above works for me too, on hy 0.13.0. The exact issue is pretty tied up to some code, but there's a few macros and imports happening all over the place. I'm not sure what's going on, exactly yet.

I can't get a minimal reproduction at the moment - I'm seeing weird stuff happening, but I did see this happen in the code, and removing the exact .pyc from the cache caused it to recompile the code and start working. I lost a few minutes on that.

Let me try to figure out why it's doing this

@paultag
Copy link
Member Author

paultag commented Jul 15, 2017

Oh crap I know why.

I have a macro in a file and I was only changing the macro. I bet that when you have a macro in one file, require it in another, and generate the file off the macro, it will cache the pyc, and modifying the original file won't cause anything using that macro to update.

@paultag
Copy link
Member Author

paultag commented Jul 15, 2017

Writing a reproduction now

@gilch
Copy link
Member

gilch commented Jul 15, 2017

Is this related to #1268?

@paultag
Copy link
Member Author

paultag commented Jul 15, 2017

Sorta, this would still be an issue if that was fixed

@paultag
Copy link
Member Author

paultag commented Jul 15, 2017

(projector) [paultag@nyx:~/x][05:36 PM] ♥  cat api.hy 
(defmacro yeah []
  (import mmm)
  (mmm.generate-function))
(yeah)
(projector) [paultag@nyx:~/x][05:36 PM] ♥  cat mmm.hy 
(defn generate-function [] `(defn yeah [] (print "hi")))
(projector) [paultag@nyx:~/x][05:36 PM] ♥  cat main.py 
import hy
import api
api.yeah()
(projector) [paultag@nyx:~/x][05:37 PM] ♥  python main.py 
hi
(projector) [paultag@nyx:~/x][05:37 PM] ♥  cat mmm.hy 
(defn generate-function [] `(defn yeah [] (print "hi!")))
(projector) [paultag@nyx:~/x][05:37 PM] ♥  python main.py 
hi
(projector) [paultag@nyx:~/x][05:37 PM] ♥  rm -rf __pycache__/
(projector) [paultag@nyx:~/x][05:37 PM] ♥  python main.py 
hi!

@paultag
Copy link
Member Author

paultag commented Jul 15, 2017

The issue is things we require may change the bytecode, and we have no way of knowing without going through requires and invalidating if the required file(s) have been updated after our bytecode.

I remain skeptical hacking that on top would work. I don't know that we should be writing .pyc files out by default -- maybe we need some sort of production-mode or compile-package helper and let admins do this explicitly?

@paultag
Copy link
Member Author

paultag commented Jul 15, 2017

I actually made this example evil by not using require - so that it proves the point about how hard this would be to detect

@paultag paultag changed the title .pyc files don't get recreated when a file has been updated .pyc files don't get recreated when they use a macro from a file that was changed Jul 15, 2017
@Kodiologist
Copy link
Member

Kodiologist commented Jul 15, 2017

The fact that macros are only run when a file is byte-compiled is a feature, not a bug. As you note, there's no way to check that a macro would expand to something different from last time without actually running it.

$ echo '(defmacro m [] (import random) (random.random)) (print (m))' > foo.hy
$ hy foo.hy 
0.9039111761737755
$ hy foo.hy
0.9039111761737755

So I'm inclined to regard this as a gotcha that the programmer needs to be aware of, not a bug in Hy itself.

@paultag
Copy link
Member Author

paultag commented Jul 15, 2017

I didn't know .pyc files were being generated - this is a huge gotcha, and I'm going to claim that we ought to not magically write out pyc without a human saying they should

@gilch
Copy link
Member

gilch commented Jul 15, 2017

I didn't know .pyc files were being generated

It should absolutely be documented. This was bigger change than we thought.

@Kodiologist is calling this a feature, not a bug.

I think macros are pointless without compilation. If you're interpreting only, you might as well use fexprs, right?

But how do Clojure and Common Lisp handle this kind of situation? We should compare those before deciding. As I recall, some CL implementations have both a compiler and interpreter, but sbcl, for example, is only a compiler.

@Kodiologist
Copy link
Member

I'm going to claim that we ought to not magically write out pyc without a human saying they should

As of f2278cf, Hy respects Python's environment variable PYTHONDONTWRITEBYTECODE. I wouldn't recommend having it the other way around, though, where the programmer has to opt into bytecode. That's forgoing a major speedup that you're going to want 99% of the time. People who really hate bytecode can always put alias hy='PYTHONDONTWRITEBYTECODE=1 hy' in their .bashrc.

But how do Clojure and Common Lisp handle this kind of situation?

That's a good question. I was always pretty mystified about how Common Lisp handles these issues.

@refi64
Copy link
Contributor

refi64 commented Jul 15, 2017

Can't Hy just track the hashes of both the main file and its requirements?

@refi64
Copy link
Contributor

refi64 commented Jul 15, 2017

IMO this is an incredibly bad bug that violates one of the core features of Hy...

@refi64
Copy link
Contributor

refi64 commented Jul 15, 2017

Also @paultag is alive :D

@Kodiologist
Copy link
Member

Can't Hy just track the hashes of both the main file and its requirements?

No; e.g., #1324 (comment). Similarly, a macro can choose a file to import randomly.

@gilch
Copy link
Member

gilch commented Jul 16, 2017

I wouldn't recommend having it the other way around, though, where the programmer has to opt into bytecode. That's forgoing a major speedup that you're going to want 99% of the time.

We made compilation automatic because Python does it that way. The speedup is considerable and compiling each file by hand seems like a pain. But there might be more nuanced options than global opt in or out. In any case, auto-compiling Hy itself (the libraries) seems like a good idea.

But how do Clojure and Common Lisp handle this kind of situation?

Common Lisp compiles forms rather than files. You can compile the contents of a file, but most implementations have no option do this at the command line. The standard way to do it is from the repl with the COMPILE-FILE form. We might want to implement similar functionality in Hy to use in our repl. This would make it easier to recompile things in emacs instead of the shell. You can explicitly recompile an individual file with hyc now. That is, instead of deleting __pycache__/, you could simply $ hyc api.hy.

Python also has a sys.dont_write_bytecode flag. In Python, you can use this to override the behavior for individual import statements. @Kodiologist did your fix support this flag in Hy too? It probably should.

This way we could turn off auto-compilation on a per-import basis while developing, without turning it off for everything else, and then turn it back on for production. Perhaps a context manager or modified import macro could make this easy to do. (This kind of flag would be a dynamic variable in Common Lisp.)

Clojure usually compiles at load time. But it also has an ahead-of-time (AOT) compiler for namespaces. Certain Java interop features require a namespace to be compiled AOT. This is also usually done at the repl using the clojure.core/compile form.

@paultag
Copy link
Member Author

paultag commented Jul 16, 2017

Correctness should always trump speed

@Kodiologist
Copy link
Member

@Kodiologist did your fix support this flag in Hy too?

No, it did not. The environment variable is what's checked.

@Kodiologist
Copy link
Member

I'm going to relabel this issue, since the compiler is working as designed, although the design is questionable.

@refi64
Copy link
Contributor

refi64 commented Jul 16, 2017

Correctness should always trump speed

This. This is Python, not C++. The developer expects full correctness first.

As you note, there's no way to check that a macro would expand to something different from last time without actually running it.

But if the required file changes, it's decently likely that the macro will have changed in some way. So just track the hashes of required files, as well as the hashes of the files that are required by the required files and so forth. This is what C build systems do with headers anyway.

IMO this is a pretty horrible gotcha...

@josephtoles
Copy link

josephtoles commented Sep 25, 2019

Would it be appropriate if I added this gotcha to the documentation somewhere? I'm a newbie to Hy and was bit by this one too. I thought the behavior was a bug until I found this issue.

@Kodiologist
Copy link
Member

Yes, the tutorial now mentions bytecode, but beyond that, I don't think automatic byte-compilation or its consequences are documented.

@Kodiologist
Copy link
Member

Update: they are now.

@gldlcks
Copy link

gldlcks commented Jan 21, 2021

I steped on this issue too but I found the answer in the log.

@Kodiologist did your fix support this flag in Hy too?

No, it did not. The environment variable is what's checked.

But I wrote and I ran:

; main.hy
(print "hello, Hy")
$ ls
main.hy
$ PYTHONDONTWRITEBYTECODE=1 hy main.hy
hello, Hy
$ ls
__pycache__  main.hy

I am really confused now. Why is it created?
After this I also tried by editing .bashrc but I got a same result.
I wrote a python program too.

# main.py
print('hello, Python')
$ env
...
PYTHONDONTWRITEBYTECODE=1
...
$ ls
main.hy  main.py
$ python main.py
hello, Python
$ ls
main.hy  main.py
$ hy main.hy
hello, Hy
$ ls
__pycache__  main.hy  main.py

FYI: In Windows 10, Hy works fine too.

$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=20.04
DISTRIB_CODENAME=focal
DISTRIB_DESCRIPTION="Ubuntu 20.04.1 LTS"
$ python --version
Python 3.8.5
$ hy --version
hy 0.19.0

The description below is what I tried but is not supported in Hy.

I wrote a python code to test to avoid creating __pycache__ and I ran:

# main.py
import sys
sys.dont_write_bytecode = True
$ python main.py
$ ls
main.py

But in Hy:

; main.hy
(import sys)

;(setv (. sys dont_write_bytecode) True)
;(setv sys.dont_write_bytecode True)
(pys "sys.dont_write_bytecode = True") ; I tried all of these 3 lines.
$ hy main.hy
$ ls
__pycache__  main.hy

@peaceamongworlds
Copy link
Contributor

Update: they are now.

Sorry if I'm missing something, but I don't see any reference to this issue in the docs. Could you point it out?

I agree with @Kodiologist - I don't think that this behavior is a bug. It's just a consequence of that fact that macros are fully expanded out at compile time.

Maybe we should include an additional flag for hy to regenerate all pyc files, apart from those in the hy package itself? That would make it somewhat convenient to deal with this issue.

@Kodiologist
Copy link
Member

The section "Macros" in tutorial.rst begins with an example of automatic byte-compilation and what that means for macro expansion. I didn't mean to imply that there's an example of the kind of gotcha this issue is about.

@scauligi
Copy link
Member

scauligi commented Mar 29, 2021

I agree that nominally this isn't a bug as long as you understand the compile-time vs run-time modes, but it is fairly unintuitive for new users (and even more experienced users -- I've already tripped over this on at least a couple occasions).

One possible solution (expanding on #1324 (comment)) is to have Hy write additional files to the __pycache__ that store compile-time dependency information. E.g., if app.hy requires macros.hy, then this dependency would be written to __pycache__/app.cpython-39.hyc.
Later, after a user edits macros.hy and then runs or imports app.hy, the Hy importer logic can check the dependencies, see that macros.hy has been updated, and re-compile app.hy before running it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

9 participants