Hi Morgan,

Morgan.J.Smith@outlook.com writes:

> From: Morgan Smith <Morgan.J.Smith@outlook.com>
>
> * gnu/packages/emacs.scm (emacs):
> [strip-double-wrap] Modify to work with emacs-next
>
> * gnu/packages/emacs.scm (emacs-next):
> [version] Change version from 28.0.50.1 to 28.0.50
> [arguments] Removed field
> [native-search-paths] New field
> ---
>
> The problem turned out to be that EMACSLOADPATH wasn't being set
> properly.  This patch fixes that.

Nice catch!

This patch looks good to me overall, I'm not a maintainer but I can
review it. See my couple of comments inline.

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

Does that make sense? It might be just a matter of taste, I don't have a
super strong opinion on this.

>                            "bin/emacs")
>                 #t)))
>           (add-before 'reset-gzip-timestamps 'make-compressed-files-writable
> @@ -279,11 +280,10 @@ languages.")
>  
>  (define-public emacs-next
>    (let ((commit "2ea34662c20f71d35dd52a5ed996542c7386b9cb")
> -        (revision "0")
> -        (emacs-version "28.0.50.1"))
> +        (revision "0"))
>      (package/inherit emacs
>        (name "emacs-next")
> -      (version (git-version emacs-version revision commit))
> +      (version (git-version "28.0.50" revision commit))
>        (source
>         (origin
>           (inherit (package-source emacs))
> @@ -295,24 +295,19 @@ languages.")
>           (sha256
>            (base32
>             "0igjm9kwiswn2dpiy2k9xikbdfc7njs07ry48fqz70anljj8y7y3"))))
> -      (arguments
> -       (substitute-keyword-arguments (package-arguments emacs)
> -         ((#:phases phases)
> -          `(modify-phases ,phases
> -             (replace 'strip-double-wrap
> -               (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.
> -                 (with-directory-excursion (assoc-ref outputs "out")
> -                   (copy-file (string-append
> -                               "bin/emacs-"
> -                               ,(version-major+minor+point (package-version emacs-next)))
> -                              "bin/emacs")
> -                   #t)))))))
>        (native-inputs
>         `(("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.

> +             (search-path-specification
> +              (variable "INFOPATH")
> +              (files '("share/info"))))))))
>  
>  (define-public emacs-minimal
>    ;; This is the version that you should use as an input to packages that just


Thanks!
Pierre