bug#47458: Terrible UX upgrading Emacs in Guix

bug#47458: Terrible UX upgrading Emacs in Guix

[Mark H Weaver]

I just updated my Guix system, which included the Emacs update from 27.1
to 27.2.  After "guix package -m mhw-manifest.scm" finished running
(which takes a long time for me, since I don't use substitutes), and
before I even noticed that it had finished, my existing Emacs session
started misbehaving badly.

It failed to even open a plain text file in fundamental mode (a .drv
file) with an inscrutible error about 'arrayp'.  I tried to enable the
debugger with M-x toggle-debug-on-error, but then I started getting
errors about 'debug' not found.  (I neglected to record the exact
messages).

I tried running "emacs -Q", and the same errors happened there too.  I
tried running "emacs -Q" from my root shell on a Linux text terminal,
and the same errors happened there.

I resorted to using 'vi' (which thankfully I had, and still remember how
to use for basic editing) to revert the emacs update on my private
branch and to rebuild my user profiles.

Eventually, I realized what the problem was:

(1) My existing emacs session started failing because
    ~/.guix-profile/share/emacs/27.1 had disappeared out from under it.

(2) My newly launched emacs sessions were failing because my
    EMACSLOADPATH variable was still set to its old value, pointing at
    /home/mhw/.guix-profile/share/emacs/27.1/lisp, which no longer
    existed.

I'm not sure why I've never run into this problem before.  I'm also not
sure what can be done to make this better, but if anyone has ideas, that
would be good.  If a 7+ year Guix veteran developer gets bitten badly by
this, I doubt that less experienced users will be impressed.

Any ideas?

       Mark

bug#47458: Terrible UX upgrading Emacs in Guix

[Leo Prikler]

Am Sonntag, den 28.03.2021, 22:02 -0400 schrieb Mark H Weaver:
> I just updated my Guix system, which included the Emacs update from
> 27.1
> to 27.2.  After "guix package -m mhw-manifest.scm" finished running
> (which takes a long time for me, since I don't use substitutes), and
> before I even noticed that it had finished, my existing Emacs session
> started misbehaving badly.
> 
> It failed to even open a plain text file in fundamental mode (a .drv
> file) with an inscrutible error about 'arrayp'.  I tried to enable
> the
> debugger with M-x toggle-debug-on-error, but then I started getting
> errors about 'debug' not found.  (I neglected to record the exact
> messages).
As you are probably aware by now, this is a result of parts of your
EMACSLOADPATH being deleted.  I don't think there's a good solution to
this, but you could (as part of your early init file) resolve the
symlinks in it, so that it behaves deterministically until it is
garbage collected?

> [...]
> 
> Eventually, I realized what the problem was:
> 
> (1) My existing emacs session started failing because
>     ~/.guix-profile/share/emacs/27.1 had disappeared out from under
> it.
> 
> (2) My newly launched emacs sessions were failing because my
>     EMACSLOADPATH variable was still set to its old value, pointing
> at
>     /home/mhw/.guix-profile/share/emacs/27.1/lisp, which no longer
>     existed.
> 
> I'm not sure why I've never run into this problem before.  I'm also
> not
> sure what can be done to make this better, but if anyone has ideas,
> that
> would be good.  If a 7+ year Guix veteran developer gets bitten badly
> by
> this, I doubt that less experienced users will be impressed.
Remember the snippet, that tells you you might want to recompute your
environment variables at the end of `guix package'?  Well, this is just
that.  I've already made it a habit to close Emacs at that point (and
you probably have as well), but as you said, you didn't even notice the
update succeed, so that's what lead to the confusion.

In a similar manner, if I see an Emacs version upgrade at the start of
the transaction, I already know to prepare for a little environment
variable dance to get it to start correctly.  I think there has been an
idea to update environment variables in GNOME Shell directly, for
instance, but
a. we're lacking the technology to do so at the moment (e.g. guile-
dbus)
b. it's not clear, that Guix itself should do such a thing rather than
relying on the user to e.g. code up a oneshot shepherd service 

Regards,
Leo

bug#47458: Terrible UX upgrading Emacs in Guix

[Maxime Devos]

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

On Mon, 2021-03-29 at 10:07 +0200, Leo Prikler wrote:
> [...]
> 
> In a similar manner, if I see an Emacs version upgrade at the start of
> the transaction, I already know to prepare for a little environment
> variable dance to get it to start correctly.  I think there has been an
> idea to update environment variables in GNOME Shell directly, for
> instance, but
> a. we're lacking the technology to do so at the moment (e.g. guile-
> dbus)

Actually, we do have dbus bindings for guile (actually, a reimplementation
of dbus in (Guile) Scheme): guile-ac-d-bus.  It doesn't support file descriptor
passing (at least on Guile, other Schemes may differ) though, but that's
probably not required for this.

Greetings,
Maxime.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#47458: Terrible UX upgrading Emacs in Guix

[Leo Prikler]

Am Montag, den 29.03.2021, 10:24 +0200 schrieb Maxime Devos:
> On Mon, 2021-03-29 at 10:07 +0200, Leo Prikler wrote:
> > [...]
> > 
> > In a similar manner, if I see an Emacs version upgrade at the start
> > of
> > the transaction, I already know to prepare for a little environment
> > variable dance to get it to start correctly.  I think there has
> > been an
> > idea to update environment variables in GNOME Shell directly, for
> > instance, but
> > a. we're lacking the technology to do so at the moment (e.g. guile-
> > dbus)
> 
> Actually, we do have dbus bindings for guile (actually, a
> reimplementation
> of dbus in (Guile) Scheme): guile-ac-d-bus.  It doesn't support file
> descriptor
> passing (at least on Guile, other Schemes may differ) though, but
> that's
> probably not required for this.
Thanks for pointing this out!  Now I can finally write Guile code to
update all those environment variables.  (Hopefully this lets us do
polkit in Guix as well.)

bug#47458: Terrible UX upgrading Emacs in Guix

[Maxim Cournoyer]

Hi Leo!

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> With this, the search path specification of EMACSLOADPATH does no longer
> depend on the version of Emacs, which should make upgrading major versions
> less painful.  See also:
> - <https://bugs.gnu.org/43627>
> - <https://bugs.gnu.org/47458>
>
> * gnu/packages/emacs.scm (emacs)[#:phases]: Add ‘wrap-load-path’.
> [native-search-path]<EMACSLOADPATH>: Do not search for builtin libraries.
> (emacs-next)[native-search-path]: Inherit from emacs.
> ---
>  gnu/packages/emacs.scm | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
> index 7447cfe33a..e12c489f8d 100644
> --- a/gnu/packages/emacs.scm
> +++ b/gnu/packages/emacs.scm
> @@ -201,6 +201,20 @@
>                  (car (find-files "bin" "^emacs-([0-9]+\\.)+[0-9]+$"))
>                  "bin/emacs")
>                 #t)))
> +         (add-after 'strip-double-wrap 'wrap-load-path
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (lisp-dirs (find-files (string-append out "/share/emacs")
> +                                           "^lisp$"
> +                                           #:directories? #t)))
> +               (for-each
> +                (lambda (prog)
> +                  (wrap-program prog
> +                    `("EMACSLOADPATH" suffix ,lisp-dirs)))
> +                (find-files (string-append out "/bin")
> +                            ;; versioned and unversioned emacs binaries
> +                            "^emacs(-[0-9]+(\\.[0-9]+)*)?$"))
> +               #t)))

