bug#59707: 29.0.50; Seeking a more robust `package-quickstart'

bug#59707: 29.0.50; Seeking a more robust `package-quickstart'

[Matt Armstrong]

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))

bug#59707: 29.0.50; Seeking a more robust `package-quickstart'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> 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

bug#59707: 29.0.50; Seeking a more robust `package-quickstart'

[Matt Armstrong]

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?

bug#59707: 29.0.50; Seeking a more robust `package-quickstart'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> 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

bug#59707: 29.0.50; Seeking a more robust `package-quickstart'

[Matt Armstrong]

[-- 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