| Message ID | CA+TvSRh2t0vx9qq+keg8CZc3nwxcrHxKNv9QojY4Ff_9dMndoQ@mail.gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#41790] Update emacs-direnv | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | fail | View Laminar job |
Hello, Katherine Cox-Buday <cox.katherine.e@gmail.com> writes: > 10:11 kate says: guix refresh -l emacs-direnv > No dependents other than itself: emacs-direnv@2.0.0 > > From 22f556d67d8ec2f548859ecd20239a859d70d102 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. Thank you. I wonder if propagating direnv is a good idea, tho. I understand that the Emacs library is not going to be useful if direnv is not available in user's profile. OTOH, installing this package from, e.g., M-x list-packages, wouldn't install direnv either. Moreover, I assume anyone installing this Emacs package would have direnv available already. Considering the rule of thumb is to limit propagated inputs, I suggest to remove direnv. WDYT? Regards,
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes: Thank you for the review! > I understand that the Emacs library is not going to be useful if > direnv is not available in user's profile. OTOH, installing this > package from, e.g., M-x list-packages, wouldn't install direnv either. The difference I see is that emacs's package manager is specifically for emacs -- not for a user's system. Guix is a package manager for a user's system which encompasses binaries and native libraries. This is one of the reasons I prefer Guix packages to various language/tool package managers because Guix can handle load paths and dependencies for me. > Moreover, I assume anyone installing this Emacs package would have > direnv available already. I did not, nor did I know that I had to until it wasn't working and I went and read the documentation. > Considering the rule of thumb is to limit propagated inputs, I suggest > to remove direnv. I disagree. If propagated inputs are not for this -- making the package even functional -- what are they for? But! I am open to discussion.
Katherine Cox-Buday <cox.katherine.e@gmail.com> writes: > I disagree. If propagated inputs are not for this -- making the package > even functional -- what are they for? IIUC, they are to be used as a last resort, e.g., when the package cannot possibly build without them. Reason is propagated inputs pollute user's profile. E.g., someone may want to use a different direnv, and this propagated input would conflict with their package. In this case, it is reasonable to expect users to install direnv themselves. > But! I am open to discussion. I hear your arguments. I'll let maintainers decide about this, and hopefully clarify what can and cannot be propagated, at least in Emacs packages. This will be useful feedback for future reviews. Meanwhile, you can still provide a patch only bumping emacs-direnv. It is also fine in you prefer to wait. Regards,
Nicolas Goaziou <mail@nicolasgoaziou.fr> writes: > Katherine Cox-Buday <cox.katherine.e@gmail.com> writes: > >> I disagree. If propagated inputs are not for this -- making the package >> even functional -- what are they for? > > IIUC, they are to be used as a last resort, e.g., when the package > cannot possibly build without them. I checked the Guix manual for the intended use of propagated inputs since I didn't completely understand how what is in the runtime environment would affect the build environment. I found something which maybe is open to interpretation: "To ensure that libraries written in those languages can find library code they depend on at run time, run-time dependencies must be listed in propagated-inputs rather than inputs." Maybe a binary required to operate is no different than "library code they depend on at run time"? > Reason is propagated inputs pollute user's profile. E.g., someone may > want to use a different direnv, and this propagated input would > conflict with their package. That's another good point I hadn't considered: what if for some reason a user wants a different version of the tool (presumably provided by Guix as well)? > I'll let maintainers decide about this, and hopefully clarify what can > and cannot be propagated, at least in Emacs packages. This will be > useful feedback for future reviews. That's a good idea. I have an opinion, but it is not fully informed. I appreciate the review, conversation, and appeal to authority! > Meanwhile, you can still provide a patch only bumping emacs-direnv. It > is also fine in you prefer to wait. I'll let this one sit. The version bump was a side-effect of me believing the package should also install the tool.
Hi. Katherine Cox-Buday <cox.katherine.e@gmail.com> writes: […] > I disagree. If propagated inputs are not for this -- making the package > even functional -- what are they for? > > But! I am open to discussion. 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’). 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? Oleg.
From 22f556d67d8ec2f548859ecd20239a859d70d102 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 | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm index 946d01cba5..caeee2d828 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,11 +2031,13 @@ Its features are: (file-name (git-file-name name version)) (sha256 (base32 - "005ibyzsx1fdyrl5iyhqpb1bg83mphzahq7zvw58x00syyqi2z49")))) + "0xkqn4604k2imas6azy1www56br8ls4iv9a44pxcd8h94j1fp44d")))) (build-system emacs-build-system) (propagated-inputs `(("dash" ,emacs-dash) - ("with-editor" ,emacs-with-editor))) + ("with-editor" ,emacs-with-editor) + ;; Without the binary, this mode is inoperable. + ("direnv" ,direnv))) (home-page "https://github.com/wbolster/emacs-direnv") (synopsis "Direnv integration for Emacs") (description -- 2.26.2