diff mbox series

[bug#34187] gnu: emacs-ert-runner: Fix lisp directory warning.

Message ID 87pnsm4qrc.fsf@gmail.com
State Accepted
Headers show
Series [bug#34187] gnu: emacs-ert-runner: Fix lisp directory warning. | expand

Checks

Context Check Description
cbaines/applying patch success Successfully applied

Commit Message

Maxim Cournoyer Jan. 24, 2019, 4:35 a.m. UTC 
Hello!

This fixes an annoyance where a warning would be print every time
ert-runner was executed, like:

--8<---------------cut here---------------start------------->8---
ert-runner --version
Warning: Lisp directory '/tmp/guix-build-emacs-ert-runner-0.7.0.drv-0/ert-runner.el-0.7.0': No such file or directory
Option ‘--version‘ not available
--8<---------------cut here---------------end--------------->8---

Thank you,

Maxim

Comments

Ludovic Courtès Jan. 29, 2019, 10:36 p.m. UTC | #1 
Hello,

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

>>From 532f0dfeaa5f9aa7c03916afc843d5556a12f53c Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Tue, 22 Jan 2019 05:33:19 +0000
> Subject: [PATCH] gnu: emacs-ert-runner: Fix lisp directory warning.
>
> This fixes the warning message such as: "Warning: Lisp directory
> '/tmp/guix-build-emacs-ert-runner-0.7.0-1.90b8fdd.drv-0/source': No such file
> or directory" that would be printed upon running ert-runner.
>
> * gnu/packages/emacs-xyz.scm (emacs-ert-runner)[modules]: Add (srfi srfi-1) to
> the list of used modules.
> [phases]{install-executable}: Compute SOURCE-DRV and filter it out from the
> paths composing the EMACSLOADPATH variable used in the wrapper.

Good catch!

>                   (wrap-program (string-append out "/bin/ert-runner")
>                     (list "EMACSLOADPATH" ":" 'prefix
> -                         (string-split (getenv "EMACSLOADPATH") #\:)))
> +                         ;; Do not capture the transient source directory in
> +                         ;; the wrapper.
> +                         (remove (lambda (p)
> +                                   (string=? p source-drv))
> +                                 (string-split (getenv "EMACSLOADPATH") #\:))))

Or simply: (delete source-drv (string-split (getenv "EMACSLOADPATH") #\:))).

> +                     (source-drv (string-append
> +                                  "/tmp/guix-build-" ,name "-" ,version
> +                                  ".drv-0/source")))

Actually ‘source-drv’ is misleading because it’s not a derivation.  How
about ‘source’, ‘source-directory’, or something like that?

Instead of hardcoding /tmp/guix-build-*, you could just use (getcwd), if
I’m not mistaken.

OK with changes along these lines, thanks!

Ludo’.
Maxim Cournoyer Feb. 5, 2019, 3:11 p.m. UTC | #2 
Hello, and sorry for the delay!

Ludovic Courtès <ludo@gnu.org> writes:

> Hello,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>>>From 532f0dfeaa5f9aa7c03916afc843d5556a12f53c Mon Sep 17 00:00:00 2001
>> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> Date: Tue, 22 Jan 2019 05:33:19 +0000
>> Subject: [PATCH] gnu: emacs-ert-runner: Fix lisp directory warning.
>>
>> This fixes the warning message such as: "Warning: Lisp directory
>> '/tmp/guix-build-emacs-ert-runner-0.7.0-1.90b8fdd.drv-0/source': No such file
>> or directory" that would be printed upon running ert-runner.
>>
>> * gnu/packages/emacs-xyz.scm (emacs-ert-runner)[modules]: Add (srfi srfi-1) to
>> the list of used modules.
>> [phases]{install-executable}: Compute SOURCE-DRV and filter it out from the
>> paths composing the EMACSLOADPATH variable used in the wrapper.
>
> Good catch!
>
>>                   (wrap-program (string-append out "/bin/ert-runner")
>>                     (list "EMACSLOADPATH" ":" 'prefix
>> -                         (string-split (getenv "EMACSLOADPATH") #\:)))
>> +                         ;; Do not capture the transient source directory in
>> +                         ;; the wrapper.
>> +                         (remove (lambda (p)
>> +                                   (string=? p source-drv))
>> +                                 (string-split (getenv "EMACSLOADPATH") #\:))))
>
> Or simply: (delete source-drv (string-split (getenv "EMACSLOADPATH") #\:))).

This is much simpler, thanks!

>> +                     (source-drv (string-append
>> +                                  "/tmp/guix-build-" ,name "-" ,version
>> +                                  ".drv-0/source")))
>
> Actually ‘source-drv’ is misleading because it’s not a derivation.  How
> about ‘source’, ‘source-directory’, or something like that?

Right. I've settled for source-directory.

> Instead of hardcoding /tmp/guix-build-*, you could just use (getcwd), if
> I’m not mistaken.

(getcwd) wasn't in the tmp directory (it should be in the store IIRC,
since we byte-compile the files in-place after installing them), but
(getenv "TMPDIR") returned exactly what was needed.

> OK with changes along these lines, thanks!

The changes read as:

--8<---------------cut here---------------start------------->8---
modified   gnu/packages/emacs-xyz.scm
@@ -6955,18 +6955,13 @@ Emacs.")
          ("emacs-s" ,emacs-s)
          ("emacs-shut-up" ,emacs-shut-up)))
       (arguments
-       `(#:modules ((guix build emacs-build-system)
-                    (guix build emacs-utils)
-                    (guix build utils)
-                    (srfi srfi-1))      ;for `remove'
-         #:phases
+       `(#:phases
          (modify-phases %standard-phases
            (add-after 'install 'install-executable
              (lambda* (#:key inputs outputs #:allow-other-keys)
                (let ((out (assoc-ref outputs "out"))
-                     (source-drv (string-append
-                                  "/tmp/guix-build-" ,name "-" ,version
-                                  ".drv-0/source")))
+                     (source-directory (string-append
+                                  (getenv "TMPDIR") "/source")))
                  (substitute* "bin/ert-runner"
                    (("ERT_RUNNER=\"\\$\\(dirname \\$\\(dirname \\$0\\)\\)")
                     (string-append "ERT_RUNNER=\"" out
@@ -6977,8 +6972,7 @@ Emacs.")
                    (list "EMACSLOADPATH" ":" 'prefix
                          ;; Do not capture the transient source directory in
                          ;; the wrapper.
-                         (remove (lambda (p)
-                                   (string=? p source-drv))
+                         (delete source-directory
                                  (string-split (getenv "EMACSLOADPATH") #\:))))
                  #t))))
          #:include (cons* "^reporters/.*\\.el$" %default-include)))
--8<---------------cut here---------------end--------------->8---

And the commit message was also reworded.

Pushed to master with commit f9dff4f7f.

Thank you!

Maxim
diff mbox series

Patch

From 532f0dfeaa5f9aa7c03916afc843d5556a12f53c Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Tue, 22 Jan 2019 05:33:19 +0000
Subject: [PATCH] gnu: emacs-ert-runner: Fix lisp directory warning.

This fixes the warning message such as: "Warning: Lisp directory
'/tmp/guix-build-emacs-ert-runner-0.7.0-1.90b8fdd.drv-0/source': No such file
or directory" that would be printed upon running ert-runner.

* gnu/packages/emacs-xyz.scm (emacs-ert-runner)[modules]: Add (srfi srfi-1) to
the list of used modules.
[phases]{install-executable}: Compute SOURCE-DRV and filter it out from the
paths composing the EMACSLOADPATH variable used in the wrapper.
---
 gnu/packages/emacs-xyz.scm | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index c98798c8d..7bacf4234 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -6956,11 +6956,18 @@  Emacs.")
          ("emacs-s" ,emacs-s)
          ("emacs-shut-up" ,emacs-shut-up)))
       (arguments
-       `(#:phases
+       `(#:modules ((guix build emacs-build-system)
+                    (guix build emacs-utils)
+                    (guix build utils)
+                    (srfi srfi-1))      ;for `remove'
+         #:phases
          (modify-phases %standard-phases
            (add-after 'install 'install-executable
              (lambda* (#:key inputs outputs #:allow-other-keys)
-               (let ((out (assoc-ref outputs "out")))
+               (let ((out (assoc-ref outputs "out"))
+                     (source-drv (string-append
+                                  "/tmp/guix-build-" ,name "-" ,version
+                                  ".drv-0/source")))
                  (substitute* "bin/ert-runner"
                    (("ERT_RUNNER=\"\\$\\(dirname \\$\\(dirname \\$0\\)\\)")
                     (string-append "ERT_RUNNER=\"" out
@@ -6969,7 +6976,11 @@  Emacs.")
                  (install-file "bin/ert-runner" (string-append out "/bin"))
                  (wrap-program (string-append out "/bin/ert-runner")
                    (list "EMACSLOADPATH" ":" 'prefix
-                         (string-split (getenv "EMACSLOADPATH") #\:)))
+                         ;; Do not capture the transient source directory in
+                         ;; the wrapper.
+                         (remove (lambda (p)
+                                   (string=? p source-drv))
+                                 (string-split (getenv "EMACSLOADPATH") #\:))))
                  #t))))
          #:include (cons* "^reporters/.*\\.el$" %default-include)))
       (home-page "https://github.com/rejeep/ert-runner.el")
-- 
2.20.1