* [PATCH] gnu: password-store: Wrap PATH.
@ 2016-07-29 0:20 Alex Griffin
2016-07-29 0:38 ` Alex Griffin
0 siblings, 1 reply; 6+ messages in thread
From: Alex Griffin @ 2016-07-29 0:20 UTC (permalink / raw)
To: guix-devel
[-- Attachment #1: Type: text/plain, Size: 123 bytes --]
This patch fixes password-store so that you no longer need to install
its dependencies into your profile.
--
Alex Griffin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-password-store-Wrap-PATH.patch --]
[-- Type: text/x-patch; name="0001-gnu-password-store-Wrap-PATH.patch", Size: 2696 bytes --]
From 60527038221d2c1c0d35ca97a65ba2a01734da75 Mon Sep 17 00:00:00 2001
From: Alex Griffin <a@ajgrf.com>
Date: Thu, 28 Jul 2016 19:06:10 -0500
Subject: [PATCH] gnu: password-store: Wrap PATH.
* gnu/packages/password-utils.scm (password-store):
[arguments]: Wrap PATH more thoroughly.
[native-inputs]: Move getopt to inputs.
[inputs]: Add sed & alphabetize packages.
---
gnu/packages/password-utils.scm | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/gnu/packages/password-utils.scm b/gnu/packages/password-utils.scm
index a03214a..eda800b 100644
--- a/gnu/packages/password-utils.scm
+++ b/gnu/packages/password-utils.scm
@@ -266,27 +266,25 @@ any X11 window.")
'(#:phases
(modify-phases %standard-phases
(delete 'configure)
- (add-after
- ;; The script requires 'getopt' at run-time, and this allows
- ;; the user to not install the providing package 'util-linux'
- ;; in their profile.
- 'unpack 'patch-path
- (lambda* (#:key inputs outputs #:allow-other-keys)
- (let ((getopt (string-append (assoc-ref inputs "getopt")
- "/bin/getopt")))
- (substitute* "src/password-store.sh"
- (("GETOPT=\"getopt\"")
- (string-append "GETOPT=\"" getopt "\"")))
- #t))))
+ (add-after 'install 'wrap-path
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ (let* ((out (assoc-ref outputs "out"))
+ (path (map (lambda (pkg)
+ (string-append (assoc-ref inputs pkg) "/bin"))
+ '("coreutils" "getopt" "git" "gnupg" "pwgen"
+ "sed" "tree" "which" "xclip"))))
+ (wrap-program (string-append out "/bin/pass")
+ `("PATH" ":" prefix (,(string-join path ":"))))))))
#:make-flags (list "CC=gcc" (string-append "PREFIX=" %output))
#:test-target "test"))
- (native-inputs `(("getopt" ,util-linux))) ; getopt for the tests
- (inputs `(("gnupg" ,gnupg)
- ("pwgen" ,pwgen)
- ("xclip" ,xclip)
+ (inputs `(("getopt" ,util-linux)
("git" ,git)
+ ("gnupg" ,gnupg)
+ ("pwgen" ,pwgen)
+ ("sed" ,sed)
("tree" ,tree)
- ("which" ,which)))
+ ("which" ,which)
+ ("xclip" ,xclip)))
(home-page "http://www.passwordstore.org/")
(synopsis "Encrypted password manager")
(description "Password-store is a password manager which uses GnuPG to
--
2.9.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] gnu: password-store: Wrap PATH.
2016-07-29 0:20 [PATCH] gnu: password-store: Wrap PATH Alex Griffin
@ 2016-07-29 0:38 ` Alex Griffin
2016-07-29 9:07 ` Mathieu Lirzin
0 siblings, 1 reply; 6+ messages in thread
From: Alex Griffin @ 2016-07-29 0:38 UTC (permalink / raw)
To: guix-devel
[-- Attachment #1: Type: text/plain, Size: 233 bytes --]
On Thu, Jul 28, 2016, at 07:20 PM, Alex Griffin wrote:
> This patch fixes password-store so that you no longer need to install
> its dependencies into your profile.
Forgot a copyright line. Here's an updated patch.
--
Alex Griffin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-password-store-Wrap-PATH.patch --]
[-- Type: text/x-patch; name="0001-gnu-password-store-Wrap-PATH.patch", Size: 2984 bytes --]
From 74b838fea52293386169299881cdd7cfefff7f4d Mon Sep 17 00:00:00 2001
From: Alex Griffin <a@ajgrf.com>
Date: Thu, 28 Jul 2016 19:06:10 -0500
Subject: [PATCH] gnu: password-store: Wrap PATH.
* gnu/packages/password-utils.scm (password-store):
[arguments]: Wrap PATH more thoroughly.
[native-inputs]: Move getopt to inputs.
[inputs]: Add sed & alphabetize packages.
---
gnu/packages/password-utils.scm | 33 ++++++++++++++++-----------------
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/gnu/packages/password-utils.scm b/gnu/packages/password-utils.scm
index a03214a..497717f 100644
--- a/gnu/packages/password-utils.scm
+++ b/gnu/packages/password-utils.scm
@@ -6,6 +6,7 @@
;;; Copyright © 2016 Jessica Tallon <tsyesika@tsyesika.se>
;;; Copyright © 2016 Andreas Enge <andreas@enge.fr>
;;; Copyright © 2016 Lukas Gradl <lgradl@openmailbox.org>
+;;; Copyright © 2016 Alex Griffin <a@ajgrf.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -266,27 +267,25 @@ any X11 window.")
'(#:phases
(modify-phases %standard-phases
(delete 'configure)
- (add-after
- ;; The script requires 'getopt' at run-time, and this allows
- ;; the user to not install the providing package 'util-linux'
- ;; in their profile.
- 'unpack 'patch-path
- (lambda* (#:key inputs outputs #:allow-other-keys)
- (let ((getopt (string-append (assoc-ref inputs "getopt")
- "/bin/getopt")))
- (substitute* "src/password-store.sh"
- (("GETOPT=\"getopt\"")
- (string-append "GETOPT=\"" getopt "\"")))
- #t))))
+ (add-after 'install 'wrap-path
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ (let* ((out (assoc-ref outputs "out"))
+ (path (map (lambda (pkg)
+ (string-append (assoc-ref inputs pkg) "/bin"))
+ '("coreutils" "getopt" "git" "gnupg" "pwgen"
+ "sed" "tree" "which" "xclip"))))
+ (wrap-program (string-append out "/bin/pass")
+ `("PATH" ":" prefix (,(string-join path ":"))))))))
#:make-flags (list "CC=gcc" (string-append "PREFIX=" %output))
#:test-target "test"))
- (native-inputs `(("getopt" ,util-linux))) ; getopt for the tests
- (inputs `(("gnupg" ,gnupg)
- ("pwgen" ,pwgen)
- ("xclip" ,xclip)
+ (inputs `(("getopt" ,util-linux)
("git" ,git)
+ ("gnupg" ,gnupg)
+ ("pwgen" ,pwgen)
+ ("sed" ,sed)
("tree" ,tree)
- ("which" ,which)))
+ ("which" ,which)
+ ("xclip" ,xclip)))
(home-page "http://www.passwordstore.org/")
(synopsis "Encrypted password manager")
(description "Password-store is a password manager which uses GnuPG to
--
2.9.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] gnu: password-store: Wrap PATH.
2016-07-29 0:38 ` Alex Griffin
@ 2016-07-29 9:07 ` Mathieu Lirzin
2016-07-29 14:02 ` Thompson, David
0 siblings, 1 reply; 6+ messages in thread
From: Mathieu Lirzin @ 2016-07-29 9:07 UTC (permalink / raw)
To: Alex Griffin; +Cc: guix-devel
Hello,
Alex Griffin <a@ajgrf.com> writes:
> On Thu, Jul 28, 2016, at 07:20 PM, Alex Griffin wrote:
>
> From 74b838fea52293386169299881cdd7cfefff7f4d Mon Sep 17 00:00:00 2001
> From: Alex Griffin <a@ajgrf.com>
> Date: Thu, 28 Jul 2016 19:06:10 -0500
> Subject: [PATCH] gnu: password-store: Wrap PATH.
>
> * gnu/packages/password-utils.scm (password-store):
> [arguments]: Wrap PATH more thoroughly.
> [native-inputs]: Move getopt to inputs.
> [inputs]: Add sed & alphabetize packages.
^^
Indentation and formatting changes can be omitted in commit log.
> ---
> gnu/packages/password-utils.scm | 33 ++++++++++++++++-----------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/gnu/packages/password-utils.scm b/gnu/packages/password-utils.scm
> index a03214a..497717f 100644
> --- a/gnu/packages/password-utils.scm
> +++ b/gnu/packages/password-utils.scm
> @@ -6,6 +6,7 @@
> ;;; Copyright © 2016 Jessica Tallon <tsyesika@tsyesika.se>
> ;;; Copyright © 2016 Andreas Enge <andreas@enge.fr>
> ;;; Copyright © 2016 Lukas Gradl <lgradl@openmailbox.org>
> +;;; Copyright © 2016 Alex Griffin <a@ajgrf.com>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -266,27 +267,25 @@ any X11 window.")
> '(#:phases
> (modify-phases %standard-phases
> (delete 'configure)
> - (add-after
> - ;; The script requires 'getopt' at run-time, and this allows
> - ;; the user to not install the providing package 'util-linux'
> - ;; in their profile.
> - 'unpack 'patch-path
> - (lambda* (#:key inputs outputs #:allow-other-keys)
> - (let ((getopt (string-append (assoc-ref inputs "getopt")
> - "/bin/getopt")))
> - (substitute* "src/password-store.sh"
> - (("GETOPT=\"getopt\"")
> - (string-append "GETOPT=\"" getopt "\"")))
> - #t))))
> + (add-after 'install 'wrap-path
> + (lambda* (#:key inputs outputs #:allow-other-keys)
> + (let* ((out (assoc-ref outputs "out"))
> + (path (map (lambda (pkg)
> + (string-append (assoc-ref inputs pkg) "/bin"))
> + '("coreutils" "getopt" "git" "gnupg" "pwgen"
> + "sed" "tree" "which" "xclip"))))
> + (wrap-program (string-append out "/bin/pass")
> + `("PATH" ":" prefix (,(string-join path ":"))))))))
'let*' can safely be replaced by 'let' here.
> #:make-flags (list "CC=gcc" (string-append "PREFIX=" %output))
> #:test-target "test"))
> - (native-inputs `(("getopt" ,util-linux))) ; getopt for the tests
> - (inputs `(("gnupg" ,gnupg)
> - ("pwgen" ,pwgen)
> - ("xclip" ,xclip)
> + (inputs `(("getopt" ,util-linux)
> ("git" ,git)
> + ("gnupg" ,gnupg)
> + ("pwgen" ,pwgen)
> + ("sed" ,sed)
> ("tree" ,tree)
> - ("which" ,which)))
> + ("which" ,which)
> + ("xclip" ,xclip)))
> (home-page "http://www.passwordstore.org/")
> (synopsis "Encrypted password manager")
> (description "Password-store is a password manager which uses GnuPG to
Pushed in commit 61201e46a72b715e1a38ce56932c3f4f2d3885b4.
Thanks.
--
Mathieu Lirzin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gnu: password-store: Wrap PATH.
2016-07-29 9:07 ` Mathieu Lirzin
@ 2016-07-29 14:02 ` Thompson, David
2016-07-29 14:27 ` Alex Griffin
0 siblings, 1 reply; 6+ messages in thread
From: Thompson, David @ 2016-07-29 14:02 UTC (permalink / raw)
To: Mathieu Lirzin; +Cc: guix-devel
On Fri, Jul 29, 2016 at 5:07 AM, Mathieu Lirzin <mthl@gnu.org> wrote:
> Hello,
>
> Alex Griffin <a@ajgrf.com> writes:
>
>> On Thu, Jul 28, 2016, at 07:20 PM, Alex Griffin wrote:
>>
>> From 74b838fea52293386169299881cdd7cfefff7f4d Mon Sep 17 00:00:00 2001
>> From: Alex Griffin <a@ajgrf.com>
>> Date: Thu, 28 Jul 2016 19:06:10 -0500
>> Subject: [PATCH] gnu: password-store: Wrap PATH.
>>
>> * gnu/packages/password-utils.scm (password-store):
>> [arguments]: Wrap PATH more thoroughly.
>> [native-inputs]: Move getopt to inputs.
>> [inputs]: Add sed & alphabetize packages.
> ^^
> Indentation and formatting changes can be omitted in commit log.
>
>> ---
>> gnu/packages/password-utils.scm | 33 ++++++++++++++++-----------------
>> 1 file changed, 16 insertions(+), 17 deletions(-)
>>
>> diff --git a/gnu/packages/password-utils.scm b/gnu/packages/password-utils.scm
>> index a03214a..497717f 100644
>> --- a/gnu/packages/password-utils.scm
>> +++ b/gnu/packages/password-utils.scm
>> @@ -6,6 +6,7 @@
>> ;;; Copyright © 2016 Jessica Tallon <tsyesika@tsyesika.se>
>> ;;; Copyright © 2016 Andreas Enge <andreas@enge.fr>
>> ;;; Copyright © 2016 Lukas Gradl <lgradl@openmailbox.org>
>> +;;; Copyright © 2016 Alex Griffin <a@ajgrf.com>
>> ;;;
>> ;;; This file is part of GNU Guix.
>> ;;;
>> @@ -266,27 +267,25 @@ any X11 window.")
>> '(#:phases
>> (modify-phases %standard-phases
>> (delete 'configure)
>> - (add-after
>> - ;; The script requires 'getopt' at run-time, and this allows
>> - ;; the user to not install the providing package 'util-linux'
>> - ;; in their profile.
>> - 'unpack 'patch-path
>> - (lambda* (#:key inputs outputs #:allow-other-keys)
>> - (let ((getopt (string-append (assoc-ref inputs "getopt")
>> - "/bin/getopt")))
>> - (substitute* "src/password-store.sh"
>> - (("GETOPT=\"getopt\"")
>> - (string-append "GETOPT=\"" getopt "\"")))
>> - #t))))
>> + (add-after 'install 'wrap-path
>> + (lambda* (#:key inputs outputs #:allow-other-keys)
>> + (let* ((out (assoc-ref outputs "out"))
>> + (path (map (lambda (pkg)
>> + (string-append (assoc-ref inputs pkg) "/bin"))
>> + '("coreutils" "getopt" "git" "gnupg" "pwgen"
>> + "sed" "tree" "which" "xclip"))))
>> + (wrap-program (string-append out "/bin/pass")
>> + `("PATH" ":" prefix (,(string-join path ":"))))))))
>
> 'let*' can safely be replaced by 'let' here.
>
>> #:make-flags (list "CC=gcc" (string-append "PREFIX=" %output))
>> #:test-target "test"))
>> - (native-inputs `(("getopt" ,util-linux))) ; getopt for the tests
>> - (inputs `(("gnupg" ,gnupg)
>> - ("pwgen" ,pwgen)
>> - ("xclip" ,xclip)
>> + (inputs `(("getopt" ,util-linux)
>> ("git" ,git)
>> + ("gnupg" ,gnupg)
>> + ("pwgen" ,pwgen)
>> + ("sed" ,sed)
>> ("tree" ,tree)
>> - ("which" ,which)))
>> + ("which" ,which)
>> + ("xclip" ,xclip)))
>> (home-page "http://www.passwordstore.org/")
>> (synopsis "Encrypted password manager")
>> (description "Password-store is a password manager which uses GnuPG to
>
> Pushed in commit 61201e46a72b715e1a38ce56932c3f4f2d3885b4.
Sorry I'm late, but I don't think this is the right fix. Wrapping
programs should only be considered after other options are exhausted.
The correct fix is to patch the source directly to refer to
executables by their absolute path. Why wasn't it done this way?
- Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gnu: password-store: Wrap PATH.
2016-07-29 14:02 ` Thompson, David
@ 2016-07-29 14:27 ` Alex Griffin
2016-07-29 16:57 ` Mathieu Lirzin
0 siblings, 1 reply; 6+ messages in thread
From: Alex Griffin @ 2016-07-29 14:27 UTC (permalink / raw)
To: Thompson, David, Mathieu Lirzin; +Cc: guix-devel
On Fri, Jul 29, 2016, at 09:02 AM, Thompson, David wrote:
> Sorry I'm late, but I don't think this is the right fix. Wrapping
> programs should only be considered after other options are exhausted.
> The correct fix is to patch the source directly to refer to
> executables by their absolute path. Why wasn't it done this way?
Mostly because it would be a lot of on-going work every time the package
is updated. As a shell script, it invokes an external tool nearly every
other line. If you look at the discussion that happened in February when
the package was first added [1], Jessica (the original packager) was
really discouraged by the prospect of patching every invocation of an
external tool. She also tried to submit changes upstream so that kind of
work wouldn't be necessary, but upstream wasn't very receptive to the
idea. In the end, the package was just committed to Guix in a broken
state.
Another approach I considered was adding a bunch of shell aliases to the
top of the script pointing to the Guix store. That makes the problem a
lot more tractable than replacing every external invocation, but it's
still kinda complicated and it still requires finding every single
program that gets called and keeping it up to date with new releases. So
even if all upstream does is use another program from coreutils, the fix
would be out-of-date again.
[1]: https://lists.gnu.org/archive/html/guix-devel/2016-02/msg01310.html
--
Alex Griffin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gnu: password-store: Wrap PATH.
2016-07-29 14:27 ` Alex Griffin
@ 2016-07-29 16:57 ` Mathieu Lirzin
0 siblings, 0 replies; 6+ messages in thread
From: Mathieu Lirzin @ 2016-07-29 16:57 UTC (permalink / raw)
To: Alex Griffin; +Cc: guix-devel
Alex Griffin <a@ajgrf.com> writes:
> On Fri, Jul 29, 2016, at 09:02 AM, Thompson, David wrote:
>> Sorry I'm late, but I don't think this is the right fix.
Sorry for not letting you the time to respond.
>> Wrapping programs should only be considered after other options are
>> exhausted. The correct fix is to patch the source directly to refer
>> to executables by their absolute path. Why wasn't it done this way?
>
> Mostly because it would be a lot of on-going work every time the package
> is updated. As a shell script, it invokes an external tool nearly every
> other line. If you look at the discussion that happened in February when
> the package was first added [1], Jessica (the original packager) was
> really discouraged by the prospect of patching every invocation of an
> external tool. She also tried to submit changes upstream so that kind of
> work wouldn't be necessary, but upstream wasn't very receptive to the
> idea. In the end, the package was just committed to Guix in a broken
> state.
>
> Another approach I considered was adding a bunch of shell aliases to the
> top of the script pointing to the Guix store. That makes the problem a
> lot more tractable than replacing every external invocation, but it's
> still kinda complicated and it still requires finding every single
> program that gets called and keeping it up to date with new releases. So
> even if all upstream does is use another program from coreutils, the fix
> would be out-of-date again.
>
> [1]: https://lists.gnu.org/archive/html/guix-devel/2016-02/msg01310.html
This makes sense to me.
--
Mathieu Lirzin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-29 16:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-29 0:20 [PATCH] gnu: password-store: Wrap PATH Alex Griffin
2016-07-29 0:38 ` Alex Griffin
2016-07-29 9:07 ` Mathieu Lirzin
2016-07-29 14:02 ` Thompson, David
2016-07-29 14:27 ` Alex Griffin
2016-07-29 16:57 ` Mathieu Lirzin
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).