diff mbox

build: emacs: Search elisp files in "share/emacs/site-lisp".

Message ID 87d1ncatv8.fsf@gmail.com
State New
Headers show

Commit Message

Alex Kost June 20, 2016, 1:10 p.m. UTC
Ricardo found a problem while working on some emacs package that
requires EMMS: the elisp file(s) of this package is(are) not compiled.

This happens because currently emacs-build-system adds only
"share/emacs/site-lisp/guix.d/<package>" to emacs load-path when it
compiles elisp files, but some emacs packages (for example, emms) put
their files in "share/emacs/site-lisp", so this directory should also be
added.  The attached patch will do it.

(I added a copyright line for David because he forgot to do it in commit 578b96af69057883a2a49a34dd6fe261cb2f4e5c)

Comments

Ricardo Wurmus June 21, 2016, 7:29 p.m. UTC | #1
Alex Kost <alezost@gmail.com> writes:

> Ricardo found a problem while working on some emacs package that
> requires EMMS: the elisp file(s) of this package is(are) not compiled.
>
> This happens because currently emacs-build-system adds only
> "share/emacs/site-lisp/guix.d/<package>" to emacs load-path when it
> compiles elisp files, but some emacs packages (for example, emms) put
> their files in "share/emacs/site-lisp", so this directory should also be
> added.  The attached patch will do it.

Thank you for the patch!  This looks like the right thing to do.

Should we also (in a later patch) add a way to override or append to the
list of directories, e.g. via the “arguments“ field?

~~ Ricardo
Ludovic Courtès June 21, 2016, 8:41 p.m. UTC | #2
Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:

> Alex Kost <alezost@gmail.com> writes:
>
>> Ricardo found a problem while working on some emacs package that
>> requires EMMS: the elisp file(s) of this package is(are) not compiled.
>>
>> This happens because currently emacs-build-system adds only
>> "share/emacs/site-lisp/guix.d/<package>" to emacs load-path when it
>> compiles elisp files, but some emacs packages (for example, emms) put
>> their files in "share/emacs/site-lisp", so this directory should also be
>> added.  The attached patch will do it.
>
> Thank you for the patch!  This looks like the right thing to do.

+1

> Should we also (in a later patch) add a way to override or append to the
> list of directories, e.g. via the “arguments“ field?

FWIW I’m not convinced this is necessary, but we can always see later.

Thanks!

Ludo’.
Alex Kost June 22, 2016, 8:16 a.m. UTC | #3
Ludovic Courtès (2016-06-21 23:41 +0300) wrote:

> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:
>
>> Alex Kost <alezost@gmail.com> writes:
>>
>>> Ricardo found a problem while working on some emacs package that
>>> requires EMMS: the elisp file(s) of this package is(are) not compiled.
>>>
>>> This happens because currently emacs-build-system adds only
>>> "share/emacs/site-lisp/guix.d/<package>" to emacs load-path when it
>>> compiles elisp files, but some emacs packages (for example, emms) put
>>> their files in "share/emacs/site-lisp", so this directory should also be
>>> added.  The attached patch will do it.
>>
>> Thank you for the patch!  This looks like the right thing to do.
>
> +1

Pushed, thanks.

>> Should we also (in a later patch) add a way to override or append to the
>> list of directories, e.g. via the “arguments“ field?
>
> FWIW I’m not convinced this is necessary, but we can always see later.

I agree: we can add this feature later if it will be needed.  But currently
it is not needed, as all our emacs packages put *.el files either in
"share/emacs/site-lisp" or in "share/emacs/site-lisp/guix.d/<package>".
Ricardo Wurmus June 23, 2016, 5:30 a.m. UTC | #4
Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> skribis:
>
>> Alex Kost <alezost@gmail.com> writes:
>>
>>> Ricardo found a problem while working on some emacs package that
>>> requires EMMS: the elisp file(s) of this package is(are) not compiled.
>>>
>>> This happens because currently emacs-build-system adds only
>>> "share/emacs/site-lisp/guix.d/<package>" to emacs load-path when it
>>> compiles elisp files, but some emacs packages (for example, emms) put
>>> their files in "share/emacs/site-lisp", so this directory should also be
>>> added.  The attached patch will do it.
>>
>> Thank you for the patch!  This looks like the right thing to do.
>
> +1
>
>> Should we also (in a later patch) add a way to override or append to the
>> list of directories, e.g. via the “arguments“ field?
>
> FWIW I’m not convinced this is necessary, but we can always see later.

You are probably right.  I thought that packages like Lilypond install
their Elisp files elsewhere, but it’s actually “share/emacs/site-lisp”
as expected.

~~ Ricardo
diff mbox

Patch

From a63b787b545d5b9fcc2a2c76e9e102144fafa0e8 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Mon, 20 Jun 2016 15:49:04 +0300
Subject: [PATCH] build: emacs: Search elisp files in "share/emacs/site-lisp".

* guix/build/emacs-build-system.scm (emacs-inputs-el-directories):
Add ".../share/emacs/site-lisp" directory to the returned result as
elisp files can also be placed there.
---
 guix/build/emacs-build-system.scm | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index ab97001..44e8b0d 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -1,5 +1,7 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2015 Federico Beffa <beffa@fbengineering.ch>
+;;; Copyright © 2016 David Thompson <davet@gnu.org>
+;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -152,10 +154,11 @@  store in '.el' files."
 (define (emacs-inputs-el-directories dirs)
   "Build the list of Emacs Lisp directories from the Emacs package directory
 DIRS."
-  (map (lambda (d)
-         (string-append d %install-suffix "/"
-                        (store-directory->elpa-name-version d)))
-       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))
 
 (define (package-name-version->elpa-name-version name-ver)
   "Convert the Guix package NAME-VER to the corresponding ELPA name-version
-- 
2.8.3