Shouldn't we wrap all the binaries to be on the safe side?  Things such
as emacsclient probably ought to have EMACSLOADPATH set correctly, no?

>           (add-before 'reset-gzip-timestamps 'make-compressed-files-writable
>             ;; The 'reset-gzip-timestamps phase will throw a permission error
>             ;; if gzip files aren't writable then.  This phase is needed when
> @@ -255,9 +269,7 @@
>      (native-search-paths
>       (list (search-path-specification
>              (variable "EMACSLOADPATH")
> -            ;; The versioned entry is for the Emacs' builtin libraries.
> -            (files (list "share/emacs/site-lisp"
> -                         (string-append "share/emacs/" version "/lisp"))))
> +            (files '("share/emacs/site-lisp")))
>             (search-path-specification
>              (variable "INFOPATH")
>              (files '("share/info")))))
> @@ -294,18 +306,7 @@ languages.")
>             "0igjm9kwiswn2dpiy2k9xikbdfc7njs07ry48fqz70anljj8y7y3"))))
>        (native-inputs
>         `(("autoconf" ,autoconf)
> -         ,@(package-native-inputs emacs)))
> -      (native-search-paths
> -       (list (search-path-specification
> -              (variable "EMACSLOADPATH")
> -              ;; The versioned entry is for the Emacs' builtin libraries.
> -              (files (list "share/emacs/site-lisp"
> -                           (string-append "share/emacs/"
> -                                          (version-major+minor+point version)
> -                                          "/lisp"))))
> -             (search-path-specification
> -              (variable "INFOPATH")
> -              (files '("share/info"))))))))
> +         ,@(package-native-inputs emacs))))))
>  
>  (define-public emacs-next-pgtk
>    (let ((commit "ae18c8ec4f0ef37c8c9cda473770ff47e41291e2")

This makes sense, and can make it to master rather than core-updates,
which is neat.

Thank you :-)

Maxim

bug#47458: Terrible UX upgrading Emacs in Guix

[Leo Prikler]

Hi Maxim!

Am Sonntag, den 04.04.2021, 00:35 -0400 schrieb Maxim Cournoyer:
> Hi Leo!
> 
> Leo Prikler <leo.prikler@student.tugraz.at> writes:
> 
> > With this, the search path specification of EMACSLOADPATH does no
> > longer
> > depend on the version of Emacs, which should make upgrading major
> > versions
> > less painful.  See also:
> > - <https://bugs.gnu.org/43627>
> > - <https://bugs.gnu.org/47458>
> > 
> > * gnu/packages/emacs.scm (emacs)[#:phases]: Add ‘wrap-load-path’.
> > [native-search-path]<EMACSLOADPATH>: Do not search for builtin
> > libraries.
> > (emacs-next)[native-search-path]: Inherit from emacs.
> > ---
> >  gnu/packages/emacs.scm | 31 ++++++++++++++++---------------
> >  1 file changed, 16 insertions(+), 15 deletions(-)
> > 
> > diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
> > index 7447cfe33a..e12c489f8d 100644
> > --- a/gnu/packages/emacs.scm
> > +++ b/gnu/packages/emacs.scm
> > @@ -201,6 +201,20 @@
> >                  (car (find-files "bin" "^emacs-([0-9]+\\.)+[0-
> > 9]+$"))
> >                  "bin/emacs")
> >                 #t)))
> > +         (add-after 'strip-double-wrap 'wrap-load-path
> > +           (lambda* (#:key outputs #:allow-other-keys)
> > +             (let* ((out (assoc-ref outputs "out"))
> > +                    (lisp-dirs (find-files (string-append out
> > "/share/emacs")
> > +                                           "^lisp$"
> > +                                           #:directories? #t)))
> > +               (for-each
> > +                (lambda (prog)
> > +                  (wrap-program prog
> > +                    `("EMACSLOADPATH" suffix ,lisp-dirs)))
> > +                (find-files (string-append out "/bin")
> > +                            ;; versioned and unversioned emacs
> > binaries
> > +                            "^emacs(-[0-9]+(\\.[0-9]+)*)?$"))
> > +               #t)))
> 
> Shouldn't we wrap all the binaries to be on the safe side?  Things
> such
> as emacsclient probably ought to have EMACSLOADPATH set correctly,
> no?
The remaining binaries are
- emacsclient, which inherits its EMACSLOADPATH from the server it
connects to
- ctags, ebrowse and etags, which are helper binaries, that don't seem
to rely on EMACSLOADPATH at all.  (Or is there an indicator, that they
do?)
- .-real binaries, that should only be wrapped once.
We could relax the regex to include the upper two, but I don't think
it's necessary to do so.

