diff mbox series

[bug#41790] Update emacs-direnv

Message ID 87eeql8xws.fsf@gmail.com
State Accepted
Headers show
Series [bug#41790] Update emacs-direnv | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

Katherine Cox-Buday June 11, 2020, 2:44 p.m. UTC 
Oleg Pykhalov <go.wigust@gmail.com> writes:

Heya Oleg, thanks for chiming in.

> Propagated inputs could lead to conflicts in a Guix profile.  The
> simplest example I could remember is - you want upgrade package ‘A’
> which propagates ‘direnv’, but you cannot because package ‘B’ propagates
> it too.  In this case you need to upgrade both ‘A’ and ‘B’ or delete ‘A’
> (or ‘B’).

Would there be a conflict if they both propagated the same input (in
this case the direnv binary)?

> Instead we could make the package functional by substituting in
> /gnu/store/…-emacs-direnv-…-checkout/direnv.el file ‘direnv--detect’
> ("Detect the direnv executable.") procedure which could return a path to
> ‘direnv’ binary as a string directly without calling ‘executable-find’.
> WDYT?

In general, I like to keep packages as close to their source as
possible, but I'm slowly learning that this is not often the case when
packaging things (which is a shame and a risk in my opinion).

But all things considered, I think this is probably the right approach
here given the feedback I'm getting.

Here's a patch which should supersede the previous patch:
Thank you both for your thoughtful reviews!

Comments

Oleg Pykhalov June 11, 2020, 8:31 p.m. UTC | #1 
Katherine Cox-Buday <cox.katherine.e@gmail.com> writes:

[…]

> In general, I like to keep packages as close to their source as
> possible, but I'm slowly learning that this is not often the case when
> packaging things (which is a shame and a risk in my opinion).
>
> But all things considered, I think this is probably the right approach
> here given the feedback I'm getting.

It depends.  If you take a look on magit-git-executable commentary, it
specifically doesn't use a full path to git binary, because of remote
systems could have it in different locations.

Does emacs-direnv could be used on remote machines?
Maxim Cournoyer Aug. 6, 2021, 4:49 a.m. UTC | #2 
Hello,

Katherine Cox-Buday <cox.katherine.e@gmail.com> writes:

[...]

>      (build-system emacs-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'patch-in-direnv
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (let* ((direnv-path (assoc-ref inputs "direnv"))
> +                    (direnv-bin (string-append
> +                                 "\"" direnv-path "/bin/direnv\"")))
> +               (substitute* "direnv.el"
> +                 (("\"direnv\"") direnv-bin))))))))
> +    (inputs
> +     `(("direnv" ,direnv)))
>      (propagated-inputs
>       `(("dash" ,emacs-dash)
>         ("with-editor" ,emacs-with-editor)))

Thanks to Nicolas and Oleg for the thoughtful review.  They had good
comments about was propagation was not the best way to make the package
usable out of the box.  Now that the above does the same but in a
functional way, I think it is a good addition.

I applied the above hunk (the package had already been updated) as
commit 0d72f24ac084acf9d69e147a692e5d8bcb2ea21b.

Thank you!

Closing.

Maxim
diff mbox series

Patch

From 52a5541b8a44c6629f6e2a6d3d47184f2ed5169b Mon Sep 17 00:00:00 2001
From: Katherine Cox-Buday <cox.katherine.e@gmail.com>
Date: Wed, 10 Jun 2020 10:09:48 -0500
Subject: [PATCH] gnu: emacs-direnv: Update to 2.1.0.

* gnu/packages/emacs-xyz.scm (emacs-direnv): Update to 2.1.0 and make direnv a
  propagated-input.
---
 gnu/packages/emacs-xyz.scm | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 946d01cba5..6eb5bc9d39 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -166,6 +166,7 @@ 
   #:use-module (gnu packages sphinx)
   #:use-module (gnu packages xdisorg)
   #:use-module (gnu packages shells)
+  #:use-module (gnu packages shellutils)
   #:use-module (gnu packages sqlite)
   #:use-module (gnu packages gnupg)
   #:use-module (gnu packages video)
@@ -2020,7 +2021,7 @@  Its features are:
 (define-public emacs-direnv
   (package
     (name "emacs-direnv")
-    (version "2.0.0")
+    (version "2.1.0")
     (source
      (origin
        (method git-fetch)
@@ -2030,8 +2031,20 @@  Its features are:
        (file-name (git-file-name name version))
        (sha256
         (base32
-         "005ibyzsx1fdyrl5iyhqpb1bg83mphzahq7zvw58x00syyqi2z49"))))
+         "0xkqn4604k2imas6azy1www56br8ls4iv9a44pxcd8h94j1fp44d"))))
     (build-system emacs-build-system)
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'patch-in-direnv
+           (lambda* (#:key inputs #:allow-other-keys)
+             (let* ((direnv-path (assoc-ref inputs "direnv"))
+                    (direnv-bin (string-append
+                                 "\"" direnv-path "/bin/direnv\"")))
+               (substitute* "direnv.el"
+                 (("\"direnv\"") direnv-bin))))))))
+    (inputs
+     `(("direnv" ,direnv)))
     (propagated-inputs
      `(("dash" ,emacs-dash)
        ("with-editor" ,emacs-with-editor)))
-- 
2.26.2