Modify `mtime` of the eln files after `make native-compile`

[Hirozy]

Hi,

TL; DR:

Modify mtime of the eln files to ensure that the eln can be loaded rather than recompiled by Emacs after make native-compile.

The long version:

Before loading the .eln file, Emacs will determine the mtime of the elc and eln files. If the eln was created earlier then elc, the eln file will be recompiled.

After upgrading DRONES, make lib/DRONE recompiled all el files to generate elc, and make native-compile recompiled the changed el files to generate eln. So for a DRONE like "magit" that contains multiple el files, all elc files will be updated, but not all eln files will be updated. The eln files are correctly, but when we use "magit" in Emacs, due to mtime, Emacs will decide that these eln files are out of data and it will recompile them.

So a better way is to modify the mtime of the eln files after make native-compile, which is what this PR is for.

[tarsius]

make native/DRONE should not have that problem.

[Hirozy]

Yes, make native/DRONE works fine. But when you use git submodule update --remote to update multiple drones, it is easy to update all elc and eln by make clean all; make native-compile (Instead of make lib/DRONE; make native/DRONE one by one), which would have the mtime problem. So I still think this PR makes sense.

[tarsius]

I am still unsure whether this is the right way to go, but let's first concentrate on some details:

• Whenever you would run make lib/DRONE; make native/DRONE run just make native/DRONE, that does byte- and native-compile.

• In addition to the stated goal, your commit also changes from add-hook finish; sit to while not-done sleep; finish. That isn't necessary to make your change work, is it? Please drop that additional change and if you think your way is better, then do it in a second commit and explain why that is so.

[tarsius]

Let's ignore native compilation for a moment. Why do I recommend make clean all? Just because foo.el did not change, that does not mean it does not have to be recompiled. It might use macros from foo-macros.el, which have changed. The only way to be sure that the byte-code is really up-to-date, is to recompile all files if any one of them changed.

I would expect the same is true for native compilation. So one could argue that make clean should also remove the *.eln files for all packages that Borg installed. Of course that would mean that make clean build-native and make clean all native-compile became really slow, all the time.

But what currently happens on my branch isn't really a compromise between that on one side and what you are suggesting on the other. On the contrary what my branch does is wrong from both perspectives. So I am inclined to go with your proposal, that implements one perspective properly. (What I am unsure about is whether it is the right one.)

Please explain the rational behind this change in more detail in the commit message.

[Hirozy]

• Whenever you would run make lib/DRONE; make native/DRONE run just make native/DRONE, that does byte- and native-compile.

make lib/DRONE; make native/DRONE or make native/DRONE is independent of make native-compile, and this PR only affects make native-compile.

• In addition to the stated goal, your commit also changes from add-hook finish; sit to while not-done sleep; finish. That isn't necessary to make your change work, is it? Please drop that additional change and if you think your way is better, then do it in a second commit and explain why that is so.

I think this way is better. After the asynchronous native compile is finished, two things need to be done, modify the eln files mtime and kill/exit emacs.

1. Add two hooks, one for modify mtime, and one for kill emacs.

   ;; last add-hook, first-out
   (add-hook 'native-comp-async-all-done-hook #'kill-emacs)  ;;do after mtime changed
   (add-hook 'native-comp-async-all-done-hook #'borg-set-eln-file-times)  ;; or lambda fun
   (sit-for 999999999 t)

2. Add lambda function as a hook.

   (add-hook 'native-comp-async-all-mode-hook (lambda ()
                                         ;; modify mtime
                                         ...
                                         (kill-emacs)))

3. The way in this PR. I think this approach would meet the same goal and be more readable.

If you have a better idea, please tell me, I will modify it according to your idea.

Let's ignore native compilation for a moment. Why do I recommend make clean all? Just because foo.el did not change, that does not mean it does not have to be recompiled. It might use macros from foo-macros.el, which have changed. The only way to be sure that the byte-code is really up-to-date, is to recompile all files if any one of them changed.

1. For Emac: Emacs determines whether to recompile an elc file based on the mtime of the elc and el files (see here).
2. For borg: make native-compile push all DROPS' el file to comp-files-queue, then call comp-run-async-workers to compile them. This means that whether a file in the queue need to be compiled or not is determined by emacs rather than borg. Based on the first point, whether this PR is accepted or not, by default, there is no way to avoid the problem that foo-macros.el has changed but foo.el has not. According to Emacs' current logic, it will only recompile foo-macros.el but not foo.el.

I would expect the same is true for native compilation. So one could argue that make clean should also remove the *.eln files for all packages that Borg installed. Of course that would mean that make clean build-native and make clean all native-compile became really slow, all the time.

But what currently happens on my branch isn't really a compromise between that on one side and what you are suggesting on the other. On the contrary what my branch does is wrong from both perspectives. So I am inclined to go with your proposal, that implements one perspective properly. (What I am unsure about is whether it is the right one.)

For emacs native comp, setq native-comp-always-compile t forces recompilation of all the el files in thecomp-files-queue. The old elc files also be deleted while compiling. The current version of borg does not read etc/borg/confog.el when call make native-compile, so it can be added in etc/borg/config.mk to make it work. make build-native and make lib/DRONE should not consider this option. Borg may not have to think clean eln files.

To summarize:

1. This PR avoids the problem of recompiling elc due to the mtime of elc and eln.
2. When executing thecomp-run-async-workers function, emacs will decide whether to recompile the el file to eln depending on the mtime (el and eln).
3. Whether or not mtime is modified, make native-compile does not avoid thefoo.el and foo-macros.el problems. I think this is a risk that borg users who enjoy make native-compile have to bear. set native-comp-always-compile t to force emacs to recompile the eln file (wait for a long time) or just recompile the changed el file (very soon).

If my description is not clear, please let me know and I will try to re-explain.