* bug#59707: 29.0.50; Seeking a more robust `package-quickstart' @ 2022-11-30 0:14 Matt Armstrong 2022-11-30 2:37 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 1 reply; 5+ messages in thread From: Matt Armstrong @ 2022-11-30 0:14 UTC (permalink / raw) To: 59707; +Cc: stefan monnier X-Debbugs-Cc: Stefan Monnier <monnier@iro.umontreal.ca> My hope for this bug is to explore, and possibly implement, improvements to Emacs that make it more robust against the class of bug I describe below. I'm looking for input from people with more knowledge/experience with Emacs packaging, and especially `package-quickstart' (hence +CC monnier@) General description ======================================== It is possible to "brick" your emacs if `package-quickstart' is t and there is a packaging bug in something you install. By "brick" I mean Emacs does not launch, but prints a potentially useless error message to stderr and exits with 255 error code. I was able to diagnose and fix the offending package, but only after spending too much time debugging, and resorting to running Emacs through gdb and disassembling compiled .elc files. Note that in this case the upstream packager was just as mystified as I was as to what the problem was, and the upstream bug was many months old before I finally diagnosed and fixed it. Because it happened only with (setq package-quickstart t) a minority of users were seeing the problem. Specific description ======================================== One instance of the issue is here: https://gitlab.com/emacs-geiser/geiser/-/issues/47 The one line fix is here: https://gitlab.com/emacs-geiser/geiser/-/merge_requests/13/diffs?commit_id=ca0407bc942d3cae5ff1ae0bd966311b425d9a86 I will summarize the repro step here, though the steps no longer work because the package has been fixed: 1) set `package-quickstart' to t in both init.el and interactively. 2) M-x package-install RET "geiser-guile" 3) quit emacs 4) launch emacs With this, emacs fails to launch, instead printing: Symbol’s value as variable is void: geiser-active-implementations The root problem was that `geiser-activate-implementation' was a defsubst, not a defun, yet it was autoloaded, which makes no sense because it is expanded inline in byte code, and thus won't load its original package. Here is an edited down version of what ends up in my ~/.config/emacs/package-quickstart.el after installing `geiser-guile': (let ((load-true-file-name "/home/matt/.config/emacs/elpa/geiser-0.28/geiser-autoloads.el") (load-file-name "/home/matt/.config/emacs/elpa/geiser-0.28/geiser-autoloads.el")) (autoload 'geiser-activate-implementation "geiser-impl" "Register the given implementation as active.") (provide 'geiser-autoloads)) (let ((load-true-file-name "/home/matt/.config/emacs/elpa/geiser-guile-0.28.0/geiser-guile-autoloads.el") (load-file-name "/home/matt/.config/emacs/elpa/geiser-guile-0.28.0/geiser-guile-autoloads.el")) (geiser-activate-implementation 'guile) (provide 'geiser-guile-autoloads)) The code for `geiser-activate-implementation' was originally: (defsubst geiser-activate-implementation (impl) (add-to-list 'geiser-active-implementations impl)) So the `(add-to-list 'geiser-active-implementations 'guile)' ended up inlined into "package-quickstart.elc", which executed directly, without first (auto) loading 'geiser-impl' as was intended. Thus, the `geiser-active-implementations' variable did not exist, yet was referenced, by inlining, but did not appear textually in package-quickstart.el. Once understood obvious fix to make geiser-activate-implementation a defun instead of a defsubst. Ideas for improvement (also seeking ideas from others) ======================================== 0) Make `package-quickstart t' the default, because it is great, and that way people tend to see the same kinds of problems. :-) 1) Harden Emacs such that signaled errors from "package-quickstart.elc" don't prevent startup (but are somehow saved and logged, maybe as warnings?). 2) Make --debug-init functional for errors thrown by "package-quickstart.elc". Currently, package-quickstart is loaded before the first frame is initialized, so Emacs takes the "print the error and exit" route for any signaled errors, instead of, for example, ending up in the elisp debugger. 3) Perhaps package-quickstart.elc could be loaded later in the initialization phase, after the first frame is created? ...though I suppose it is nice to load packages that are themes before the first frame is created... 4) Perhaps if loading package-quickstart signals an error it can be ignored and the slow path can be taken instead? 5) Change "package-quickstart.el" code gen to provide more context for errors, perhaps with `condition-case-unless-debug'. The jwigley's use-package package does something similar in its macro expansions and it is nice. In the specific case with the geiser packages, because Emacs exited immediately, it would have been useful to know that the error came from the "geiser-guile-autloads" portion of "package-quickstart.elc". Currently there is no backtrace to speak of because "package-quickstart" is so flat. 6) Print a warning when a `defsubst' function is autoloaded, as the autoload won't work from byte compiled code. In GNU Emacs 29.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.24.34, cairo version 1.16.0) of 2022-11-25 built on naz Repository revision: 2389158a31b4a126a328146399fe7ef304c97fef Repository branch: master System Description: Debian GNU/Linux bookworm/sid Configured using: 'configure 'CFLAGS=-Og -g3 -fsanitize=address' 'CXXFLAGS=-Og -g3 -fsanitize=address' LDFLAGS=-static-libasan --enable-checking=yes --enable-check-lisp-object-type --with-pgtk' Configured features: ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PGTK PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS XIM GTK3 ZLIB Important settings: value of $LANG: en_US.UTF-8 value of $XMODIFIERS: @im=ibus locale-coding-system: utf-8-unix Major mode: ELisp/d Minor modes in effect: global-git-commit-mode: t magit-auto-revert-mode: t msb-mode: t display-time-mode: t flyspell-mode: t global-tab-line-mode: t tab-line-mode: t server-mode: t shell-dirtrack-mode: t auto-insert-mode: t keyfreq-autosave-mode: t keyfreq-mode: t savehist-mode: t icomplete-vertical-mode: t icomplete-mode: t editorconfig-mode: t which-key-mode: t electric-pair-mode: t override-global-mode: t tooltip-mode: t global-eldoc-mode: t eldoc-mode: t show-paren-mode: t electric-indent-mode: t mouse-wheel-mode: t tab-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t column-number-mode: t line-number-mode: t transient-mark-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t temp-buffer-resize-mode: t Load-path shadows: /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-guile-0.28.0/geiser-guile hides /home/matt/.config/emacs/elpa/geiser-guile-0.28.0/geiser-guile /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-guile-0.28.0/geiser-guile-autoloads hides /home/matt/.config/emacs/elpa/geiser-guile-0.28.0/geiser-guile-autoloads /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-guile-0.28.0/geiser-guile-pkg hides /home/matt/.config/emacs/elpa/geiser-guile-0.28.0/geiser-guile-pkg /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-impl hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-impl /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-xref hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-xref /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-table hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-table /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-syntax hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-syntax /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-repl hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-repl /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-reload hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-reload /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-popup hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-popup /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-mode hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-mode /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-menu hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-menu /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-log hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-log /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-image hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-image /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-eval hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-eval /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-edit hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-edit /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-doc hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-doc /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-debug hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-debug /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-custom hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-custom /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-connection hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-connection /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-completion hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-completion /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-compile hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-compile /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-capf hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-capf /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-base hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-base /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-autodoc hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-autodoc /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-autoloads hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-autoloads /home/matt/scratch/geiser-repro/.emacs.d/elpa/geiser-0.28/geiser-pkg hides /home/matt/.config/emacs/elpa/geiser-0.28/geiser-pkg ~/env/elisp/ol-notmuch hides /home/matt/.config/emacs/elpa/ol-notmuch-2.0.0/ol-notmuch /home/matt/scratch/geiser-repro/.emacs.d/elpa/transient-0.3.7/transient hides /home/matt/.config/emacs/elpa/transient-0.3.7/transient /home/matt/scratch/geiser-repro/.emacs.d/elpa/transient-0.3.7/transient-autoloads hides /home/matt/.config/emacs/elpa/transient-0.3.7/transient-autoloads /home/matt/scratch/geiser-repro/.emacs.d/elpa/transient-0.3.7/transient-pkg hides /home/matt/.config/emacs/elpa/transient-0.3.7/transient-pkg /home/matt/scratch/geiser-repro/.emacs.d/elpa/transient-0.3.7/transient hides /home/matt/git/e/daily-driver/lisp/transient Features: (shadow emacsbug semantic/symref/grep grep semantic/symref semantic/util-modes semantic/util semantic semantic/tag semantic/lex semantic/fw mode-local cedet disass shfmt reformatter package-x ielm cl-print geiser-reload geiser-mode geiser-xref geiser-compile hacroexp shortdoc sh-script executable files-x dabbrev pulse misearch multi-isearch textsec uni-scripts idna-mapping ucs-normalize uni-confusable textsec-check sort company-oddmuse company-keywords company-etags company-gtags company-dabbrev-code company-dabbrev company-files company-capf company-cmake company-xcode company-clang company-semantic company-eclim company-template company-bbdb company mail-extr whitespace magit-patch magit-subtree magit-gitignore magit-ediff ediff ediff-merg ediff-mult ediff-wind ediff-diff ediff-help ediff-init ediff-util bug-reference copyright markdown-mode color vc-hg vc-git vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs log-view vc vc-dispatcher go-mode find-file eglot array jsonrpc ert pp ewoc flymake-proc flymake cap-words superword subword my llvm-c-style google-c-style cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs magit-extras face-remap magit-submodule magit-obsolete magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit magit-sequence magit-notes magit-worktree magit-tag magit-merge magit-branch magit-reset magit-files magit-refs magit-status magit magit-repos magit-apply magit-wip magit-log which-func imenu edebug debug backtrace magit-diff git-commit log-edit pcvs-util add-log magit-core magit-autorevert autorevert magit-margin magit-transient magit-process with-editor magit-mode magit-git magit-utils dired-aux protbuf msb time smerge-mode diff mm-archive mule-util notmuch notmuch-tree notmuch-jump notmuch-hello notmuch-show notmuch-print notmuch-crypto notmuch-mua notmuch-message notmuch-draft notmuch-maildir-fcc notmuch-address notmuch-company notmuch-parser notmuch-wash diff-mode coolj goto-addr icalendar diary-lib diary-loaddefs notmuch-tag crm notmuch-lib notmuch-compat hl-line flyspell ispell org-element avl-tree ol-w3m ol-rmail ol-mhe ol-irc ol-info org-habit org-agenda org-refile ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum gnus-group gnus-undo gnus-start gnus-dbus dbus gnus-cloud nnimap nnmail mail-source utf7 nnoo parse-time gnus-spec gnus-int gnus-range message sendmail yank-media rfc822 mml mml-sec epa derived epg rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus-win ol-eww eww xdg url-queue shr pixel-fill kinsoku url-file svg xml dom puny mm-url gnus nnheader gnus-util mail-utils range wid-edit mm-util mail-prsvr ol-doi org-link-doi ol-docview doc-view filenotify jka-compr image-mode exif dired dired-loaddefs ol-bibtex ol-bbdb midnight tab-line server geiser-guile info-look geiser-debug transient geiser-repl compile text-property-search geiser-image geiser-capf geiser-doc geiser-menu geiser-autodoc geiser-edit etags fileloop generator xref geiser-completion geiser-eval geiser-connection tq geiser-syntax scheme geiser-impl help-fns radix-tree geiser-log geiser-popup view geiser-custom geiser-base geiser web-mode disp-table nix-mode ffap thingatpt smie nix-repl nix-shell nix-store magit-section dash nix-log nix-instantiate nix-shebang nix-format nix finder-inf dirtrack ob-shell shell ob-ruby ob-python python pcase treesit ob-dot org-protocol org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete pcomplete comint ansi-osc ansi-color ring org-list org-faces org-entities noutline outline org-version ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex iso8601 time-date org-keys oc org-loaddefs find-func cal-menu calendar cal-loaddefs ol-notmuch ol rx org-compat org-macs format-spec skeleton autoinsert advice keyfreq project edmacro kmacro warnings icons savehist icomplete editorconfig editorconfig-core editorconfig-core-handle editorconfig-fnmatch which-key package browse-url url url-proxy url-privacy url-expand url-methods url-history url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse auth-source eieio eieio-core password-cache json subr-x map byte-opt url-vars cl-extra help-mode cl-macs gv cl-seq elec-pair use-package use-package-ensure use-package-delight use-package-diminish use-package-bind-key bind-key easy-mmode use-package-core cl-loaddefs cl-lib bytecomp byte-compile info geiser-guile-autoloads geiser-autoloads nix-mode-autoloads graphviz-dot-mode-autoloads markdown-mode-autoloads org-drill-autoloads persist-autoloads ox-hugo-autoloads tomelr-autoloads ol-notmuch-autoloads notmuch-autoloads pylint-autoloads elpy-autoloads highlight-indentation-autoloads pyvenv-autoloads s-autoloads flymake-ruby-autoloads flymake-easy-autoloads go-mode-autoloads yasnippet-autoloads company-autoloads shfmt-autoloads flymake-yamllint-autoloads yaml-mode-autoloads debbugs-autoloads bazel-autoloads clang-format+-autoloads clang-format-autoloads google-c-style-autoloads cmake-mode-autoloads meson-mode-autoloads d-mode-autoloads magit-autoloads git-commit-autoloads transient-autoloads magit-section-autoloads dash-autoloads with-editor-autoloads vertico-autoloads orderless-autoloads editorconfig-autoloads which-key-autoloads exec-path-from-shell-autoloads use-package-autoloads bind-key-autoloads envrc-autoloads inheritenv-autoloads web-mode-autoloads nixpkgs-fmt-autoloads reformatter-autoloads rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/pgtk-win pgtk-win term/common-win pgtk-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic indonesian philippine cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese composite emoji-zwj charscript charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget keymap hashtable-print-readable backquote threads dbusbind inotify dynamic-setting system-font-setting font-render-setting cairo gtk pgtk lcms2 multi-tty make-network-process emacs) Memory information: ((conses 16 822106 331203) (symbols 48 49001 252) (strings 32 256796 20096) (string-bytes 1 7820674) (vectors 16 119792) (vector-slots 8 2159467 271935) (floats 8 588 744) (intervals 56 4125 4439) (buffers 984 34)) ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#59707: 29.0.50; Seeking a more robust `package-quickstart' 2022-11-30 0:14 bug#59707: 29.0.50; Seeking a more robust `package-quickstart' Matt Armstrong @ 2022-11-30 2:37 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-11-30 5:59 ` Matt Armstrong 0 siblings, 1 reply; 5+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-11-30 2:37 UTC (permalink / raw) To: Matt Armstrong; +Cc: 59707 > With this, emacs fails to launch, instead printing: > > Symbol’s value as variable is void: geiser-active-implementations Did `--debug-init` provide a backtrace? > The code for `geiser-activate-implementation' was originally: > > (defsubst geiser-activate-implementation (impl) > (add-to-list 'geiser-active-implementations impl)) > > So the `(add-to-list 'geiser-active-implementations 'guile)' ended up > inlined into "package-quickstart.elc", which executed directly, without > first (auto) loading 'geiser-impl' as was intended. Thus, the > `geiser-active-implementations' variable did not exist, yet was > referenced, by inlining, but did not appear textually in > package-quickstart.el. > > Once understood obvious fix to make geiser-activate-implementation a > defun instead of a defsubst. FWIW, that's a rather poor fix, since it causes `geiser-impl.el` (and the files it requires) to be unconditionally loaded during Emacs startup. The better fix would be to make sure the `geiser-active-implementations` itself gets initialized before the calls to `geiser-activate-implementation`. > Ideas for improvement (also seeking ideas from others) > ======================================== > > 0) Make `package-quickstart t' the default, because it is great, and > that way people tend to see the same kinds of problems. :-) I think it's a good idea to do that on `master`, indeed. > 1) Harden Emacs such that signaled errors from "package-quickstart.elc" > don't prevent startup (but are somehow saved and logged, maybe as > warnings?). Agreed. I suspect it should also do things like delete the `.elc` file (and/or the `.el` file), or at least suggest doing it, so as to help diagnose/circumvent the problem. > 2) Make --debug-init functional for errors thrown by > "package-quickstart.elc". Currently, package-quickstart is loaded > before the first frame is initialized, so Emacs takes the "print the > error and exit" route for any signaled errors, instead of, for example, > ending up in the elisp debugger. In the case of `--debug-init` it could at least print the backtrace on `stderr`. Or store the backtrace and display it later (even though the debugger wouldn't be active, it would still be nicer to manipulate than when sent to stderr). > 3) Perhaps package-quickstart.elc could be loaded later in the > initialization phase, after the first frame is created? I'm not in favor of such a change, no. > 5) Change "package-quickstart.el" code gen to provide more context for > errors, perhaps with `condition-case-unless-debug'. The jwigley's > use-package package does something similar in its macro expansions and > it is nice. In the specific case with the geiser packages, because > Emacs exited immediately, it would have been useful to know that the > error came from the "geiser-guile-autloads" portion of > "package-quickstart.elc". We could probably use `(setq load-file-name ...)` between file chunks (instead of `let` binding the var) and wrap the whole file with a `condition-case` which then prints the "current" `load-file-name`. > 6) Print a warning when a `defsubst' function is autoloaded, as the > autoload won't work from byte compiled code. It works. It just works differently (it copies the whole `defsubst` instead of placing an `autoload` statement). AFAIK byte-compiled or not doesn't make much difference in this respect. Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#59707: 29.0.50; Seeking a more robust `package-quickstart' 2022-11-30 2:37 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-11-30 5:59 ` Matt Armstrong 2022-11-30 13:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-11-30 19:13 ` Matt Armstrong 0 siblings, 2 replies; 5+ messages in thread From: Matt Armstrong @ 2022-11-30 5:59 UTC (permalink / raw) To: Stefan Monnier; +Cc: 59707 Stefan Monnier <monnier@iro.umontreal.ca> writes: >> With this, emacs fails to launch, instead printing: >> >> Symbol’s value as variable is void: geiser-active-implementations > > Did `--debug-init` provide a backtrace? --debug-init did nothing insofar as I could tell. Emacs behavior did not observably change. It prints the following to stderr and exits with 255: Symbol’s value as variable is void: geiser-active-implementations In gdb I end up in a call to Fcommand_error_default_function with SELECTED_FRAME()->glyphs_initialized_p false, so it takes the Fkill_emacs(-1) path. The trace below is easy for me to repro so feel free to ask me to root around. #0 __GI_exit (status=-1) at ./stdlib/exit.c:143 #1 0x000055555567ffab in Fkill_emacs (arg=Python Exception <class 'gdb.error'>: value has been optimized out , arg@entry=make_fixnum(-1), restart=XIL(0)) at emacs.c:2916 #2 0x00005555556884f1 in Fcommand_error_default_function (data=Python Exception <class 'gdb.error'>: value has been optimized out , context=Python Exception <class 'gdb.error'>: value has been optimized out , signal=XIL(0x2aaa9b653e40)) at /home/matt/git/e/emacs/src/lisp.h:758 #3 0x0000555555719217 in funcall_subr (subr=0x555555dcf620 <Scommand_error_default_function>, numargs=numargs@entry=3, args=args@entry=0x7ffff0dff050) at eval.c:3026 #4 0x0000555555756f03 in exec_byte_code (fun=Python Exception <class 'gdb.error'>: value has been optimized out , fun@entry=XIL(0x7ffff18fdfc5), args_template=args_template@entry=771, nargs=<optimized out>, nargs@entry=3, args=<optimized out>, args@entry=0x7fffffffda18) at bytecode.c:809 #5 0x0000555555718aa6 in fetch_and_exec_byte_code (fun=fun@entry=XIL(0x7ffff18fdfc5), args_template=771, nargs=nargs@entry=3, args=args@entry=0x7fffffffda18) at eval.c:3069 #6 0x000055555571a165 in funcall_lambda (fun=fun@entry=XIL(0x7ffff18fdfc5), nargs=nargs@entry=3, arg_vector=arg_vector@entry=0x7fffffffda18) at eval.c:3141 #7 0x000055555571a593 in funcall_general (fun=XIL(0x7ffff18fdfc5), numargs=numargs@entry=3, args=args@entry=0x7fffffffda18) at eval.c:2933 #8 0x000055555571707c in Ffuncall (nargs=nargs@entry=4, args=args@entry=0x7fffffffda10) at eval.c:2983 #9 0x0000555555682726 in call3 (fn=Python Exception <class 'gdb.error'>: value has been optimized out , arg1=Python Exception <class 'gdb.error'>: value has been optimized out , arg1@entry=XIL(0x555555f74ef3), arg2=Python Exception <class 'gdb.error'>: value has been optimized out , arg3=Python Exception <class 'gdb.error'>: value has been optimized out ) at /home/matt/git/e/emacs/src/lisp.h:3256 #10 0x0000555555689328 in cmd_error_internal (data=data@entry=XIL(0x555555f74ef3), context=context@entry=0x7fffffffda60 "") at keyboard.c:1005 #11 0x000055555568952e in cmd_error (data=XIL(0x555555f74ef3)) at keyboard.c:973 #12 0x0000555555715df3 in internal_condition_case (bfun=bfun@entry=0x5555556826c1 <top_level_2>, handlers=Python Exception <class 'gdb.error'>: value has been optimized out , hfun=hfun@entry=0x555555689363 <cmd_error>) at eval.c:1470 #13 0x00005555556825a1 in top_level_1 (ignore=Python Exception <class 'gdb.error'>: value has been optimized out , ignore@entry=XIL(0)) at keyboard.c:1142 #14 0x0000555555715d43 in internal_catch (tag=Python Exception <class 'gdb.error'>: value has been optimized out , func=func@entry=0x55555568256b <top_level_1>, arg=Python Exception <class 'gdb.error'>: value has been optimized out , arg@entry=XIL(0)) at eval.c:1197 #15 0x00005555556824d7 in command_loop () at keyboard.c:1102 #16 0x0000555555688e10 in recursive_edit_1 () at keyboard.c:712 #17 0x000055555568925c in Frecursive_edit () at keyboard.c:795 #18 0x0000555555681255 in main (argc=4, argv=<optimized out>) at emacs.c:2517 Lisp Backtrace: "command-error-default-function" (0xf0dff050) "help-command-error-confusable-suggestions" (0xffffda18) >> The code for `geiser-activate-implementation' was originally: >> >> (defsubst geiser-activate-implementation (impl) >> (add-to-list 'geiser-active-implementations impl)) >> >> So the `(add-to-list 'geiser-active-implementations 'guile)' ended up >> inlined into "package-quickstart.elc", which executed directly, without >> first (auto) loading 'geiser-impl' as was intended. Thus, the >> `geiser-active-implementations' variable did not exist, yet was >> referenced, by inlining, but did not appear textually in >> package-quickstart.el. >> >> Once understood obvious fix to make geiser-activate-implementation a >> defun instead of a defsubst. > > FWIW, that's a rather poor fix, since it causes `geiser-impl.el` (and > the files it requires) to be unconditionally loaded during > Emacs startup. The better fix would be to make sure the > `geiser-active-implementations` itself gets initialized before the calls > to `geiser-activate-implementation`. Yes, I agree that the fix is suboptimal -- it was the smallest diff that made the code do what it clearly intended to do. I'm not sure I like your suggestion to keep using the `geiser-activeate-implementation' function, because I don't think it is because calling functions from "foo" without loading "foo" first is confusing, and works only if they are defsubst (or macros). Is this kind of thing commonly done in autoloads? Seems quite subtle. I'd rather suggest that the gesier package autoloads simply not use the helper function at all. Instead, just have the autoloads defvar `geiser-active-implementations' and call `add-to-list' on it explicitly. Seems simpler and unsurprising. >> Ideas for improvement (also seeking ideas from others) >> ======================================== >> >> 0) Make `package-quickstart t' the default, because it is great, and >> that way people tend to see the same kinds of problems. :-) > > I think it's a good idea to do that on `master`, indeed. Ok. >> 1) Harden Emacs such that signaled errors from "package-quickstart.elc" >> don't prevent startup (but are somehow saved and logged, maybe as >> warnings?). > > Agreed. I suspect it should also do things like delete the `.elc` file > (and/or the `.el` file), or at least suggest doing it, so as to help > diagnose/circumvent the problem. Ok, let's keep it on the list of possibilities. >> 2) Make --debug-init functional for errors thrown by >> "package-quickstart.elc". Currently, package-quickstart is loaded >> before the first frame is initialized, so Emacs takes the "print the >> error and exit" route for any signaled errors, instead of, for example, >> ending up in the elisp debugger. > > In the case of `--debug-init` it could at least print the backtrace on > `stderr`. Or store the backtrace and display it later (even though the > debugger wouldn't be active, it would still be nicer to manipulate than > when sent to stderr). Yes, though see the backtrace above. It isn't very helpful, at least for me, since the bottom of the lisp stack is the current `command-error-function'. Is this because the stack has already been unwound somehow? >> 3) Perhaps package-quickstart.elc could be loaded later in the >> initialization phase, after the first frame is created? > > I'm not in favor of such a change, no. > >> 5) Change "package-quickstart.el" code gen to provide more context for >> errors, perhaps with `condition-case-unless-debug'. The jwigley's >> use-package package does something similar in its macro expansions and >> it is nice. In the specific case with the geiser packages, because >> Emacs exited immediately, it would have been useful to know that the >> error came from the "geiser-guile-autloads" portion of >> "package-quickstart.elc". > > We could probably use `(setq load-file-name ...)` between file chunks > (instead of `let` binding the var) and wrap the whole file with > a `condition-case` which then prints the "current" `load-file-name`. ...adding to the list. :) >> 6) Print a warning when a `defsubst' function is autoloaded, as the >> autoload won't work from byte compiled code. > > It works. It just works differently (it copies the whole `defsubst` > instead of placing an `autoload` statement). AFAIK byte-compiled or > not doesn't make much difference in this respect. Ah, ok. It looks like geiser's autoloads are hand written (and might predate package.el), so an actual autoload for the defsubst ends up in package-quickstart.el. See https://gitlab.com/emacs-geiser/geiser/-/blob/master/elisp/geiser.el#L104 So, there is an extra level of indirection there compared to what presumably a "modern" emacs package would do. Yet, geiser wants to support use directly from downloaded source. What is our current suggestion for packages that want to ship their own pre-canned autoloads? Do you know of a package supporting this without resorting to hand edited autoloads? I assume there is some canonical 'emacs -f batch_blah_blah' but, maybe not? ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#59707: 29.0.50; Seeking a more robust `package-quickstart' 2022-11-30 5:59 ` Matt Armstrong @ 2022-11-30 13:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-11-30 19:13 ` Matt Armstrong 1 sibling, 0 replies; 5+ messages in thread From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-11-30 13:11 UTC (permalink / raw) To: Matt Armstrong; +Cc: 59707 > I'd rather suggest that the gesier package autoloads simply not use the > helper function at all. Instead, just have the autoloads defvar > `geiser-active-implementations' and call `add-to-list' on it explicitly. > Seems simpler and unsurprising. But that exposes the implementation of `geiser-activate-implementation`. Copying the whole function definition to the autoloads file is "cleaner" in this sense. >> In the case of `--debug-init` it could at least print the backtrace on >> `stderr`. Or store the backtrace and display it later (even though the >> debugger wouldn't be active, it would still be nicer to manipulate than >> when sent to stderr). > > Yes, though see the backtrace above. It isn't very helpful, at least > for me, since the bottom of the lisp stack is the current > `command-error-function'. The GDB backtrace you got it from a time after the actual Lisp error was caught, so you didn't get the relevant Lisp backtrace. If you put a breakpoint on something like `Fsignal` you'd get something closer. This said, it's still the case that the backtrace wouldn't show very much info because the autoloads file is very "flat". > Ah, ok. It looks like geiser's autoloads are hand written (and might > predate package.el), so an actual autoload for the defsubst ends up in > package-quickstart.el. See > https://gitlab.com/emacs-geiser/geiser/-/blob/master/elisp/geiser.el#L104 Ah! :-( > Yet, geiser wants to support use directly from downloaded source. What > is our current suggestion for packages that want to ship their own > pre-canned autoloads? Don't do that. Help us improve `package-vc` instead :-) > Do you know of a package supporting this without resorting to hand > edited autoloads? Most largish packages do, sadly. > I assume there is some canonical 'emacs -f batch_blah_blah' but, > maybe not? Kind of, but it's not very widely advertized (nor defined well enough to be convenient to use) so it's a bit cumbersome and the actual incantation used varies. E.g. Hyperbole uses things like: $(EMACS_BATCH) --debug --eval "(progn (setq generated-autoload-file (expand-file-name \"kotl/kotl-autoloads.el\") backup-inhibited t) (let (find-file-hooks) (hload-path--make-directory-autoloads \"kotl/\" generated-autoload-file)))" Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#59707: 29.0.50; Seeking a more robust `package-quickstart' 2022-11-30 5:59 ` Matt Armstrong 2022-11-30 13:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-11-30 19:13 ` Matt Armstrong 1 sibling, 0 replies; 5+ messages in thread From: Matt Armstrong @ 2022-11-30 19:13 UTC (permalink / raw) To: Stefan Monnier; +Cc: 59707 [-- Attachment #1: Type: text/plain, Size: 1181 bytes --] Matt Armstrong <matt@rfc20.org> writes: >>> 1) Harden Emacs such that signaled errors from >>> "package-quickstart.elc" don't prevent startup (but are somehow >>> saved and logged, maybe as warnings?). >> >> Agreed. I suspect it should also do things like delete the `.elc` >> file (and/or the `.el` file), or at least suggest doing it, so as to >> help diagnose/circumvent the problem. > > Ok, let's keep it on the list of possibilities. This (attached) approach seems to work in my manual testing. If you like the general idea I can polish it off (try to make a test, etc.). One thing that bothers me: because I fall back to `package--activate-all' even package that successfully activate in the quickstart file can have their activation code run twice, and they aren't necessarily idempotent operations. Do you think this is a significant problem? One possibility is to update `package-activated-list' and `Info-directory-list' incrementally in the quickstart file, so any signaled errors leave `package-activated-list' mostly-correct. This way, `package--activate-all' will attempt to activate only one package twice -- the one that signaled from the quickstart file. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Make-package-activate-all-resilient-to-quickload-err.patch --] [-- Type: text/x-diff, Size: 2451 bytes --] From 0ec2a7408eb248eef21c0a431eaf9c23cd5e99d3 Mon Sep 17 00:00:00 2001 From: Matt Armstrong <matt@rfc20.org> Date: Wed, 30 Nov 2022 10:56:59 -0800 Subject: [PATCH] Make `package-activate-all' resilient to quickload errors. If `package-activate-all' fails Emacs fails to start, so if quickloading fails fall back to per-package activation. Note that per-package activation already has logic to report package level actiation errors with `message' and continue. --- lisp/emacs-lisp/package.el | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el index 8d44fae30a..95921256d6 100644 --- a/lisp/emacs-lisp/package.el +++ b/lisp/emacs-lisp/package.el @@ -1714,16 +1714,23 @@ package-activate-all package-quickstart-file)))) ;; The quickstart file presumes that it has a blank slate, ;; so don't use it if we already activated some packages. - (if (and qs (not (bound-and-true-p package-activated-list))) - ;; Skip load-source-file-function which would slow us down by a factor - ;; 2 when loading the .el file (this assumes we were careful to - ;; save this file so it doesn't need any decoding). - (let ((load-source-file-function nil)) - (unless (boundp 'package-activated-list) - (setq package-activated-list nil)) - (load qs nil 'nomessage)) - (require 'package) - (package--activate-all))))) + (or (and qs (not (bound-and-true-p package-activated-list)) + ;; Skip load-source-file-function which would slow us + ;; down by a factor 2 when loading the .el file (this + ;; assumes we were careful to save this file so it + ;; doesn't need any decoding). + (let ((load-source-file-function nil)) + (unless (boundp 'package-activated-list) + (setq package-activated-list nil)) + (condition-case err + (load qs nil 'nomessage) + ;; If quickstart activation fails fall through to + ;; `package--activate-all' activation. + (error (message "Error loading %s: %s" + qs (error-message-string err)))))) + (progn + (require 'package) + (package--activate-all)))))) ;;;###autoload (defun package--activate-all () -- 2.35.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-11-30 19:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-30 0:14 bug#59707: 29.0.50; Seeking a more robust `package-quickstart' Matt Armstrong 2022-11-30 2:37 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-11-30 5:59 ` Matt Armstrong 2022-11-30 13:11 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-11-30 19:13 ` Matt Armstrong
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).