| Message ID | cover.1700314926.git.liliana.prikler@gmail.com |
|---|---|
| Headers | show |
| Series | Think ahead when compiling | expand |
On 2023-11-18 14:42, Liliana Marie Prikler wrote: > Hi Guix, > > this series (hopefully) makes it so that everything we need to be natively > compiled in Emacs a) is natively compiled, and b) is found in the right > location. Please check that you no longer get gratuitous writes to your > local eln-cache when trying this out. > > Cheers > > Fixes: emacs-build-system … mismatching hashes <https://bugs.gnu.org/66864> > > Liliana Marie Prikler (2): > gnu: emacs: Build trampolines. > gnu: emacs: Don't hash file names in native compilation. > > gnu/local.mk | 1 + > gnu/packages/emacs.scm | 6 +- > .../emacs-native-comp-fix-filenames.patch | 93 +++++++++++++++++++ > 3 files changed, 99 insertions(+), 1 deletion(-) > create mode 100644 gnu/packages/patches/emacs-native-comp-fix-filenames.patch > > > base-commit: 60c97924e9519361494aaf0686e28eb831a42315 --8<---------------cut here---------------start------------->8--- $ rm -r ~/.config/emacs/eln-cache/29.1-09dbbf4e $ guix time-machine --branch=emacs-team -- shell emacs-pgtk emacs-magit --pure -E '.*GTK.*|.*XDG.*|.*DISPLAY.*' -- emacs -Q (require 'magit) C-x C-c $ ls ~/.config/emacs/eln-cache/29.1-09dbbf4e dash-6c86c585-82fea3ab.eln magit-log-58eeae71-b655dbfb.eln magit-section-6e195547-0461e8af.eln git-commit-927a864a-05a352a6.eln magit-margin-ec1cc1e9-b1c0e957.eln magit-sequence-40a35869-1b719931.eln magit-7056b331-38ef85d9.eln magit-merge-0070f654-7bc0ed2b.eln magit-stash-06fe9f6e-f2a708b65jGJrl.eln.tmp magit-autorevert-54dea36c-bf1f5ea3.eln magit-mode-5ad95198-31fbb927.eln magit-status-70fcb10a-f6654216.eln magit-base-cd520092-751fcac2.eln magit-notes-17dfe23c-ce57b283.eln magit-tag-e968dc8c-f7b42d89.eln magit-bisect-8f9f6b8f-6fbb9bc2.eln magit-process-3e9d760a-410e523c.eln magit-transient-1d163154-ba4b9651.eln magit-branch-b9c8386b-b362e334.eln magit-pull-1f116009-3bf3af7f.eln magit-wip-99682fc2-0371c052.eln magit-clone-28b35658-e3db2e06.eln magit-push-08e42ed4-b67bbe05.eln magit-worktree-8f50ba9f-7b120e5d.eln magit-commit-18780595-ac089f0e.eln magit-reflog-4106970e-69fc9edb.eln subr--trampoline-6d616b652d70726f63657373_make_process_0.eln magit-diff-278da2fe-8132fe46.eln magit-refs-1e67efee-ab4fef69.eln transient-29183598-be63c251.eln magit-fetch-5ba6406a-029daf61.eln magit-remote-55bccbe3-bb4a4595.eln with-editor-415da08e-ddc1b93b.eln magit-files-1ee9fbef-d629cbfc.eln magit-repos-a48553be-da9ca79c.eln magit-git-736caf3b-1398a0cc.eln magit-reset-902f52f7-709d5f56.eln --8<---------------cut here---------------end--------------->8---
Am Samstag, dem 18.11.2023 um 19:44 +0400 schrieb Andrew Tropin: > On 2023-11-18 14:42, Liliana Marie Prikler wrote: > > > Hi Guix, > > > > this series (hopefully) makes it so that everything we need to be > > natively compiled in Emacs a) is natively compiled, and b) is found > > in the right location. Please check that you no longer get > > gratuitous writes to your local eln-cache when trying this out. > > > > Cheers > > > > Fixes: emacs-build-system … mismatching hashes > > <https://bugs.gnu.org/66864> > > > > Liliana Marie Prikler (2): > > gnu: emacs: Build trampolines. > > gnu: emacs: Don't hash file names in native compilation. > > > > gnu/local.mk | 1 + > > gnu/packages/emacs.scm | 6 +- > > .../emacs-native-comp-fix-filenames.patch | 93 > > +++++++++++++++++++ > > 3 files changed, 99 insertions(+), 1 deletion(-) > > create mode 100644 gnu/packages/patches/emacs-native-comp-fix- > > filenames.patch > > > > > > base-commit: 60c97924e9519361494aaf0686e28eb831a42315 > > --8<---------------cut here---------------start------------->8--- > $ rm -r ~/.config/emacs/eln-cache/29.1-09dbbf4e > $ guix time-machine --branch=emacs-team -- shell emacs-pgtk emacs- > magit --pure -E '.*GTK.*|.*XDG.*|.*DISPLAY.*' -- emacs -Q > (require 'magit) > C-x C-c > $ ls ~/.config/emacs/eln-cache/29.1-09dbbf4e > dash-6c86c585-82fea3ab.eln magit-log-58eeae71- > b655dbfb.eln magit-section-6e195547-0461e8af.eln > git-commit-927a864a-05a352a6.eln magit-margin-ec1cc1e9- > b1c0e957.eln magit-sequence-40a35869-1b719931.eln > magit-7056b331-38ef85d9.eln magit-merge-0070f654- > 7bc0ed2b.eln magit-stash-06fe9f6e-f2a708b65jGJrl.eln.tmp > magit-autorevert-54dea36c-bf1f5ea3.eln magit-mode-5ad95198- > 31fbb927.eln magit-status-70fcb10a-f6654216.eln > magit-base-cd520092-751fcac2.eln magit-notes-17dfe23c- > ce57b283.eln magit-tag-e968dc8c-f7b42d89.eln > magit-bisect-8f9f6b8f-6fbb9bc2.eln magit-process-3e9d760a- > 410e523c.eln magit-transient-1d163154-ba4b9651.eln > magit-branch-b9c8386b-b362e334.eln magit-pull-1f116009- > 3bf3af7f.eln magit-wip-99682fc2-0371c052.eln > magit-clone-28b35658-e3db2e06.eln magit-push-08e42ed4- > b67bbe05.eln magit-worktree-8f50ba9f-7b120e5d.eln > magit-commit-18780595-ac089f0e.eln magit-reflog-4106970e- > 69fc9edb.eln subr--trampoline- > 6d616b652d70726f63657373_make_process_0.eln > magit-diff-278da2fe-8132fe46.eln magit-refs-1e67efee- > ab4fef69.eln transient-29183598-be63c251.eln > magit-fetch-5ba6406a-029daf61.eln magit-remote-55bccbe3- > bb4a4595.eln with-editor-415da08e-ddc1b93b.eln > magit-files-1ee9fbef-d629cbfc.eln magit-repos-a48553be- > da9ca79c.eln > magit-git-736caf3b-1398a0cc.eln magit-reset-902f52f7- > 709d5f56.eln > --8<---------------cut here---------------end--------------->8--- Oof.
2023-11-18 14:42 liliana.prikler@gmail.com: > Hi Guix, Hello and thank you for your efforts on this issue. > Liliana Marie Prikler (2): > gnu: emacs: Build trampolines. > gnu: emacs: Don't hash file names in native compilation. It seems like this is a series of two commits but I only received one, namely the second one ("2/2"). Is it just me or did the first commit went missing for everyone? Also, are these commits still on emacs-team branch? Andrew's command (guix time-machine --branch=emacs-team -- shell ...) made me assume that these commits were pushed to emacs-team branch, but I can't find them there. I'd be glad to try this patch-series out - at some point, when I have more time.
Am Mittwoch, dem 22.11.2023 um 12:09 +0000 schrieb Mekeor Melire: > 2023-11-18 14:42 liliana.prikler@gmail.com: > > > Hi Guix, > > Hello and thank you for your efforts on this issue. > > > Liliana Marie Prikler (2): > > gnu: emacs: Build trampolines. > > gnu: emacs: Don't hash file names in native compilation. > > It seems like this is a series of two commits but I only received > one, namely the second one ("2/2"). Is it just me or did the first > commit went missing for everyone? > > Also, are these commits still on emacs-team branch? Andrew's command > (guix time-machine --branch=emacs-team -- shell ...) made me assume > that these commits were pushed to emacs-team branch, but I can't find > them there. > > I'd be glad to try this patch-series out - at some point, when I have > more time. These are commits submitted to the emacs-branch – they are not upstreamed yet. I'll have a v2 up hopefully soon; things sadly aren't as smooth as I'd assumed. Cheers
Reporting some observations for the benefit of the community. I managed to install the following sequence of patches on emacs-team commit 60c97924e9 : - v3 1/3 - v3 2/3 - v3 3/3 - v4 4/5 - v4 5/5 I was able to build emacs, emacs-org and a few additional packages using the options "--no-grafts --with-input=emacs-minimal=emacs". When building "emacs-org-contrib" the additional option "--without-tests=emacs-clojure-mode" was needed as well. Having done so, upon starting emacs via guix-shell, I noticed a few messages in *Async-native-compile-log* which noted that libraries such as rx.el.gz and cl-macs.el.gz were being compiled. I observed native-compiled versions of these show up in ~/.emacs.d/eln-cache. Now this seems similar to the behaviour I observe in a non-Guix host where Emacs 29.1 is installed via the OS. There too my ~/.emacs.d/eln-cache has a number of entries that I was a little surprised to see such as those corresponding to rx and cl-macs. One difference, however, is in the names and the relative locations of these natively-compiled files. Outside of the guix shell (i.e., using the OS's native Emacs package) the eln files are named like ~/.emacs.d/eln-cache/29.1-115521d4/cl-macs-7ae82f81-e626a10e.eln whereas within the Guix shell using Emacs built from the installed patches you get names like ~/.emacs.d/eln-cache/29.1-c187d49d/emacs-lisp/cl-macs.eln. Note the sub-directory within the 29.1-* directory present within guix-shell container, as well as the lack of hash fingerprint in the *.eln file itself. I do not know if these differences are to be expected, or whether or not they're material in any way. Thoughts? Can this patch series be installed into emacs-team as-is? If not, what are the current blockers, if any? On a somewhat related note, when is the next merge from emacs-team to master expected to be?
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > * gnu/packages/patches/emacs-disable-jit-compilation.patch: New file. > * gnu/local.mk (dist_patch_DATA): Register it here. > * gnu/packages/emacs.scm (emacs-minimal)[patches]: Use it here. > --- > gnu/local.mk | 1 + > gnu/packages/emacs.scm | 3 ++- > .../emacs-disable-jit-compilation.patch | 19 +++++++++++++++++++ > 3 files changed, 22 insertions(+), 1 deletion(-) > create mode 100644 gnu/packages/patches/emacs-disable-jit-compilation.patch I don't believe this patch is sufficient. It seems that the variable 'native-comp-eln-load-path may also need to be tweaked. The path to the "native-lisp" directory needs to be specified absolutely to make it not be dependent on the invocation-directory. Details below. #+begin_src bash :results replace emacs --batch --eval "(message \"%s\" (car (split-string (describe-function 'describe-function) \"\n\")))" #+end_src #+RESULTS: : Type q in help window to delete it : describe-function is an autoloaded interactive byte-compiled Lisp #+begin_src bash :results replace type emacs ls ~/.guix-profile/lib/emacs/29.2/native-lisp/29.2-e9db68ab/help-fns.eln ls ~/.guix-profile/lib/emacs/29.2/native-lisp/29.2-e9db68ab/*.eln | wc -l ls ~/.guix-profile/lib/emacs/native-site-lisp/29.2-e9db68ab/*.eln | wc -l emacs --batch --eval "(message \"%s\" native-comp-eln-load-path)" #+end_src #+RESULTS: : emacs is hashed (/home/user/.guix-profile/bin/emacs) : /home/user/.guix-profile/lib/emacs/29.2/native-lisp/29.2-e9db68ab/help-fns.eln : 1715 : 236 : (/home/user/.emacs.d/eln-cache/ /home/user/.guix-profile/lib/emacs/native-site-lisp ../native-lisp/) #+begin_src bash :results replace emacs --batch --eval "(message \"%s\" (progn (add-to-list 'native-comp-eln-load-path \"/home/user/.guix-profile/lib/emacs/29.2/native-lisp\") (car (split-string (describe-function 'describe-function) \"\n\"))))" #+end_src #+RESULTS: : Type q in help window to delete it : describe-function is an autoloaded interactive native-compiled Lisp
Suhail via Guix-patches via <guix-patches@gnu.org> writes: > I don't believe this patch is sufficient. It seems that the variable > 'native-comp-eln-load-path may also need to be tweaked. The path to the > "native-lisp" directory needs to be specified absolutely to make it not > be dependent on the invocation-directory. Or it's possible that a patch that ensures that 'native-comp-eln-load-path contains the absolute value of the "native-lisp" directory obviates this one (6/6) in its entirety.
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: >> Suhail via Guix-patches via <guix-patches@gnu.org> writes: >> >> > I don't believe this patch is sufficient. It seems that the >> > variable 'native-comp-eln-load-path may also need to be tweaked. >> > The path to the "native-lisp" directory needs to be specified >> > absolutely to make it not be dependent on the invocation-directory. >> > ... I've managed to resolve it I can confirm that Emacs built by installing v6 patches on emacs-team branch and built with --no-grafts is now able to locate natively-compiled versions of features like help-fns etc. (which it couldn't with v5 patches). #+begin_src bash :results replace emacs --batch --eval "(message \"%s\" (car (split-string (describe-function 'describe-function) \"\n\")))" #+end_src #+RESULTS: : Type q in help window to delete it : describe-function is an autoloaded interactive native-compiled Lisp However, there may be a spurious entry in native-comp-eln-load-path that should probably be removed. #+begin_src bash :results replace emacs --batch --eval "(message \"%s\" native-comp-eln-load-path)" #+end_src #+RESULTS: : (/home/user/.emacs.d/eln-cache/ /home/user/.guix-profile/lib/emacs/native-site-lisp /gnu/store/4jvap9wxifizm56p6pmxc6rk0zyvm5zc-emacs-29.2/lib/emacs/29.2/native-lisp ../native-lisp/) The "../native-lisp/" entry above, seems out of place. Based on the documentation of native-comp-eln-load-path: > If the name of a directory in this list is not absolute, it is assumed > to be relative to invocation-directory. Contrasting the above with an Emacs 29.1 installation on a non-Guix system, I notice that the latter doesn't have any non-absolute paths in native-comp-eln-load-path. Thoughts?
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: >> Contrasting the above with an Emacs 29.1 installation on a non-Guix >> system, I notice that the latter doesn't have any non-absolute paths >> in native-comp-eln-load-path. Thoughts? > I have noticed this myself, but I don't think that it's related to this > series. It seems to be. It's possible that it may also be related to other things already in the emacs-team branch (e.g. if the patch series is only responsible for uncovering a pre-existing defect as opposed to causing it), but the relative path only shows up during the series. > Can you try 29.2 without the patches? emacs-team branch at aae61f54ff doesn't have the relative path in native-comp-eln-load-path. The relative path shows up only after [PATCH v6 3/7] is applied. Hope this helps and hopefully the patch series can soon be merged into master for general availability. [PATCH v6 3/7] gnu: emacs: Don't hash file names in native compilation.
Suhail via Guix-patches via <guix-patches@gnu.org> writes: > hopefully the patch series can soon be merged into master for general > availability. Btw, does every package require something like [PATCH v6 5/7] or [PATCH v6 6/7] in order for it to be natively compiled?
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: >> Btw, does every package require something like [PATCH v6 5/7] or >> [PATCH v6 6/7] in order for it to be natively compiled? > No, these patches are only for natively compiled packages that don't > use emacs-build-system. I.e., any and all packages using the emacs-build-system should be natively-compiled? If so, I believe I've found a counter-example. After installing v6 series and building emacs-htmlize my expectation is that functions provided by it, such as htmlize-buffer, would be natively-compiled. However, I get: > htmlize-buffer is an autoloaded interactive Lisp function in > `htmlize.el'. Note the lack of "native-compiled" in the above.
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > If you compile with emacs-minimal, that's the expected results. It was compiled with emacs-minimal=emacs. For reference, I have inlined the manifest file that was used (with the --no-grafts CLI option to guix-package) towards the end of this email. I'm sharing the one that was actually used, but it could be minimized. I believe there are two issues (not including the spurious entry in native-comp-eln-load-path which may already have been resolved in v7 of the series). All my observations below are based on v6 patch series. For emacs-htmlize, I believe the issue may stem from the fact that some .eln files are available from entries in native-comp-eln-load-path only via symlinks and the .eln loader may not be able to load symlinks. Packages such as ox-html are available from two locations in the native-comp-eln-load-path. In one location they are symlinked, but (presumably because org is a builtin package) it is also available from another location in the native-comp-eln-load-path and in the latter location it's not symlinked in. I believe this difference is why for some packages natively-compiled versions are loaded (e.g. org, ox-html etc) whereas for others it's not the case. In addition, I believe there's another issue. Some packages' names are getting truncated. For instance, instead of uniquify.eln, I observe niquify.eln. In this case, the .eln isn't symlinked (presumably because it's a builtin), but due to the name being messed up (perhaps too aggressive a truncation of hashes?) the natively compiled version is not available: > uniquify-item-p is a byte-compiled Lisp function in `uniquify.el'. The manifest file that was used is below. #+begin_src scheme (use-modules (guix transformations) (gnu packages)) (define transform1 (options->transformation '((with-input . "emacs-minimal=emacs") (without-tests . "emacs-clojure-mode")))) (packages->manifest (list (transform1 (specification->package "bash")) (transform1 (specification->package "coreutils")) (transform1 (specification->package "git")) (transform1 (specification->package "make")) (transform1 (specification->package "emacs")) (transform1 (specification->package "emacs-org")) (transform1 (specification->package "emacs-org-contrib")) (transform1 (specification->package "emacs-citeproc-el")) (transform1 (specification->package "emacs-engrave-faces")) (transform1 (specification->package "emacs-haskell-mode")) (transform1 (specification->package "emacs-htmlize")) (transform1 (specification->package "emacs-markdown-mode")) (transform1 (specification->package "emacs-org-pandoc-import")) (transform1 (specification->package "pandoc")) )) #+end_src Thank you for your continued work on this!
Suhail via Guix-patches via <guix-patches@gnu.org> writes: > I believe there are two issues (not including the spurious entry in > native-comp-eln-load-path which may already have been resolved in v7 of > the series). All my observations below are based on v6 patch series. I can confirm that the v7 series fixes the spurious entry in native-comp-eln-load-path. However, the 2 issues pointed out in the previous email remain: 1. failure to load natively-compiled version of non-builtin libraries like emacs-htmlize possibly due to symlinks, and 2. failure to load natively-compiled version of some builtin libraries like uniquify due to incorrect naming of .eln files
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > Would you care to debug this a bit more and report your findings? The locations in question are in the store and hence write-protected. What's a "safe" way to debug this? >> Packages such as ox-html are available from two locations in the >> native-comp-eln-load-path. In one location they are symlinked, but >> (presumably because org is a builtin package) it is also available >> from another location in the native-comp-eln-load-path and in the >> latter location it's not symlinked in. I believe this difference is >> why for some packages natively-compiled versions are loaded (e.g. >> org, ox-html, etc) whereas for others it's not the case. > Well, as pointed out in the deleted code, dlopen has a "one shared > library per file name" limitation. I don't think we're hit by that > thanks to unique hashes in the store directory, but I might be wrong > about that. Maybe we have to do java-style FQDNs instead. Perhaps. I'd wondered, more simply, if symlinked .eln files were ever being loaded. In the limited testing I did, I didn't find such an instance. For what it's worth, only one of the entries in native-comp-eln-load-path seems to have symlinked entries (namely, ~/.guix-profile/lib/emacs/native-site-lisp). >> In addition, I believe there's another issue. Some packages' names >> are getting truncated. For instance, instead of uniquify.eln, I >> observe niquify.eln. In this case, the .eln isn't symlinked >> (presumably because it's a builtin), but due to the name being messed >> up (perhaps too aggressive a truncation of hashes?) the natively >> compiled version is not available: > I am not truncating any hashes, I'm not even computing them in the > first place. The functions I'm modifying are publicly callable, namely > comp-el-to-eln-rel-filename for the relative file names, and > comp-el-to-eln-filename for the absolute ones. > > There could be an off-by-one error hidden in the stripping of the > BOGUS_DIRS, however. Let's investigate that. Were you thinking out loud, or are there specific steps you'd like me to help with? > Note, that you can map transform1, saving some typing overhead, and > since you are transforming all of your packages you could compose that > with specification->package. Appreciate the tips, but I should clarify that the manifest file in question was generated by way of guix shell --export-manifest. I'd omitted the header.
Suhail via Guix-patches via <guix-patches@gnu.org> writes: > However, the 2 issues pointed out in the previous email remain: Correction, there's only 1 issue. The first issue was a false positive. Apologies for the noise. > 1. failure to load natively-compiled version of non-builtin libraries > like emacs-htmlize possibly due to symlinks, and This was a false positive. Turns out, if the function in question is only autoloaded describe-function won't correctly identify the function as being natively-compiled. This is consistent with what happens on Emacs 29.1 on a non-Guix system. Not an issue. > 2. failure to load natively-compiled version of some builtin libraries > like uniquify due to incorrect naming of .eln files This issue remains as of v7 of the patch series.
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: >> > 2. failure to load natively-compiled version of some builtin >> > libraries like uniquify due to incorrect naming of .eln files >> >> This issue remains as of v7 of the patch series. > I fixed this one locally, I can confirm that the .eln filenames are no longer truncated (as far as I can tell). > but ‘uniquify’ remains byte-compiled for no explainable reason. And too that as of v8 patch, uniquify still isn't natively-compiled. However, if you unload it and then reload it, it becomes natively-compiled. #+begin_src bash emacs --batch --eval "(message \"%s\" (progn (unload-feature 'uniquify) (require 'uniquify) (take 1 (split-string (substring-no-properties (describe-function 'uniquify-item-p)) \"\\n\" t))))" #+end_src #+RESULTS: : Type q in help window to delete it : (uniquify-item-p is a native-compiled Lisp function in `uniquify.el'.) I think the issue is because uniquify (along with some others such as prog-mode, backquote etc that I tested) are preloaded in Emacs. I believe that these preloaded packages need to be treated specially. In a non-Guix system, these preloaded packages have their .eln files stored under something like <native-comp-eln-load-path-entry>/29.x-<hash>/preloaded/ , whereas in the v8 series there is no "preloaded" directory. Instead .eln files for packages such as uniquify, prog-mode, backquote etc. are in various locations: - uniquify :: <n-c-e-l-p-entry>/29.2-<hash>/uniquify.eln - prog-mode :: <n-c-e-l-p-entry>/29.2-<hash>/progmodes/prog-mode.eln - backquote :: <n-c-e-l-p-entry>/29.2-<hash>/emacs-lisp/backquote.eln I suspect that the code which does the preloading may need to be patched and/or the preloaded packages may need to have their .eln files generated under a preloaded sub-directory (similar to what happens in non-Guix systems). Below is a list of .eln files that exist in the preloaded sub-directory in Emacs 29.1 on a non-Guix system. It's possible that the set of preloaded packages on Emacs 29.2 is different. I suspect that all of these packages are similarly affected. #+begin_example abbrev.eln backquote.eln bindings.eln buff-menu.eln burmese.eln button.eln byte-run.eln case-table.eln cconv.eln cham.eln characters.eln chinese.eln cl-generic.eln cl-preloaded.eln common-win.eln composite.eln cp51932.eln cus-face.eln cus-start.eln custom.eln cyrillic.eln czech.eln debug-early.eln disp-table.eln dnd.eln dos-fns.eln dos-vars.eln dos-w32.eln dynamic-setting.eln easymenu.eln ediff-hook.eln eldoc.eln electric.eln elisp-mode.eln english.eln env.eln epa-hook.eln ethiopic.eln eucjp-ms.eln european.eln faces.eln files.eln fill.eln float-sup.eln font-core.eln font-lock.eln fontset.eln format.eln frame.eln fringe.eln georgian.eln greek.eln haiku-win.eln hebrew.eln help.eln image.eln indent.eln indian.eln indonesian.eln internal.eln isearch.eln iso-transl.eln japanese.eln jit-lock.eln jka-cmpr-hook.eln keymap.eln khmer.eln korean.eln lao.eln lisp.eln lisp-mode.eln ls-lisp.eln macroexp.eln map-ynp.eln menu-bar.eln minibuffer.eln misc-lang.eln mouse.eln mule.eln mule-cmds.eln mule-conf.eln mule-util.eln mwheel.eln nadvice.eln newcomment.eln ns-win.eln obarray.eln oclosure.eln page.eln paragraphs.eln paren.eln pc-win.eln pgtk-dnd.eln pgtk-win.eln philippine.eln prog-mode.eln regexp-opt.eln register.eln replace.eln rfn-eshadow.eln rmc.eln romanian.eln scroll-bar.eln select.eln seq.eln shorthands.eln simple.eln sinhala.eln slovak.eln startup.eln subr.eln syntax.eln tab-bar.eln tabulated-list.eln tai-viet.eln text-mode.eln thai.eln tibetan.eln timer.eln tool-bar.eln tooltip.eln tty-colors.eln ucs-normalize.eln uniquify.eln utf-8-lang.eln vc-hooks.eln version.eln vietnamese.eln w32-fns.eln w32-vars.eln w32-win.eln widget.eln window.eln x-dnd.eln x-win.eln #+end_example
Suhail via Guix-patches via <guix-patches@gnu.org> writes: > Below is a list of .eln files that exist in the preloaded > sub-directory in Emacs 29.1 on a non-Guix system. Minor correction. The list of files I posted was /adapted/ from the list of files in the preloaded sub-directory on a non-Guix system (with Emacs 29.1). Specifically, the list I shared had the hash suffix removed to make comparison with the .eln file on Guix easier.
I'm assuming you're able to reproduce the issue on your end as well with other features like prog-mode, backquote etc. Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > I instrumented lread.c to print the relative and absolute file names > tried, and it appears that these aren't loaded at all after install > while EMACSNATIVELOADPATH is ignored when dumping. Interesting. I did a grep of the Emacs source (branch emacs-29) and line 1770 of src/lread.c (in maybe_swap_for_eln) seemed relevant: > /* Look also in preloaded subfolder of the last entry in > `comp-eln-load-path'. */ What surprised me was that not only was the "preloaded" directory special-cased, but so too was the last entry of "comp-eln-load-path" (which I imagine is referring to native-comp-eln-load-path). Judging by your message, I'm guessing you're well aware of this. I, unfortunately, have no additional insights or suggestions. However, given that ensuring Emacs is able to find the natively-compiled version of files seems non-trivial, it might help for a future patch series to be accompanied with tests. Thoughts?
The latest patch series (titled v7, but really v9) shows some improvement, however, there are still some issues. Specifically, functions from preloaded library uniquify are now correctly noted as being natively-compiled. The functions that were confirmed were uniquify-item-base and uniquify-item-p. However, functions from certain other preloaded libraries are still noted as being byte-compiled instead. At the very least this set includes backquote and eldoc. #+begin_src sh emacs --batch --eval (message "%s" (take 1 (split-string (substring-no-properties (describe-function 'backquote-process)) "\n" t))) #+end_src #+RESULTS: : Type q in help window to delete it : (backquote-process is a byte-compiled Lisp function in `backquote.el'.) #+begin_src sh emacs --batch --eval (message "%s" (take 1 (split-string (substring-no-properties (describe-function 'eldoc-mode)) "\n" t))) #+end_src #+RESULTS: : Type q in help window to delete it : (eldoc-mode is an interactive byte-compiled Lisp function in)
So I wrote some code to make it easier to report on the state of
native-comp for some of the packages previously mentioned (the *.eln
files for these packages reside under the "preloaded" directory in a
non-Guix distribution). I'm including below both the contents of the
script as well as the results.
First the results. Assuming that the file
test-native-comp-p--preloaded.el exists, evaluating the below:
#+begin_src sh :results replace
emacs -Q --batch \
-l path/to/test-native-comp-p--preloaded.el \
--eval "(princ (report/compilation-status/run))"
#+end_src
With a non-Guix Emacs results in:
#+RESULTS:
: [97/97] passed. Success!
Whereas with the latest patch-series it results in 41 failing cases:
#+RESULTS:
#+begin_example
[56/97] passed. 41 failing cases below:
((backquote-process . byte-compiled)
(byte-run-strip-symbol-positions . byte-compiled)
(cconv-convert . byte-compiled)
(use-default-char-width-table . byte-compiled)
(cl-generic-p . byte-compiled)
(cl-struct-define . byte-compiled)
(x-setup-function-keys . byte-compiled)
(debug-early . byte-compiled)
(easy-menu-item-present-p . byte-compiled)
(eldoc-mode . byte-compiled)
(elisp-mode-syntax-propertize . byte-compiled)
(fill-region . byte-compiled)
(fontset-plain-name . byte-compiled)
(indian-compose-regexp . byte-compiled)
(msdos-setup-keyboard . byte-compiled)
(iso-transl-set-language . byte-compiled)
(forward-sexp . byte-compiled)
(lisp-string-in-doc-position-p . byte-compiled)
(macroexp-compiling-p . byte-compiled)
(map-y-or-n-p . byte-compiled)
(egyptian-shape-grouping . byte-compiled)
(convert-define-charset-argument . byte-compiled)
(coding-system-change-eol-conversion . byte-compiled)
(store-substring . byte-compiled)
(advice-function-mapc . byte-compiled)
(oclosure-type . byte-compiled)
(forward-page . byte-compiled)
(sentence-end . byte-compiled)
(prog-context-menu . byte-compiled)
(regexp-opt . byte-compiled)
(read-multiple-choice . byte-compiled)
(seq-first . byte-compiled)
(hack-read-symbol-shorthands . byte-compiled)
(syntax-propertize-multiline . byte-compiled)
(tabulated-list-put-tag . byte-compiled)
(text-mode . byte-compiled)
(timer-activate . byte-compiled)
(tty-color-desc . byte-compiled)
(ucs-normalize-hfs-nfd-comp-p . byte-compiled)
(vc-mode . byte-compiled)
(x-handle-no-bitmap-icon . byte-compiled))
#+end_example
The content of test-native-comp-p--preloaded.el are as follows:
#+begin_src elisp :eval never
(require 'help-fns)
(defun report/compilation-status (fun &optional feature)
"Report on the compilation status of function FUN.
Optionally load FEATURE before reporting on compilation status."
(when feature
(require feature))
(let ((descstr (substring-no-properties
(with-output-to-string
(help-fns-function-description-header fun)))))
(cons fun
(cond
((string-search " native-compiled" descstr) 'native-compiled)
((string-search " byte-compiled" descstr) 'byte-compiled)
(t descstr)))))
;; [[/usr/share/emacs/29.2/lisp]]
(defvar report/compilation-status/cases/preloaded nil
"Functions that ought to be natively-compiled.")
(setq report/compilation-status/cases/preloaded
'((abbrev-mode)
(backquote-process)
(mode-line-widen)
(buffer-menu)
;; burmese
(button-mode)
(byte-run-strip-symbol-positions)
(case-table-get-table)
(cconv-convert)
;; cham
(use-default-char-width-table)
;; chinese
(cl-generic-p)
(cl-struct-define)
(x-setup-function-keys)
(encode-composition-rule)
;; cp51932
(custom-declare-face)
(minibuffer-prompt-properties--setter) ;; cus-start.el
(custom-add-choice)
;; cyrillic
;; czech
(debug-early)
(display-table-slot disp-table) ;; disp-table.eln exists
(dnd-open-file)
(dos-mode25 dos-fns) ;; dos-fns.eln exists
;; dos-vars
(find-file-text dos-w32) ;; dos-w32.eln exists
(dynamic-setting-handle-config-changed-event)
(easy-menu-item-present-p)
;; ediff-hook
(eldoc-mode)
(electric-indent-mode)
(elisp-mode-syntax-propertize)
;; english
(getenv)
(epa-file-find-file-hook)
;; ethiopic
;; eucjp-ms
;; european
(face-list)
(find-file-noselect)
(fill-region)
;; float-sup
(font-lock-change-mode)
(font-lock-add-keywords)
(fontset-plain-name)
(format-read)
(frame-edges)
(fringe-mode)
;; georgian
;; greek
;; haiku-win
;; hebrew
(help-quick)
(image-type)
(indent-region)
(indian-compose-regexp)
;; indonesian
(msdos-setup-keyboard term/internal) ;; internal.eln exists
(isearch-abort)
(iso-transl-set-language)
;; japanese
(jit-lock-mode)
(jka-compr-build-file-regexp)
(keymap-global-set)
;; khmer
;; korean
;; lao
(forward-sexp)
(lisp-string-in-doc-position-p)
(ls-lisp-set-options ls-lisp) ;; ls-lisp.eln exists
(macroexp-compiling-p)
(map-y-or-n-p)
(menu-find-file-existing)
(completion-boundaries)
(egyptian-shape-grouping)
(mouse-double-click-time)
(convert-define-charset-argument)
(coding-system-change-eol-conversion)
;; mule-conf.eln
(store-substring mule-util) ;; mule-util.eln exists
(mouse-wheel-change-button)
(advice-function-mapc)
(comment-string-strip)
;; (ns-handle-nxopen term/ns-win)
(obarray-make)
(oclosure-type)
(forward-page)
(sentence-end)
(show-paren-function)
;; (msdos-face-setup term/pc-win)
(pgtk-dnd-init-frame pgtk-dnd) ;; pgtk-dnd.eln exists
;; (pgtk-drag-n-drop term/pgtk-win)
;; philippine
(prog-context-menu)
(regexp-opt)
(get-register)
(query-replace-descr)
(rfn-eshadow-setup-minibuffer)
(read-multiple-choice)
;; romanian
(scroll-bar-scale)
(gui-select-text)
(seq-first)
(hack-read-symbol-shorthands)
(next-error-find-buffer)
;; sinhala
;; slovak
(exit-splash-screen)
(buffer-local-boundp)
(syntax-propertize-multiline)
(tab-bar-mode)
(tabulated-list-put-tag)
;; tai-viet
(text-mode)
;; thai
;; tibetan
(timer-activate)
(tool-bar-mode)
(tooltip-mode)
(tty-color-desc)
(ucs-normalize-hfs-nfd-comp-p ucs-normalize) ;; ucs-normalize.eln exists
(uniquify-item-p)
;; utf-8-lang.eln
(vc-mode)
(emacs-version)
;; vietnamese
;; (w32-shell-name)
;; w32-vars.eln
;; (w32-handle-dropped-file 'term/w32-win)
(define-widget)
(window-right)
(x-dnd-init-frame)
(x-handle-no-bitmap-icon)))
(defun report/compilation-status/run ()
"Run all cases and report those that aren't native-compiled."
(let* ((results (mapcar (lambda (args) (apply #'report/compilation-status args))
report/compilation-status/cases/preloaded))
(failing (seq-filter (lambda (x) (not (eq (cdr x) 'native-compiled)))
results))
(numtotal (seq-length results))
(numfailing (seq-length failing))
(numpassing (- numtotal numfailing)))
(concat (format "[%s/%s] passed."
numpassing numtotal)
(if failing
(concat (format " %s failing cases below:\n\n" numfailing)
(pp-to-string failing))
" Success!"))))
#+end_src
Hope this helps!
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > With this series I get 94/97. I was able to confirm that as well. > The remaining three Namely, the 'ucs-normalize, 'mule-util, and 'term/internal features. For each of these the .eln files exist, but the version that gets loaded in Guix is the byte-compiled one, unless some extra steps are taken. > ... don't appear to be preloaded through the loadup script, so I have > no idea how to catch them or whether it's even worth doing so. I dug into this a little deeper. It seems that by adjusting the load-path one is able to load the natively-compiled variants instead of the byte-compiled for these features. Oddly, this adjustment doesn't seem to be needed in non-Guix Emacs which may suggest the presence of a bug. I'm hoping you have some insight that explains this difference in behaviour. After modifying the previous test-native-comp-p--preloaded.el (in hindsight, the preloaded part may be a misnomer) with the below patch I got all the 97 cases passing. #+begin_src diff diff -u ./test-native-comp-p--preloaded.el ./test-native-comp-p--preloaded.el --- ./test-native-comp-p--preloaded.el 2024-02-17 08:49:36.294930488 -0500 +++ ./test-native-comp-p--preloaded.el 2024-02-17 08:38:45.102839041 -0500 @@ -155,8 +155,50 @@ (x-dnd-init-frame) (x-handle-no-bitmap-icon))) +(defun hack/tweak-load-path-in-guix () + "Tweak `load-path' to allow natively compiled versions to be loaded. + +If we ensure that the `load-path' entries for `term/internal', +`mule-util' and `ucs-normalize' exist and they occur before the +share/emacs/<emacs-version>/lisp entry then we are able to load +the natively compiled versions of these libraries. Of those +three, the entry for `term/internal' is missing whereas the +others currently occur after the share/emacs/<emacs-version>/lisp +entry. We remedy that here. + +Notably, in non-Guix Emacs this isn't needed. I.e., the fact +that the share/emacs/<emacs-version>/lisp entry precedes the +entries for `mule-util' and `ucs-normalize' is okay as is the +fact that the entry for `term/internal' is missing." + (when (getenv "GUIX_ENVIRONMENT") + (require 'find-func) + ;; first we'll add the missing entry for `term/internal' + (add-to-list 'load-path + (directory-file-name + (file-name-directory + (find-library-name (symbol-name 'term/internal)))) + ;; NOTE: we don't need to append; doing so simply to confirm that + ;; it's only the relative position wrt the + ;; share/emacs/<emacs-version>/lisp entry that matters. + t) + ;; then we'll ensure that the entry for share/emacs/<emacs-version>/lisp is at + ;; the end + (let* ((sitelisppath (format "/share/emacs/%s/lisp" + emacs-version)) + (pathsuffix (seq-filter + #'(lambda (x) + (string-suffix-p sitelisppath x)) + load-path)) + (pathprefix (seq-filter + #'(lambda (x) + (not + (string-suffix-p sitelisppath x))) + load-path))) + (setq load-path (append pathprefix pathsuffix))))) + (defun report/compilation-status/run () "Run all cases and report those that aren't native-compiled." + (hack/tweak-load-path-in-guix) (let* ((results (mapcar (lambda (args) (apply #'report/compilation-status args)) report/compilation-status/cases/preloaded)) (failing (seq-filter (lambda (x) (not (eq (cdr x) 'native-compiled))) @@ -169,5 +211,5 @@ (if failing (concat (format " %s failing cases below:\n\n" numfailing) (pp-to-string failing)) - " Success!")))) + " Success!\n")))) ;; guix/hacking/reviews/emacs-aot/test-native-comp-p/preloaded ends here Diff finished. Sat Feb 17 08:50:04 2024 #+end_src I'm also attaching, for reference, the updated test-native-comp-p--preloaded.el in its entirety.
Am Samstag, dem 17.02.2024 um 14:49 +0000 schrieb Suhail: > Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > > ... don't appear to be preloaded through the loadup script, so I > > have no idea how to catch them or whether it's even worth doing so. > > I dug into this a little deeper. It seems that by adjusting the > load-path one is able to load the natively-compiled variants instead > of the byte-compiled for these features. Oddly, this adjustment > doesn't seem to be needed in non-Guix Emacs which may suggest the > presence of a bug. I'm hoping you have some insight that explains > this difference in behaviour. We adjust our load paths with environment variables. I don't think our choice of putting Emacs itself last is wrong here. > After modifying the previous test-native-comp-p--preloaded.el (in > hindsight, the preloaded part may be a misnomer) with the below patch > I got all the 97 cases passing. > > #+begin_src diff > diff -u ./test-native-comp-p--preloaded.el ./test-native-comp-p-- > preloaded.el > --- ./test-native-comp-p--preloaded.el 2024-02-17 > 08:49:36.294930488 -0500 > +++ ./test-native-comp-p--preloaded.el 2024-02-17 > 08:38:45.102839041 -0500 > @@ -155,8 +155,50 @@ > (x-dnd-init-frame) > (x-handle-no-bitmap-icon))) > > +(defun hack/tweak-load-path-in-guix () > + "Tweak `load-path' to allow natively compiled versions to be > loaded. > + > +If we ensure that the `load-path' entries for `term/internal', > +`mule-util' and `ucs-normalize' exist and they occur before the > +share/emacs/<emacs-version>/lisp entry then we are able to load > +the natively compiled versions of these libraries. Of those > +three, the entry for `term/internal' is missing whereas the > +others currently occur after the share/emacs/<emacs-version>/lisp > +entry. We remedy that here. > + > +Notably, in non-Guix Emacs this isn't needed. I.e., the fact > +that the share/emacs/<emacs-version>/lisp entry precedes the > +entries for `mule-util' and `ucs-normalize' is okay as is the > +fact that the entry for `term/internal' is missing." > + (when (getenv "GUIX_ENVIRONMENT") > + (require 'find-func) > + ;; first we'll add the missing entry for `term/internal' > + (add-to-list 'load-path > + (directory-file-name > + (file-name-directory > + (find-library-name (symbol-name > 'term/internal)))) > + ;; NOTE: we don't need to append; doing so simply > to confirm that > + ;; it's only the relative position wrt the > + ;; share/emacs/<emacs-version>/lisp entry that > matters. > + t) > + ;; then we'll ensure that the entry for share/emacs/<emacs- > version>/lisp is at > + ;; the end > + (let* ((sitelisppath (format "/share/emacs/%s/lisp" > + emacs-version)) > + (pathsuffix (seq-filter > + #'(lambda (x) > + (string-suffix-p sitelisppath x)) > + load-path)) > + (pathprefix (seq-filter > + #'(lambda (x) > + (not > + (string-suffix-p sitelisppath x))) > + load-path))) > + (setq load-path (append pathprefix pathsuffix))))) > + > (defun report/compilation-status/run () > "Run all cases and report those that aren't native-compiled." > + (hack/tweak-load-path-in-guix) > (let* ((results (mapcar (lambda (args) (apply > #'report/compilation-status args)) > report/compilation- > status/cases/preloaded)) > (failing (seq-filter (lambda (x) (not (eq (cdr x) > 'native-compiled))) > @@ -169,5 +211,5 @@ > (if failing > (concat (format " %s failing cases below:\n\n" > numfailing) > (pp-to-string failing)) > - " Success!")))) > + " Success!\n")))) > ;; guix/hacking/reviews/emacs-aot/test-native-comp-p/preloaded > ends here > > Diff finished. Sat Feb 17 08:50:04 2024 > #+end_src This won't work for the common use case of running Emacs from the store. Can you do some more research as to how this confusion comes to be? Cheers
"Liliana Marie Prikler" <liliana.prikler@gmail.com> writes: > I don't think our choice of putting Emacs itself last is wrong here. I'm not sure I understand. Just to be clear (please ignore in case this was already clear), in non-Guix Emacs the situation is as follows: 1. The directory where 'mule-util and 'ucs-normalize are located ("/usr/share/emacs/29.2/lisp/international") occurs in the load-path. And this entry occurs in the load-path AFTER "/usr/share/emacs/29.2/lisp". 2. The directory where 'term/internal is located ("/usr/share/emacs/29.2/lisp/international") does NOT occur in the load-path (and thus trivially doesn't occur before the "share/emacs/29.2/lisp" entry). After installing the v10 patch series, both 1 and 2 hold in Guix Emacs as well. However, Guix Emacs's behaviour when locating/loading natively-compiled versions of the above three features differs from the behaviour in non-Guix Emacs. Specifically, 1 and 2 above seem to pose a problem for only Guix Emacs and after remedying 1 and 2 above, as in the test script, the tests pass. All this to say, if by "putting Emacs itself last" you meant the change I made to the test script to make the tests pass, then while it may not be wrong, it also isn't correct either (seeing how it's not needed in non-Guix Emacs). My goal in sharing the patch was not to suggest a fix, but rather to possibly highlight something correlated with the cause of the problem we're observing. > Can you do some more research as to how this confusion comes to be? Since I have less familiarity with the internals of how Emacs locates natively-compiled features and loads them, I'm not sure where to begin. Do you have some concrete suggestions? What (I believe) we know: - Not all the .eln entries in the "preloaded" native-comp-eln-load-path directory in Emacs are actually loaded by default. This doesn't directly concern the issue, but it's to clarify that my use of the term "preloaded" in this thread is regarding the former and not the latter. - The issue of whether-natively-compiled-variants-are-loaded-or-not-depends-on-order-in-load-path doesn't seem to affect packages that aren't built-in. Specifically, if the load-entry for a not-built-in package is put after the "share/emacs/29.2/lisp" entry, Guix Emacs is still able to load the natively-compiled variant. - It is unclear why other packages such 'log-edit, 'find-func etc. (built-in, but not loaded by default, having their load-entry after the "share/emacs/29.2/lisp" entry) aren't affected.
Am Sonntag, dem 18.02.2024 um 00:56 +0000 schrieb Suhail: > "Liliana Marie Prikler" <liliana.prikler@gmail.com> writes: > > > I don't think our choice of putting Emacs itself last is wrong > > here. > > I'm not sure I understand. Just to be clear (please ignore in case > this was already clear), in non-Guix Emacs the situation is as > follows: > > 1. The directory where 'mule-util and 'ucs-normalize are located > ("/usr/share/emacs/29.2/lisp/international") occurs in the load- > path. > And this entry occurs in the load-path AFTER > "/usr/share/emacs/29.2/lisp". > > 2. The directory where 'term/internal is located > ("/usr/share/emacs/29.2/lisp/international") does NOT occur in the > load-path (and thus trivially doesn't occur before the > "share/emacs/29.2/lisp" entry). Directory (2) is the same as directory (1). I think you meant $prefix/share/emacs/$emacs_version/lisp/term? > After installing the v10 patch series, both 1 and 2 hold in Guix > Emacs as well. However, Guix Emacs's behaviour when locating/loading > natively-compiled versions of the above three features differs from > the behaviour in non-Guix Emacs. Specifically, 1 and 2 above seem to > pose a problem for only Guix Emacs and after remedying 1 and 2 above, > as in the test script, the tests pass. From my understanding (1) poses a problem because it messes up the way our patch computes relative file names. We could fix that with the newly introduced NATIVE_COMP_BOGUS_DIRS… hopefully. > All this to say, if by "putting Emacs itself last" you meant the > change I made to the test script to make the tests pass, then while > it may not be wrong, it also isn't correct either (seeing how it's > not needed in non-Guix Emacs). The last (first) directory in EMACSLOADPATH (EMACSNATIVELOADPATH) is $prefix/share/emacs/$emacs_version/lisp ($prefix/lib/emacs/…) on Guix System. > My goal in sharing the patch was not to suggest a fix, but rather to > possibly highlight something correlated with the cause of the problem > we're observing. > > > Can you do some more research as to how this confusion comes to be? > > Since I have less familiarity with the internals of how Emacs locates > natively-compiled features and loads them, I'm not sure where to > begin. Do you have some concrete suggestions? > What (I believe) we know: > > - Not all the .eln entries in the "preloaded" > native-comp-eln-load-path directory in Emacs are actually loaded by > default. This doesn't directly concern the issue, but it's to > clarify that my use of the term "preloaded" in this thread is > regarding the former and not the latter. Well, it does concern the issue in that locating such files becomes even harder. We need some place to put them and complicating these things for preloaded packages (for no real reason, might I add) is not fun. > - The issue of whether natively compiled variants are loaded or > depending on load-path order doesn't seem to affect packages that > aren't built-in. Specifically, if the load-entry for a > not-built-in package is put after the "share/emacs/29.2/lisp" > entry, Guix Emacs is still able to load the natively-compiled > variant. Well, obviously not. Those are found under their packages and are probably still within a flat directory hierarchy. > - It is unclear why other packages such 'log-edit, 'find-func > etc. (built-in, but not loaded by default, having their load-entry > after the "share/emacs/29.2/lisp" entry) aren't affected. Update your script to account for them and we shall find out. Cheers
"Liliana Marie Prikler" <liliana.prikler@gmail.com> writes: > Am Sonntag, dem 18.02.2024 um 00:56 +0000 schrieb Suhail: >> 1. The directory where 'mule-util and 'ucs-normalize are located >> ("/usr/share/emacs/29.2/lisp/international") occurs in the load- >> path. >> And this entry occurs in the load-path AFTER >> "/usr/share/emacs/29.2/lisp". >> >> 2. The directory where 'term/internal is located >> ("/usr/share/emacs/29.2/lisp/international") does NOT occur in the >> load-path (and thus trivially doesn't occur before the >> "share/emacs/29.2/lisp" entry). > Directory (2) is the same as directory (1). I think you meant > $prefix/share/emacs/$emacs_version/lisp/term? I did; thank you for catching that. >> - It is unclear why other packages such 'log-edit, 'find-func >> etc. (built-in, but not loaded by default, having their load-entry >> after the "share/emacs/29.2/lisp" entry) aren't affected. > Update your script to account for them and we shall find out. See the updated test script below. The above are accounted for in the third test case. The result of invoking the test script using the following invocation: #+begin_src sh :results replace emacs -Q --batch \ -l path/to/test-native-comp-p.el \ -f ert-run-tests-batch-and-exit 2>&1 #+end_src On non-Guix Emacs results in: #+begin_example Running 5 tests (2024-02-19 16:35:27-0500, selector ‘t’) passed 1/5 01-natively-compiled-features-passing-as-of-v10 (0.000035 sec) passed 2/5 02-natively-compiled-features-failing-as-of-v10 (0.000016 sec) passed 3/5 03-some-features-in-later-load-path-entries-are-still-natively-compiled (0.000012 sec) passed 4/5 04-load-path-order-should-not-determine-natively-compiled-status (0.000015 sec) passed 5/5 05-there-exists-load-path-order-where-all-tests-pass (0.000010 sec) Ran 5 tests, 5 results as expected, 0 unexpected (2024-02-19 16:35:27-0500, 0.000206 sec) #+end_example And with v10 patch series on Guix Emacs results in: #+begin_example Running 5 tests (2024-02-19 21:36:32+0000, selector `t') passed 1/5 01-natively-compiled-features-passing-as-of-v10 (0.000066 sec) failed 2/5 02-natively-compiled-features-failing-as-of-v10 (0.000034 sec) passed 3/5 03-some-features-in-later-load-path-entries-are-still-natively-compiled (0.000013 sec) failed 4/5 04-load-path-order-should-not-determine-natively-compiled-status (0.000034 sec) passed 5/5 05-there-exists-load-path-order-where-all-tests-pass (0.000014 sec) Ran 5 tests, 5 results as expected, 0 unexpected (2024-02-19 21:36:32+0000, 0.000322 sec) 2 expected failures #+end_example The test script (test-native-comp-p.el): #+begin_src elisp :eval never ;;; Code: (require 'ert) (setq ert-quiet nil ert-batch-print-level 10 ert-batch-print-length 10 ert-batch-backtrace-line-length t) ;;; utils/ (eval-and-compile (require 'help-fns) (defmacro utils/report-compilation-status (fun &optional feature) "Report on the compilation status of function FUN. Optionally load FEATURE before reporting on compilation status." `(progn (eval-when-compile (when ',feature (require ',feature))) (let ((descstr (substring-no-properties (with-output-to-string (help-fns-function-description-header ',fun))))) (cons ',fun (cond ((string-search " native-compiled" descstr) 'native-compiled) ;; ((string-search " autoloaded" descstr) descstr) ((string-search " byte-compiled" descstr) 'byte-compiled) (t descstr)))))) (defun utils/report-compilation-status/apply (fun &optional feature) "Invoke `utils/report-compilation-status' with FUN and FEATURE." (eval `(utils/report-compilation-status ,fun ,feature))) (require 'find-func) (defun utils/find-library-dir (feature) "Output directory where FEATURE resides." (directory-file-name (file-name-directory (find-library-name (symbol-name feature))))) (defun utils/report-failing-cases (cases) "From CASES, report failing tests. Test failure are those where `utils/report-compilation-status' doesn't report natively-compiled. CASES is a list where each element are ARGUMENTS for `utils/report-compilation-status'." (let ((results (mapcar (lambda (args) (apply #'utils/report-compilation-status/apply args)) cases))) (seq-filter (lambda (x) (not (eq (cdr x) 'native-compiled))) results)))) ;;; hack/ (eval-and-compile (defun hack/new-load-path-that-can-make-v10-test-failures-pass () "Return a list that can be used as the `load-path'. The returned list is assured to have the entry for share/emacs/<emacs-version>/lisp occur after the entry for lisp/international (corresponding to the `mule-util' and `ucs-normalize' failing test cases) and the entry for lisp/term (corresponding to the `term/internal' failing test case). If the `load-path' is set to the returned value, all tests pass in v10. Notably, in non-Guix Emacs this isn't needed. I.e., the fact that the share/emacs/<emacs-version>/lisp entry precedes the entries for `mule-util' and `ucs-normalize' is okay as is the fact that the entry for `term/internal' is missing." (eval-when-compile (require 'find-func)) ;; we'll ensure that the entry for share/emacs/<emacs-version>/lisp comes ;; after the lisp/international and lisp/term entries (let* ((new-load-path load-path) ;; add the missing entry for `term/internal' (_ (add-to-list 'new-load-path (utils/find-library-dir 'term/internal) ;; NOTE: we don't need to append; doing so simply to confirm that ;; it's only the relative position wrt the ;; share/emacs/<emacs-version>/lisp entry that matters. t)) (sitelisppath (format "/share/emacs/%s/lisp" emacs-version)) (pathsuffix (seq-filter (lambda (x) (string-suffix-p sitelisppath x)) new-load-path)) (pathprefix (seq-filter (lambda (x) (not (string-suffix-p sitelisppath x))) new-load-path))) (append pathprefix pathsuffix)))) ;;; cases/ ;; [[/usr/share/emacs/29.2/lisp]] (eval-when-compile (defvar cases/eln-in-preloaded-dir nil "Functions that ought to be natively-compiled.") (setq cases/eln-in-preloaded-dir '((abbrev-mode) (backquote-process) (mode-line-widen) (buffer-menu) ;; burmese (button-mode) (byte-run-strip-symbol-positions) (case-table-get-table) (cconv-convert) ;; cham (use-default-char-width-table) ;; chinese (cl-generic-p) (cl-struct-define) (x-setup-function-keys) (encode-composition-rule) ;; cp51932 (custom-declare-face) (minibuffer-prompt-properties--setter) ;; cus-start.el (custom-add-choice) ;; cyrillic ;; czech (debug-early) (display-table-slot disp-table) ;; disp-table.eln exists (dnd-open-file) (dos-mode25 dos-fns) ;; dos-fns.eln exists ;; dos-vars (find-file-text dos-w32) ;; dos-w32.eln exists (dynamic-setting-handle-config-changed-event) (easy-menu-item-present-p) ;; ediff-hook (eldoc-mode) (electric-indent-mode) (elisp-mode-syntax-propertize) ;; english (getenv) (epa-file-find-file-hook) ;; ethiopic ;; eucjp-ms ;; european (face-list) (find-file-noselect) (fill-region) ;; float-sup (font-lock-change-mode) (font-lock-add-keywords) (fontset-plain-name) (format-read) (frame-edges) (fringe-mode) ;; georgian ;; greek ;; haiku-win ;; hebrew (help-quick) (image-type) (indent-region) (indian-compose-regexp) ;; indonesian (msdos-setup-keyboard term/internal) ;; internal.eln exists (isearch-abort) (iso-transl-set-language) ;; japanese (jit-lock-mode) (jka-compr-build-file-regexp) (keymap-global-set) ;; khmer ;; korean ;; lao (forward-sexp) (lisp-string-in-doc-position-p) (ls-lisp-set-options ls-lisp) ;; ls-lisp.eln exists (macroexp-compiling-p) (map-y-or-n-p) (menu-find-file-existing) (completion-boundaries) (egyptian-shape-grouping) (mouse-double-click-time) (convert-define-charset-argument) (coding-system-change-eol-conversion) ;; mule-conf.eln (store-substring mule-util) ;; mule-util.eln exists (mouse-wheel-change-button) (advice-function-mapc) (comment-string-strip) ;; (ns-handle-nxopen term/ns-win) (obarray-make) (oclosure-type) (forward-page) (sentence-end) (show-paren-function) ;; (msdos-face-setup term/pc-win) (pgtk-dnd-init-frame pgtk-dnd) ;; pgtk-dnd.eln exists ;; (pgtk-drag-n-drop term/pgtk-win) ;; philippine (prog-context-menu) (regexp-opt) (get-register) (query-replace-descr) (rfn-eshadow-setup-minibuffer) (read-multiple-choice) ;; romanian (scroll-bar-scale) (gui-select-text) (seq-first) (hack-read-symbol-shorthands) (next-error-find-buffer) ;; sinhala ;; slovak (exit-splash-screen) (buffer-local-boundp) (syntax-propertize-multiline) (tab-bar-mode) (tabulated-list-put-tag) ;; tai-viet (text-mode) ;; thai ;; tibetan (timer-activate) (tool-bar-mode) (tooltip-mode) (tty-color-desc) (ucs-normalize-hfs-nfd-comp-p ucs-normalize) ;; ucs-normalize.eln exists (uniquify-item-p) ;; utf-8-lang.eln (vc-mode) (emacs-version) ;; vietnamese ;; (w32-shell-name) ;; w32-vars.eln ;; (w32-handle-dropped-file 'term/w32-win) (define-widget) (window-right) (x-dnd-init-frame) (x-handle-no-bitmap-icon)))) ;;; ERT tests (ert-deftest 01-natively-compiled-features-passing-as-of-v10 () "The 94 cases which pass for v10 patch series. These cases are taken from .eln files that are located in the preloaded directory for non-Guix Emacs." (eval-when-compile (let ((cases (seq-filter (lambda (x) (not (memq (cadr x) '(mule-util term/internal ucs-normalize)))) cases/eln-in-preloaded-dir))) (should-not (utils/report-failing-cases cases))))) (ert-deftest 02-natively-compiled-features-failing-as-of-v10 () "The 3 cases which fail for v10 patch series on Guix Emacs. These cases are taken from .eln files that are located in the preloaded directory for non-Guix Emacs." :expected-result (if (getenv "GUIX_ENVIRONMENT") :failed :passed) (should-not (eval-when-compile (let ((cases (seq-filter (lambda (x) (memq (cadr x) '(mule-util term/internal ucs-normalize))) cases/eln-in-preloaded-dir))) (utils/report-failing-cases cases))))) (ert-deftest 03-some-features-in-later-load-path-entries-are-still-natively-compiled () "These cases pass as of v10 of the patch. These cases share the fact that their directory entries occur in the `load-path' after the $prefix/share/emacs/$emacs_version/lisp entry. This is something these cases have in common with the three cases that are known to fail, however, unlike them these succeed (i.e., natively-compiled variants are loaded)." (eval-when-compile (let* ((cases '((cl-position cl-seq) (find-library-name find-func) (log-edit log-edit))) (failing (utils/report-failing-cases cases)) (features-loadpath-entries (mapcar #'utils/find-library-dir (mapcar #'cadr cases))) (features-entry-pos (mapcar (lambda (x) (cl-position (utils/find-library-dir (cadr x)) load-path :test #'equal)) cases)) (share-emacs-lisp-entry-pos (cl-position "/share/emacs/29.2/lisp" load-path :test #'string-suffix-p))) (should-not failing) (should-not (seq-filter (lambda (x) (< x share-emacs-lisp-entry-pos)) features-entry-pos))))) (ert-deftest 04-load-path-order-should-not-determine-natively-compiled-status () "This seems like an invariant that would be useful to have. It is unclear if this is guaranteed by upstream Emacs, but observations seem consistent with it." :expected-result (if (getenv "GUIX_ENVIRONMENT") :failed :passed) (eval-when-compile (defvar original-load-path load-path)) (let ((failures-prior-to-load-path-shuffle (eval-when-compile (utils/report-failing-cases cases/eln-in-preloaded-dir))) (failures-post-load-path-shuffle (eval-when-compile (progn (setq load-path (hack/new-load-path-that-can-make-v10-test-failures-pass)) (dolist (item cases/eln-in-preloaded-dir) (when (cadr item) (unload-feature (cadr item)))) (utils/report-failing-cases cases/eln-in-preloaded-dir))))) (should (equal failures-prior-to-load-path-shuffle failures-post-load-path-shuffle)))) (ert-deftest 05-there-exists-load-path-order-where-all-tests-pass () "Proof witness that the v10 failing cases relate to load-path ordering." (should-not (eval-when-compile (when (and (boundp 'original-load-path) original-load-path) (setq load-path original-load-path) (dolist (item cases/eln-in-preloaded-dir) (when (cadr item) (unload-feature (cadr item))))) (defvar original-load-path load-path) (setq load-path (hack/new-load-path-that-can-make-v10-test-failures-pass)) (utils/report-failing-cases cases/eln-in-preloaded-dir)))) ;;; test-native-comp-p.el ends here #+end_src
Am Montag, dem 19.02.2024 um 21:42 +0000 schrieb Suhail: > "Liliana Marie Prikler" <liliana.prikler@gmail.com> writes: > > > Am Sonntag, dem 18.02.2024 um 00:56 +0000 schrieb Suhail: > > > 1. The directory where 'mule-util and 'ucs-normalize are located > > > ("/usr/share/emacs/29.2/lisp/international") occurs in the > > > load- > > > path. > > > And this entry occurs in the load-path AFTER > > > "/usr/share/emacs/29.2/lisp". > > > > > > 2. The directory where 'term/internal is located > > > ("/usr/share/emacs/29.2/lisp/international") does NOT occur in > > > the > > > load-path (and thus trivially doesn't occur before the > > > "share/emacs/29.2/lisp" entry). > > Directory (2) is the same as directory (1). I think you meant > > $prefix/share/emacs/$emacs_version/lisp/term? > > I did; thank you for catching that. > > > > - It is unclear why other packages such 'log-edit, 'find-func > > > etc. (built-in, but not loaded by default, having their load- > > > entry > > > after the "share/emacs/29.2/lisp" entry) aren't affected. > > Update your script to account for them and we shall find out. > > See the updated test script below. The above are accounted for in > the third test case. We still only have 97 tests squashed to 5 cases IIUC. There's some 1000 files in the native-lisp directory. What I was actually hoping for is more or less one test per feature. > The result of invoking the test script using the following > invocation: > > #+begin_src sh :results replace > emacs -Q --batch \ > -l path/to/test-native-comp-p.el \ > -f ert-run-tests-batch-and-exit 2>&1 > #+end_src > > On non-Guix Emacs results in: > > #+begin_example > Running 5 tests (2024-02-19 16:35:27-0500, selector ‘t’) > passed 1/5 01-natively-compiled-features-passing-as-of-v10 > (0.000035 sec) > passed 2/5 02-natively-compiled-features-failing-as-of-v10 > (0.000016 sec) > passed 3/5 03-some-features-in-later-load-path-entries-are- > still-natively-compiled (0.000012 sec) > passed 4/5 04-load-path-order-should-not-determine-natively- > compiled-status (0.000015 sec) > passed 5/5 05-there-exists-load-path-order-where-all-tests- > pass (0.000010 sec) > > Ran 5 tests, 5 results as expected, 0 unexpected (2024-02-19 > 16:35:27-0500, 0.000206 sec) > #+end_example > > And with v10 patch series on Guix Emacs results in: > > #+begin_example > Running 5 tests (2024-02-19 21:36:32+0000, selector `t') > passed 1/5 01-natively-compiled-features-passing-as-of-v10 > (0.000066 sec) > failed 2/5 02-natively-compiled-features-failing-as-of-v10 > (0.000034 sec) > passed 3/5 03-some-features-in-later-load-path-entries-are- > still-natively-compiled (0.000013 sec) > failed 4/5 04-load-path-order-should-not-determine-natively- > compiled-status (0.000034 sec) > passed 5/5 05-there-exists-load-path-order-where-all-tests- > pass (0.000014 sec) > > Ran 5 tests, 5 results as expected, 0 unexpected (2024-02-19 > 21:36:32+0000, 0.000322 sec) > 2 expected failures > #+end_example > > The test script (test-native-comp-p.el): > > #+begin_src elisp :eval never > ;;; Code: > (require 'ert) > (setq ert-quiet nil > ert-batch-print-level 10 > ert-batch-print-length 10 > ert-batch-backtrace-line-length t) > > ;;; utils/ > (eval-and-compile > (require 'help-fns) > (defmacro utils/report-compilation-status (fun &optional feature) > "Report on the compilation status of function FUN. > Optionally load FEATURE before reporting on compilation status." > `(progn > (eval-when-compile > (when ',feature > (require ',feature))) > (let ((descstr (substring-no-properties > (with-output-to-string > (help-fns-function-description-header > ',fun))))) > (cons ',fun > (cond > ((string-search " native-compiled" descstr) > 'native-compiled) > ;; ((string-search " autoloaded" descstr) descstr) > ((string-search " byte-compiled" descstr) 'byte- > compiled) > (t descstr)))))) > > (defun utils/report-compilation-status/apply (fun &optional > feature) > "Invoke `utils/report-compilation-status' with FUN and > FEATURE." > (eval `(utils/report-compilation-status ,fun ,feature))) > > (require 'find-func) > (defun utils/find-library-dir (feature) > "Output directory where FEATURE resides." > (directory-file-name > (file-name-directory > (find-library-name (symbol-name feature))))) > > (defun utils/report-failing-cases (cases) > "From CASES, report failing tests. > Test failure are those where `utils/report-compilation-status' > doesn't > report natively-compiled. CASES is a list where each element are > ARGUMENTS for `utils/report-compilation-status'." > (let ((results (mapcar (lambda (args) > (apply #'utils/report-compilation- > status/apply args)) > cases))) > (seq-filter (lambda (x) (not (eq (cdr x) 'native-compiled))) > results)))) > > ;;; hack/ > (eval-and-compile > (defun hack/new-load-path-that-can-make-v10-test-failures-pass () > "Return a list that can be used as the `load-path'. > > The returned list is assured to have the entry for > share/emacs/<emacs-version>/lisp occur after the entry for > lisp/international (corresponding to the `mule-util' and > `ucs-normalize' failing test cases) and the entry for lisp/term > (corresponding to the `term/internal' failing test case). > > If the `load-path' is set to the returned value, all tests pass > in v10. Notably, in non-Guix Emacs this isn't needed. I.e., the > fact that the share/emacs/<emacs-version>/lisp entry precedes the > entries for `mule-util' and `ucs-normalize' is okay as is the > fact that the entry for `term/internal' is missing." > (eval-when-compile > (require 'find-func)) > ;; we'll ensure that the entry for share/emacs/<emacs- > version>/lisp comes > ;; after the lisp/international and lisp/term entries > (let* ((new-load-path load-path) > ;; add the missing entry for `term/internal' > (_ (add-to-list 'new-load-path > (utils/find-library-dir 'term/internal) > ;; NOTE: we don't need to append; doing > so simply to confirm that > ;; it's only the relative position wrt > the > ;; share/emacs/<emacs-version>/lisp > entry that matters. > t)) > (sitelisppath (format "/share/emacs/%s/lisp" > emacs-version)) > (pathsuffix (seq-filter > (lambda (x) > (string-suffix-p sitelisppath x)) > new-load-path)) > (pathprefix (seq-filter > (lambda (x) > (not > (string-suffix-p sitelisppath x))) > new-load-path))) > (append pathprefix pathsuffix)))) > > ;;; cases/ > ;; [[/usr/share/emacs/29.2/lisp]] > (eval-when-compile > (defvar cases/eln-in-preloaded-dir nil > "Functions that ought to be natively-compiled.") > (setq cases/eln-in-preloaded-dir > '((abbrev-mode) > (backquote-process) > (mode-line-widen) > (buffer-menu) > ;; burmese > (button-mode) > (byte-run-strip-symbol-positions) > (case-table-get-table) > (cconv-convert) > ;; cham > (use-default-char-width-table) > ;; chinese > (cl-generic-p) > (cl-struct-define) > (x-setup-function-keys) > (encode-composition-rule) > ;; cp51932 > (custom-declare-face) > (minibuffer-prompt-properties--setter) ;; cus-start.el > (custom-add-choice) > ;; cyrillic > ;; czech > (debug-early) > (display-table-slot disp-table) ;; disp-table.eln exists > (dnd-open-file) > (dos-mode25 dos-fns) ;; dos-fns.eln exists > ;; dos-vars > (find-file-text dos-w32) ;; dos-w32.eln exists > (dynamic-setting-handle-config-changed-event) > (easy-menu-item-present-p) > ;; ediff-hook > (eldoc-mode) > (electric-indent-mode) > (elisp-mode-syntax-propertize) > ;; english > (getenv) > (epa-file-find-file-hook) > ;; ethiopic > ;; eucjp-ms > ;; european > (face-list) > (find-file-noselect) > (fill-region) > ;; float-sup > (font-lock-change-mode) > (font-lock-add-keywords) > (fontset-plain-name) > (format-read) > (frame-edges) > (fringe-mode) > ;; georgian > ;; greek > ;; haiku-win > ;; hebrew > (help-quick) > (image-type) > (indent-region) > (indian-compose-regexp) > ;; indonesian > (msdos-setup-keyboard term/internal) ;; internal.eln > exists > (isearch-abort) > (iso-transl-set-language) > ;; japanese > (jit-lock-mode) > (jka-compr-build-file-regexp) > (keymap-global-set) > ;; khmer > ;; korean > ;; lao > (forward-sexp) > (lisp-string-in-doc-position-p) > (ls-lisp-set-options ls-lisp) ;; ls-lisp.eln exists > (macroexp-compiling-p) > (map-y-or-n-p) > (menu-find-file-existing) > (completion-boundaries) > (egyptian-shape-grouping) > (mouse-double-click-time) > (convert-define-charset-argument) > (coding-system-change-eol-conversion) > ;; mule-conf.eln > (store-substring mule-util) ;; mule-util.eln exists > (mouse-wheel-change-button) > (advice-function-mapc) > (comment-string-strip) > ;; (ns-handle-nxopen term/ns-win) > (obarray-make) > (oclosure-type) > (forward-page) > (sentence-end) > (show-paren-function) > ;; (msdos-face-setup term/pc-win) > (pgtk-dnd-init-frame pgtk-dnd) ;; pgtk-dnd.eln exists > ;; (pgtk-drag-n-drop term/pgtk-win) > ;; philippine > (prog-context-menu) > (regexp-opt) > (get-register) > (query-replace-descr) > (rfn-eshadow-setup-minibuffer) > (read-multiple-choice) > ;; romanian > (scroll-bar-scale) > (gui-select-text) > (seq-first) > (hack-read-symbol-shorthands) > (next-error-find-buffer) > ;; sinhala > ;; slovak > (exit-splash-screen) > (buffer-local-boundp) > (syntax-propertize-multiline) > (tab-bar-mode) > (tabulated-list-put-tag) > ;; tai-viet > (text-mode) > ;; thai > ;; tibetan > (timer-activate) > (tool-bar-mode) > (tooltip-mode) > (tty-color-desc) > (ucs-normalize-hfs-nfd-comp-p ucs-normalize) ;; ucs- > normalize.eln exists > (uniquify-item-p) > ;; utf-8-lang.eln > (vc-mode) > (emacs-version) > ;; vietnamese > ;; (w32-shell-name) > ;; w32-vars.eln > ;; (w32-handle-dropped-file 'term/w32-win) > (define-widget) > (window-right) > (x-dnd-init-frame) > (x-handle-no-bitmap-icon)))) > > ;;; ERT tests > (ert-deftest 01-natively-compiled-features-passing-as-of-v10 () > "The 94 cases which pass for v10 patch series. > These cases are taken from .eln files that are located in the > preloaded > directory for non-Guix Emacs." > (eval-when-compile > (let ((cases (seq-filter (lambda (x) > (not (memq (cadr x) '(mule-util > term/internal ucs-normalize)))) > cases/eln-in-preloaded-dir))) > (should-not (utils/report-failing-cases cases))))) > > (ert-deftest 02-natively-compiled-features-failing-as-of-v10 () > "The 3 cases which fail for v10 patch series on Guix Emacs. > These cases are taken from .eln files that are located in the > preloaded > directory for non-Guix Emacs." > :expected-result (if (getenv "GUIX_ENVIRONMENT") That is not a good way of checking whether it's Guix' emacs or not. I propose doing a per-file deftest instead. > :failed > :passed) > (should-not > (eval-when-compile > (let ((cases (seq-filter (lambda (x) > (memq (cadr x) '(mule-util > term/internal ucs-normalize))) > cases/eln-in-preloaded-dir))) > (utils/report-failing-cases cases))))) > > (ert-deftest 03-some-features-in-later-load-path-entries-are-still- > natively-compiled () > "These cases pass as of v10 of the patch. > These cases share the fact that their directory entries occur in > the `load-path' after the $prefix/share/emacs/$emacs_version/lisp > entry. This is something these cases have in common with the > three cases that are known to fail, however, unlike them these > succeed (i.e., natively-compiled variants are loaded)." > (eval-when-compile > (let* ((cases '((cl-position cl-seq) > (find-library-name find-func) > (log-edit log-edit))) > (failing (utils/report-failing-cases cases)) > (features-loadpath-entries (mapcar #'utils/find-library- > dir > (mapcar #'cadr > cases))) > (features-entry-pos (mapcar (lambda (x) > (cl-position > (utils/find-library-dir > (cadr x)) > load-path :test #'equal)) > cases)) > (share-emacs-lisp-entry-pos (cl-position > "/share/emacs/29.2/lisp" > load-path > :test #'string- > suffix-p))) > (should-not failing) > (should-not (seq-filter (lambda (x) (< x share-emacs-lisp- > entry-pos)) > features-entry-pos))))) > > (ert-deftest 04-load-path-order-should-not-determine-natively- > compiled-status () > "This seems like an invariant that would be useful to have. > It is unclear if this is guaranteed by upstream Emacs, but > observations seem consistent with it." > :expected-result (if (getenv "GUIX_ENVIRONMENT") > :failed > :passed) > (eval-when-compile > (defvar original-load-path load-path)) > (let ((failures-prior-to-load-path-shuffle > (eval-when-compile > (utils/report-failing-cases cases/eln-in-preloaded- > dir))) > (failures-post-load-path-shuffle > (eval-when-compile > (progn > (setq load-path > (hack/new-load-path-that-can-make-v10-test- > failures-pass)) > (dolist (item cases/eln-in-preloaded-dir) > (when (cadr item) > (unload-feature (cadr item)))) > (utils/report-failing-cases cases/eln-in-preloaded- > dir))))) > (should (equal failures-prior-to-load-path-shuffle > failures-post-load-path-shuffle)))) > > (ert-deftest 05-there-exists-load-path-order-where-all-tests-pass > () > "Proof witness that the v10 failing cases relate to load-path > ordering." > (should-not (eval-when-compile > (when (and (boundp 'original-load-path) original- > load-path) > (setq load-path original-load-path) > (dolist (item cases/eln-in-preloaded-dir) > (when (cadr item) > (unload-feature (cadr item))))) > (defvar original-load-path load-path) > (setq load-path > (hack/new-load-path-that-can-make-v10-test- > failures-pass)) > (utils/report-failing-cases cases/eln-in-preloaded- > dir)))) > > ;;; test-native-comp-p.el ends here > #+end_src Could you do a MIME attachment next time? I think I know the heart of the issue now, but I still need to code up a solution. Cheers
"Liliana Marie Prikler" <liliana.prikler@gmail.com> writes: > We still only have 97 tests squashed to 5 cases IIUC. Yes. > There's some 1000 files in the native-lisp directory. What I was > actually hoping for is more or less one test per feature. Yes, I agree that that would be useful, but doing so was (is) more than what I was (am) able to do in the time I was (am) able to commit (at present). I believe it would be valuable to have such an exhaustive test included as part of the patch submission to prevent future regressions. > That is not a good way of checking whether it's Guix' emacs or not. What would be a better way? Matching against the --prefix value in the output of emacs-build-description function? > I propose doing a per-file deftest instead. I don't understand the connection between a per-file deftest and the manner in which guix-emacs-or-not is tested, but otherwise agree on the utility of per-feature deftests. > Could you do a MIME attachment next time? Sure. For now, for what it's worth and in case it helps, please see attached a copy of the same file as before.
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > I finally came up with the perfectly cursed mix that enables us to > actually load *and* graft our natively compiled packages with ease. I can confirm that all the features being checked show up as natively compiled as default - yay! However, there seems to be some brittleness in whether or not natively-compiled versions can be located. Specifically, by reordering the entries of the load-path, unloading previously loaded features, and then reloading them some features will be reported as being byte-compiled instead of natively-compiled. Is my expectation that reordering entries in the load-path shouldn't affect the natively-compiled status misplaced? From the previously shared test script, cases 01, 02, 03 pass now. However, case 04-load-path-order-should-not-determine-natively-compiled-status still fails. Where in v10 it failed because 3 features previously reported as being byte-compiled became natively-compiled, now it fails because the same 3 features go from being natively-compiled to byte-compiled after reordering load-path. The features in question are the same ones that were problematic in v10: - term/internal - mule-util - ucs-normalize My opinion: v11 seems to be an improvement over v10. While I don't believe that v11 is free of *all* bugs, I don't believe that that should be a necessary pre-requisite before merging in. I believe it would be beneficial to merge v11 into emacs-team soon(ish) and for the emacs-team branch to be merged into master soon thereafter. A separate bug issue can be created to track the peculiar dependence of the native-compilation status on the ordering of entries in load-path. Thoughts?
Am Freitag, dem 01.03.2024 um 17:35 +0000 schrieb Suhail: > Is my expectation that reordering entries in the load-path shouldn't > affect the natively-compiled status misplaced? Yes. We take the first prefix match and compute the file names from there. This is consistent with emacs matching the first file it finds on the load-path. You can't do much better, because your load path may only be partially specified at compile time and later expanded with normal-top-level-add-to-load-path. > Where in v10 it failed because 3 features previously reported as > being byte-compiled became natively-compiled, now it fails because > the same 3 features go from being natively-compiled to byte-compiled > after reordering load-path. The expectation that load-path order does not matter is imho quite unfounded. Note that we do ship the actually important test cases with v11. > A separate bug issue can be created to track the peculiar dependence > of the native-compilation status on the ordering of entries in load- > path. Well, to me it's not peculiar as I wrote the code, but I'm not sure how familiar you are with Emacs' internals. If you feel up for it, go for it, but I'd rather tackle other problems related to our emacs ecosystem. Anyway, as you said, I'm pushing this now so that we can do a double merge dance (i.e. master → emacs-team, then request the other way) starting early tomorrow. Cheers
On 2024-03-01 20:40, Liliana Marie Prikler wrote: > Am Freitag, dem 01.03.2024 um 17:35 +0000 schrieb Suhail: >> Is my expectation that reordering entries in the load-path shouldn't >> affect the natively-compiled status misplaced? > Yes. We take the first prefix match and compute the file names from > there. This is consistent with emacs matching the first file it finds > on the load-path. You can't do much better, because your load path may > only be partially specified at compile time and later expanded with > normal-top-level-add-to-load-path. > >> Where in v10 it failed because 3 features previously reported as >> being byte-compiled became natively-compiled, now it fails because >> the same 3 features go from being natively-compiled to byte-compiled >> after reordering load-path. > The expectation that load-path order does not matter is imho quite > unfounded. Note that we do ship the actually important test cases with > v11. > >> A separate bug issue can be created to track the peculiar dependence >> of the native-compilation status on the ordering of entries in load- >> path. > Well, to me it's not peculiar as I wrote the code, but I'm not sure how > familiar you are with Emacs' internals. If you feel up for it, go for > it, but I'd rather tackle other problems related to our emacs > ecosystem. > > Anyway, as you said, I'm pushing this now so that we can do a double > merge dance (i.e. master → emacs-team, then request the other way) > starting early tomorrow. Hi Liliana! guix time-machine --branch=emacs-team -- shell emacs-pgtk emacs-magit --pure -E '.*GTK.*|.*XDG.*|.*DISPLAY.*' -- emacs -Q fails with: --8<---------------cut here---------------start------------->8--- passed 94/97 expect-window-right-native (0.002334 sec) Test expect-x-dnd-init-frame-native backtrace: signal(void-function (x-dnd-init-frame)) apply(signal (void-function (x-dnd-init-frame))) (setq value-477 (apply fn-475 args-476)) (unwind-protect (setq value-477 (apply fn-475 args-476)) (setq form- (if (unwind-protect (setq value-477 (apply fn-475 args-476)) (setq f (let (form-description-479) (if (unwind-protect (setq value-477 (app (let ((value-477 'ert-form-evaluation-aborted-478)) (let (form-descr (let* ((fn-475 #'eq) (args-476 (condition-case err (let ((signal-hoo (lambda nil (let* ((fn-475 #'eq) (args-476 (condition-case err (let ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test ert-run-test(#s(ert-test :name expect-x-dnd-init-frame-native :docum ert-run-or-rerun-test(#s(ert--stats :selector t :tests ... :test-map ert-run-tests(t #f(compiled-function (event-type &rest event-args) # ert-run-tests-batch(nil) ert-run-tests-batch-and-exit() command-line-1(("--load" "/gnu/store/ql9y5r4ydz4dl8jjf2b2bz9fqhxmbm3 command-line() normal-top-level() Test expect-x-dnd-init-frame-native condition: (void-function x-dnd-init-frame) FAILED 95/97 expect-x-dnd-init-frame-native (0.000067 sec) at ../../../gnu/store/ql9y5r4ydz4dl8jjf2b2bz9fqhxmbm3x-comp-integrity.el:1 Test expect-x-handle-no-bitmap-icon-native backtrace: signal(void-function (x-handle-no-bitmap-icon)) apply(signal (void-function (x-handle-no-bitmap-icon))) (setq value-482 (apply fn-480 args-481)) (unwind-protect (setq value-482 (apply fn-480 args-481)) (setq form- (if (unwind-protect (setq value-482 (apply fn-480 args-481)) (setq f (let (form-description-484) (if (unwind-protect (setq value-482 (app (let ((value-482 'ert-form-evaluation-aborted-483)) (let (form-descr (let* ((fn-480 #'eq) (args-481 (condition-case err (let ((signal-hoo (lambda nil (let* ((fn-480 #'eq) (args-481 (condition-case err (let ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test ert-run-test(#s(ert-test :name expect-x-handle-no-bitmap-icon-native ert-run-or-rerun-test(#s(ert--stats :selector t :tests ... :test-map ert-run-tests(t #f(compiled-function (event-type &rest event-args) # ert-run-tests-batch(nil) ert-run-tests-batch-and-exit() command-line-1(("--load" "/gnu/store/ql9y5r4ydz4dl8jjf2b2bz9fqhxmbm3 command-line() normal-top-level() Test expect-x-handle-no-bitmap-icon-native condition: (void-function x-handle-no-bitmap-icon) FAILED 96/97 expect-x-handle-no-bitmap-icon-native (0.000103 sec) at ../../../gnu/store/ql9y5r4ydz4dl8jjf2b2bz9fqhxmbm3x-comp-integrity.el:1 passed 97/97 expect-x-setup-function-keys-native (0.002261 sec) Ran 97 tests, 95 results as expected, 2 unexpected (2024-03-07 08:50:40+0000, 0.327688 sec) 2 unexpected results: FAILED expect-x-dnd-init-frame-native FAILED expect-x-handle-no-bitmap-icon-native error: in phase 'validate-comp-integrity': uncaught exception: %exception #<&invoke-error program: "/gnu/store/lkqd9kx4nb4y6dw58bn8gqhid6q51i30-emacs-pgtk-29.2/bin/emacs" arguments: ("--batch" "--load" "/gnu/store/ql9y5r4ydz4dl8jjf2b2bz9fqhxmbm3x-comp-integrity.el" "-f" "ert-run-tests-batch-and-exit") exit-status: 1 term-signal: #f stop-signal: #f> phase `validate-comp-integrity' failed after 0.5 seconds command "/gnu/store/lkqd9kx4nb4y6dw58bn8gqhid6q51i30-emacs-pgtk-29.2/bin/emacs" "--batch" "--load" "/gnu/store/ql9y5r4ydz4dl8jjf2b2bz9fqhxmbm3x-comp-integrity.el" "-f" "ert-run-tests-batch-and-exit" failed with status 1 --8<---------------cut here---------------end--------------->8---