Package: guix-patches;
Reported by: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Date: Sat, 18 Nov 2023 13:50:02 UTC
Severity: normal
Tags: patch
Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Bug is archived. No further changes may be made.
Message #320 received at 67260 <at> debbugs.gnu.org (full text, mbox):
From: Liliana Marie Prikler <liliana.prikler <at> gmail.com> To: Suhail <suhail <at> bayesians.ca> Cc: cox.katherine.e+guix <at> gmail.com, 67260 <at> debbugs.gnu.org, andrew <at> trop.in Subject: Re: [bug#67260] [PATCH emacs-team v10 0/7] Preload most of the things Date: Tue, 20 Feb 2024 18:51:01 +0100
Am Montag, dem 19.02.2024 um 21:42 +0000 schrieb Suhail:
> "Liliana Marie Prikler" <liliana.prikler <at> 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.