[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#30119] [PATCHv2] Add emacs-realgud and varia
From: |
Ludovic Courtès |
Subject: |
[bug#30119] [PATCHv2] Add emacs-realgud and varia |
Date: |
Tue, 30 Jan 2018 22:05:53 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Hi Maxim,
Maxim Cournoyer <address@hidden> skribis:
> From 379cf143bb078c7785d104a41a762d6136f1508e Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <address@hidden>
> 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 <address@hidden>
> 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 <address@hidden>
> 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 <address@hidden>
> 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’.