[bug#30119] [PATCHv2] Add emacs-realgud and varia

[ludo@gnu.org (Ludovic Courtès)]

Hi Maxim,

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

> From 379cf143bb078c7785d104a41a762d6136f1508e Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Sat, 13 Jan 2018 17:54:18 -0500
> Subject: [PATCH 1/4] emacs-build-system: Add set-emacs-load-path phase.
>
> This generalizes the mechanism by which the Emacs dependencies are made visible,
> so that any build phase can make use of them.
>
> * guix/build/emacs-build-system.scm (%legacy-install-suffix): New variable.
> (%install-suffix): Redefine in terms of %legacy-install-suffix.
> (set-emacs-load-path): Add new phase used for dependency resolution.
> (build): Remove ad-hoc dependency discovery mechanism.
> (emacs-input->el-directory): Add new procedure.
> (emacs-inputs-el-directories): Use it.
> (package-name-version->elpa-name-version): Fix typo.
> (%standard-phases): Include the new `set-emacs-load-path' phase. Refactor to
> make the ordering of the phases clearer.
> * guix/build/emacs-utils.scm (emacs-byte-compile-directory): Remove the
> optional `dependency-dirs' argument, which is now obsoleted by the
> `set-emacs-load-path' phase.

Nice!  At first sight it looks good to me.  If you’ve checked on a
sample that Emacs packages still build fine, and if nobody replies in
the meantime, I’ll apply it in a day or two.

This will trigger on the order of 200 rebuilds per architecture, but
these are small packages, so I think it’s fine.

Nitpick:

>  (define (emacs-inputs-el-directories dirs)
>    "Build the list of Emacs Lisp directories from the Emacs package directory
>  DIRS."
> -  (append-map (lambda (d)
> -                (list (string-append d "/share/emacs/site-lisp")
> -                      (string-append d %install-suffix "/"
> -                                     (store-directory->elpa-name-version d))))
> -              dirs))
> +  (filter string? (map emacs-input->el-directory dirs)))

This can be written as:

  (filter-map emacs-input->el-directory dirs)

> From f76b5faee8b0752d1aae95b9df7a1e9e6d88bd08 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Sat, 13 Jan 2018 17:54:57 -0500
> Subject: [PATCH 2/4] emacs-build-system: Reinstate the check phase.
>
> * guix/build/emacs-build-system.scm (%standard-phases): Reinstate the check
> phase from the gnu-build-system.
> * guix/build-system/emacs.scm (emacs-build)[tests?]: But do not enable it by default.
> [parallel-tests?]: Add argument.

OK.

> From 50a671765b3d610e38f6e052a59b3eef316f4226 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Sun, 14 Jan 2018 22:38:20 -0500
> Subject: [PATCH 3/4] emacs-build-system: Work around issue 30611.
>
> This is a temporary workaround issue 30611, where substitute* crashes on
> files containing NUL characters.
>
> * guix/build/emacs-build-system.scm (patch-el-files): Filter out elisp files
> that contain NUL characters.

[...]

> +  ;; TODO: Remove after issue 30611 is fixed in master (see:
> +  ;; https://debbugs.gnu.org/30116).

Which number is correct?  :-)

I’m not convinced we need special treatment for this case directly in
emacs-build-system.  This has happened only once on 200+ packages, so I
would rather leave the special case in the package definition itself.

WDYT?

> From 1e4a28920b17f7a3bf3e34a999b29e0245233942 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Mon, 11 Dec 2017 00:07:57 -0500
> Subject: [PATCH 4/4] gnu: Add emacs-realgud.
>
> * gnu/packages/emacs.scm (emacs-test-simple, emacs-load-relative,
> emacs-loc-changes, emacs-realgud): New public variables.

LGTM.   However, there’s a tradition to add one package per commit, so
it would be great if you could split it and send updated patches.

Thank you!

Ludo’.