Subject: Re: bug#43277: [PATCH] gnu: emacs-next: Fix load path and version

From: Maxim Cournoyer <[email-masked]>
Date: Thu, 01 Oct 2020 11:05:46 -0400

Hello!

[...]

>>  gnu/packages/emacs.scm | 37 ++++++++++++++++---------------------
>>  1 file changed, 16 insertions(+), 21 deletions(-)
>>
>> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
>> index 03c28ee7a7..b3d099257d 100644
>> --- a/gnu/packages/emacs.scm
>> +++ b/gnu/packages/emacs.scm
>> @@ -196,11 +196,12 @@
>>             (lambda* (#:key outputs #:allow-other-keys)
>>               ;; Directly copy emacs-X.Y to emacs, so that it is not wrapped
>>               ;; twice.  This also fixes a minor issue, where WMs would not be
>> -             ;; able to track emacs back to emacs.desktop.
>> +             ;; able to track emacs back to emacs.desktop.  It's done using
>> +             ;; this-package so emacs-next can reuse it
>>               (with-directory-excursion (assoc-ref outputs "out")
>>                 (copy-file (string-append
>>                             "bin/emacs-"
>> -                           ,(version-major+minor (package-version emacs)))
>> +                           ,(car (string-split (package-version this-package) #\-)))
>
> I agree in general it's good to reuse code, however in this particular
> case it's probably better to keep the phases duplicated.  For example,
> in the future one could update the emacs-next package to not require a
> revision number anymore, and it's likely they'd forget to update the
> emacs package since it'll still work.

It's unlikely the emacs-next package would be pegged against a stable
version, but in the event it would, the above code would still work.

[...]

>>         `(("autoconf" ,autoconf)
>> -         ,@(package-native-inputs emacs))))))
>> +         ,@(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/" (car (string-split version #\-)) "/lisp"))))
>
> nit: This line seems to be a bit long.

Reformatted, and edited the commit message to match our standards.

I made minor, cosmetic changes like below:

modified   gnu/packages/emacs.scm
@@ -196,12 +196,16 @@
            (lambda* (#:key outputs #:allow-other-keys)
              ;; Directly copy emacs-X.Y to emacs, so that it is not wrapped
              ;; twice.  This also fixes a minor issue, where WMs would not be
-             ;; able to track emacs back to emacs.desktop.  It's done using
-             ;; this-package so emacs-next can reuse it
+             ;; able to track emacs back to emacs.desktop.  The version is
+             ;; accessed using using THIS-PACKAGE so it "just works" for
+             ;; inherited Emacs packages of different versions.
              (with-directory-excursion (assoc-ref outputs "out")
                (copy-file (string-append
                            "bin/emacs-"
-                           ,(car (string-split (package-version this-package) #\-)))
+                           ,(let ((this-version (package-version this-package)))
+                              (or (false-if-exception
+                                   (version-major+minor+point this-version))
+                                  (version-major+minor this-version))))
                           "bin/emacs")
                #t)))
          (add-before 'reset-gzip-timestamps 'make-compressed-files-writable
@@ -304,7 +308,9 @@ languages.")
               (variable "EMACSLOADPATH")
               ;; The versioned entry is for the Emacs' builtin libraries.
               (files (list "share/emacs/site-lisp"
-                           (string-append "share/emacs/" (car (string-split version #\-)) "/lisp"))))
+                           (string-append "share/emacs/"
+                                          (version-major+minor+point version)
+                                          "/lisp"))))
              (search-path-specification
               (variable "INFOPATH")
               (files '("share/info"))))))))

Verified it produced a correct EMACSLOADPATH and ran using:

--8<---------------cut here---------------start------------->8---
$ guix environment --pure --ad-hoc emacs-next
[...]
[env]$ echo $EMACSLOADPATH
/gnu/store/6s7p3yi969pm2xmkdd45dljbnwy5107g-profile/share/emacs/site-lisp:/gnu/store/6s7p3yi969pm2xmkdd45dljbnwy5107g-profile/share/emacs/28.0.50/

[env]$ emacs --version
GNU Emacs 28.0.50
--8<---------------cut here---------------end--------------->8---

And pushed to master as commit 0f88fea0eaa.

Thanks everyone!

Closing,

Maxim