bug#34954: 26.1.92; eshell gets loaded twice

bug#34954: 26.1.92; eshell gets loaded twice

[Mauro Aranda]

Hello Emacs.
I was messing around with eshell hooks when I encountered this bug.
This behavior is present in a fairly recent build of emacs 26 and in
master as well.

Here's a recipe:
1) Start emacs: emacs -Q.
2) On *scratch* buffer, write the following expressions:
The function can be whatever indicates a duplicate (I keep it simple for
myself):
(defun eshell-loaded-msg () (insert "Eshell was loaded.\n"))
(add-hook 'eshell-load-hook 'eshell-loaded-msg)
3) M-x eval-buffer
4) M-x eshell
5) After Eshell gets loaded, type C-x b RET to go back to *scratch*
buffer.
6) Notice that "Eshell was loaded" appears twice.

My conclusion is that eshell gets loaded twice, becuase eshell.el
requires esh-mode, which requires esh-module, which in turn requires
eshell.  Since this require expression comes before the provide
expresion (provide 'eshell) in eshell.el, eshell gets loaded twice.

In GNU Emacs 26.1.92 (build 1, i686-pc-linux-gnu, GTK+ Version 3.18.9)
 of 2019-03-22 built on the-blackbeard
Repository revision: 24fc1337c169aeb1095dcc58bf8d4bbe48abefc0
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description:	Ubuntu 16.04.6 LTS

Recent messages:
Loading em-basic...done
Loading em-cmpl...done
Loading em-dirs...done
Loading em-glob...done
Loading em-hist...done
Loading em-ls...done
Loading em-prompt...done
Loading em-script...done
Loading em-term...done
Loading em-unix...done

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM THREADS LCMS2

Important settings:
  value of $LANG: en_US.utf8
  value of $XMODIFIERS:
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
mml-sec password-cache epa derived epg epg-config gnus-util rmail
rmail-loaddefs mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils em-unix em-term term disp-table easymenu
ehelp em-script em-prompt em-ls cl-loaddefs cl-lib em-hist em-pred
em-glob em-dirs em-cmpl em-basic em-banner em-alias pcomplete comint
ansi-color ring esh-var esh-io esh-cmd esh-opt esh-ext esh-proc esh-arg
esh-groups eshell esh-module esh-mode esh-util elec-pair time-date
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic 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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dbusbind
inotify lcms2 dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 8 111308 10030)
 (symbols 24 22085 1)
 (miscs 20 57 161)
 (strings 16 33578 1765)
 (string-bytes 1 938325)
 (vectors 12 17691)
 (vector-slots 4 564929 12642)
 (floats 8 56 61)
 (intervals 28 231 1)
 (buffers 536 12)
 (heap 1024 32162 939))

bug#34954: 26.1.92; eshell gets loaded twice

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 103 bytes --]

The attached patch fixes it for me, but maybe someone with more insight
can provide a better solution.

[-- Attachment #2: 0001-Avoid-recursive-load-of-eshell.patch --]
[-- Type: text/x-patch, Size: 1041 bytes --]

From 5474e7891678d0957a8552bc2761608ef71b1531 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Sat, 23 Mar 2019 11:38:14 -0300
Subject: [PATCH] Avoid recursive load of eshell

* lisp/eshell/eshell.el: Provide eshell before requiring esh-mode to
avoid a recursive load when esh-mode requires esh-module (which in
turn requires eshell).  (Bug #34954)
This bug shows up because 'eshell-load-hook' is run twice.
---
 lisp/eshell/eshell.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/eshell/eshell.el b/lisp/eshell/eshell.el
index 4516800..c7ed710 100644
--- a/lisp/eshell/eshell.el
+++ b/lisp/eshell/eshell.el
@@ -175,6 +175,9 @@
 (eval-when-compile
   (require 'cl-lib))
 (require 'esh-util)
+;; Provide eshell before requiring esh-mode, to avoid a recursive load.
+;; (Bug #34954)
+(provide 'eshell)
 (require 'esh-mode)
 
 (defgroup eshell nil
@@ -403,6 +406,4 @@ eshell-unload-all-modules
 
 (run-hooks 'eshell-load-hook)
 
-(provide 'eshell)
-
 ;;; eshell.el ends here
-- 
2.7.4

bug#34954: 26.1.92; eshell gets loaded twice

[Glenn Morris]

For the specific issue of the load-hook, see https://debbugs.gnu.org/24491#8

bug#34954: 26.1.92; eshell gets loaded twice

[Mauro Aranda]

[-- Attachment #1: Type: text/plain, Size: 1761 bytes --]

Sorry for the noise, I forgot to keep 34954@debbugs.gnu.org

Hello Glenn.

> For the specific issue of the load-hook, see
https://debbugs.gnu.org/24491#8

Sorry I missed that bug report.  Could both reports be merged?

> You ought to be able to avoid such problems by using
> (with-)eval-after-load instead of foo-load-hook, since the former runs
> after provide (effectively).

Your recommendation works, but only if the order is:
(require 'eshell)
(with-eval-after-load "eshell" (insert "Eshell was loaded.\n")

When the order is:
(with-eval-after-load "eshell" (insert "Eshell was loaded.\n")
(require 'eshell)

The string is inserted twice, because eshell is loaded twice.

Is the former order recommended over the latter? Mandatory?  I can't
find that information on the manual.

Of course, if I eval (starting from emacs -Q):
(with-eval-after-load 'eshell (insert "Eshell was loaded.\n")
And then type `M-x eshell' I get the repeated string inserted, because
again, eshell is loaded twice.

> Frankly all foo-load-hooks are probably historical baggage that should
> be obsoleted. There's no need for them to exist given that a general
> solution does.

Anyway, my recipe was meant to provide evidence that eshell gets loaded
twice.  That still happens, either eshell-load-hook or
with-eval-after-load is used in real code.  It would be good to solve
that double loading.  That way, either eshell-load-hook (until it is
obsoleted, if it eventually happens) or with-eval-after-load (any order
of execution) can be used.


Best Regards,
Mauro.

El mié., 27 mar. 2019 a las 9:31, Glenn Morris (<rgm@gnu.org>) escribió:

>
> For the specific issue of the load-hook, see
> https://debbugs.gnu.org/24491#8
>

[-- Attachment #2: Type: text/html, Size: 2540 bytes --]

bug#34954: 26.1.92; eshell gets loaded twice

[Noam Postavsky]

merge 34954 24491
tags 34954 fixed
close 34954 27.1
quit

Mauro Aranda <maurooaranda@gmail.com> writes:
>
> Anyway, my recipe was meant to provide evidence that eshell gets loaded
> twice.  That still happens, either eshell-load-hook or
> with-eval-after-load is used in real code.  It would be good to solve
> that double loading.  That way, either eshell-load-hook (until it is
> obsoleted, if it eventually happens) or with-eval-after-load (any order
> of execution) can be used.

Yep, makes sense, I've pushed your patch [1: 5e55b1b829].  To your
question about a better solution, I think it would require a major
reorganizing of eshell's code to avoid circular requires, and we already
have some prior art where some eshell have the provide at the top, so we
may as well go with what you posted.

[1: 5e55b1b829]: 2019-04-03 22:47:10 -0400
  Avoid recursive load of eshell
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=5e55b1b82952a03b704c464e8086d3c41e993a46