> >           (add-before 'reset-gzip-timestamps 'make-compressed-
> > files-writable
> >             ;; The 'reset-gzip-timestamps phase will throw a
> > permission error
> >             ;; if gzip files aren't writable then.  This phase is
> > needed when
> > @@ -255,9 +269,7 @@
> >      (native-search-paths
> >       (list (search-path-specification
> >              (variable "EMACSLOADPATH")
> > -            ;; The versioned entry is for the Emacs' builtin
> > libraries.
> > -            (files (list "share/emacs/site-lisp"
> > -                         (string-append "share/emacs/" version
> > "/lisp"))))
> > +            (files '("share/emacs/site-lisp")))
> >             (search-path-specification
> >              (variable "INFOPATH")
> >              (files '("share/info")))))
> > @@ -294,18 +306,7 @@ languages.")
> >             "0igjm9kwiswn2dpiy2k9xikbdfc7njs07ry48fqz70anljj8y7y3")
> > )))
> >        (native-inputs
> >         `(("autoconf" ,autoconf)
> > -         ,@(package-native-inputs emacs)))
> > -      (native-search-paths
> > -       (list (search-path-specification
> > -              (variable "EMACSLOADPATH")
> > -              ;; The versioned entry is for the Emacs' builtin
> > libraries.
> > -              (files (list "share/emacs/site-lisp"
> > -                           (string-append "share/emacs/"
> > -                                          (version-
> > major+minor+point version)
> > -                                          "/lisp"))))
> > -             (search-path-specification
> > -              (variable "INFOPATH")
> > -              (files '("share/info"))))))))
> > +         ,@(package-native-inputs emacs))))))
> >  
> >  (define-public emacs-next-pgtk
> >    (let ((commit "ae18c8ec4f0ef37c8c9cda473770ff47e41291e2")
> 
> This makes sense, and can make it to master rather than core-updates,
> which is neat.
I'd like to avoid pushing this to master just yet, because we also have
changes in the Emacs build system to discuss and I don't want to cause
an "Emacs world" rebuild twice in a row.  That said, I'm including this
patch in wip-emacs with the plan to push to master or staging once
everything there is resolved.

Regards,
Leo

bug#47458: Terrible UX upgrading Emacs in Guix

[Maxim Cournoyer]

Hi Leo!

Leo Prikler <leo.prikler@student.tugraz.at> writes:

[...]

>> Shouldn't we wrap all the binaries to be on the safe side?  Things
>> such
>> as emacsclient probably ought to have EMACSLOADPATH set correctly,
>> no?
> The remaining binaries are
> - emacsclient, which inherits its EMACSLOADPATH from the server it
> connects to
> - ctags, ebrowse and etags, which are helper binaries, that don't seem
> to rely on EMACSLOADPATH at all.  (Or is there an indicator, that they
> do?)
> - .-real binaries, that should only be wrapped once.
> We could relax the regex to include the upper two, but I don't think
> it's necessary to do so.

OK, thanks for the explanation, it makes sense.

[...]

> I'd like to avoid pushing this to master just yet, because we also have
> changes in the Emacs build system to discuss and I don't want to cause
> an "Emacs world" rebuild twice in a row.  That said, I'm including this
> patch in wip-emacs with the plan to push to master or staging once
> everything there is resolved.

Sure!  Which changes do you have in mind?  Are they already on the
tracker for review?

Thank you,

Maxim

bug#47458: Terrible UX upgrading Emacs in Guix

[Leo Prikler]

Hi Maxim!

Am Dienstag, den 06.04.2021, 08:09 -0400 schrieb Maxim Cournoyer:
> Hi Leo!
> 
> Leo Prikler <leo.prikler@student.tugraz.at> writes:
> 
> [...]
> 
> > > Shouldn't we wrap all the binaries to be on the safe
> > > side?  Things
> > > such
> > > as emacsclient probably ought to have EMACSLOADPATH set
> > > correctly,
> > > no?
> > The remaining binaries are
> > - emacsclient, which inherits its EMACSLOADPATH from the server it
> > connects to
> > - ctags, ebrowse and etags, which are helper binaries, that don't
> > seem
> > to rely on EMACSLOADPATH at all.  (Or is there an indicator, that
> > they
> > do?)
> > - .-real binaries, that should only be wrapped once.
> > We could relax the regex to include the upper two, but I don't
> > think
> > it's necessary to do so.
> 
> OK, thanks for the explanation, it makes sense.
> 
Should I also document that (as a comment in the code) or is it
somewhat intuitive, that only Emacs is interested in these variables
(just EMACSLOADPATH currently, maybe also PATH later)?
> 
> > I'd like to avoid pushing this to master just yet, because we also
> > have
> > changes in the Emacs build system to discuss and I don't want to
> > cause
> > an "Emacs world" rebuild twice in a row.  That said, I'm including
> > this
> > patch in wip-emacs with the plan to push to master or staging once
> > everything there is resolved.
> 
> Sure!  Which changes do you have in mind?  Are they already on the
> tracker for review?
> 

I've sent some of the changes for emacs-build-system to 45316.  The
rest only lives on wip-emacs as of yet.  The first 5 patches on that
branch are the ones that include the big changes (well, four of them
anyway), one of which is not yet up to review as it results from the
combined fixes to 45316 and 47458, the rest are mostly small "fixup"
commits.

Regards,
Leo

bug#47458: Terrible UX upgrading Emacs in Guix

[Maxim Cournoyer]

Hi Leo!

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> Hi Maxim!
>
> Am Dienstag, den 06.04.2021, 08:09 -0400 schrieb Maxim Cournoyer:
>> Hi Leo!
>>
>> Leo Prikler <leo.prikler@student.tugraz.at> writes:
>>
>> [...]
>>
>> > > Shouldn't we wrap all the binaries to be on the safe
>> > > side?  Things
>> > > such
>> > > as emacsclient probably ought to have EMACSLOADPATH set
>> > > correctly,
>> > > no?
>> > The remaining binaries are
>> > - emacsclient, which inherits its EMACSLOADPATH from the server it
>> > connects to
>> > - ctags, ebrowse and etags, which are helper binaries, that don't
>> > seem
>> > to rely on EMACSLOADPATH at all.  (Or is there an indicator, that
>> > they
>> > do?)
>> > - .-real binaries, that should only be wrapped once.
>> > We could relax the regex to include the upper two, but I don't
>> > think
>> > it's necessary to do so.
>>
>> OK, thanks for the explanation, it makes sense.
>>
> Should I also document that (as a comment in the code) or is it
> somewhat intuitive, that only Emacs is interested in these variables
> (just EMACSLOADPATH currently, maybe also PATH later)?

It wouldn't hurt!  It was not obvious to me that emacsclient didn't need
it (I have no knowledge of emacsclient's internals).

>> > I'd like to avoid pushing this to master just yet, because we also
>> > have
>> > changes in the Emacs build system to discuss and I don't want to
>> > cause
>> > an "Emacs world" rebuild twice in a row.  That said, I'm including
>> > this
>> > patch in wip-emacs with the plan to push to master or staging once
>> > everything there is resolved.
>>
>> Sure!  Which changes do you have in mind?  Are they already on the
>> tracker for review?
>>
>
> I've sent some of the changes for emacs-build-system to 45316.  The
> rest only lives on wip-emacs as of yet.  The first 5 patches on that
> branch are the ones that include the big changes (well, four of them
> anyway), one of which is not yet up to review as it results from the
> combined fixes to 45316 and 47458, the rest are mostly small "fixup"
> commits.

OK.  I'll have a look.

Maxim

bug#47458: Terrible UX upgrading Emacs in Guix

[Maxim Cournoyer]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

This should be fixed with the recently merged changes from Leo.  See
commit 307a2d2e2a833c2e1f7a79f46e4c6945c618cd8c. Thank you!

Closing.

Maxim