bug#26770: [PATCH] gnu: tailon: Add missing inputs.

bug#26770: [PATCH] gnu: tailon: Add missing inputs.

[Christopher Baines]

* gnu/packages/logging.scm (tailon)[inputs]: Add grep, gawk, sed and coreutils
  as inputs.
  [arguments]: Wrap bin/tailon to include some inputs in the PATH.
---
 gnu/packages/logging.scm | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/logging.scm b/gnu/packages/logging.scm
index 2523d65f6..060521eab 100644
--- a/gnu/packages/logging.scm
+++ b/gnu/packages/logging.scm
@@ -26,6 +26,8 @@
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system python)
   #:use-module (gnu packages)
+  #:use-module (gnu packages base)
+  #:use-module (gnu packages gawk)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages python)
   #:use-module (gnu packages autotools))
@@ -108,7 +110,25 @@ command line.")
     (inputs
      `(("python-pyyaml" ,python-pyyaml)
        ("python-sockjs-tornado" ,python-sockjs-tornado)
-       ("python-tornado" ,python-tornado)))
+       ("python-tornado" ,python-tornado)
+       ("grep" ,grep)
+       ("gawk" ,gawk)
+       ("sed" ,sed)
+       ("tail" ,coreutils)))
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-after 'install 'wrap-tailon-path
+                    (lambda* (#:key inputs outputs #:allow-other-keys)
+                      (let ((out (assoc-ref outputs "out")))
+                        (wrap-program (string-append out "/bin/tailon")
+                          `("PATH" ":" prefix ,(map
+                                                (lambda (input)
+                                                  (string-append
+                                                   (assoc-ref inputs input)
+                                                   "/bin"))
+                                                '("grep" "gawk" "sed" "tail"))))
+                        #t))))))
     (home-page "https://tailon.readthedocs.io/")
     (synopsis
      "Webapp for looking at and searching through log files")
-- 
2.12.0

bug#26770: [PATCH] gnu: tailon: Add missing inputs.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

Christopher Baines <mail@cbaines.net> writes:

> * gnu/packages/logging.scm (tailon)[inputs]: Add grep, gawk, sed and coreutils
>   as inputs.
>   [arguments]: Wrap bin/tailon to include some inputs in the PATH.

Generally it's better to fully qualify the paths to these programs in
the code. Is that difficult here?

I think all of these inputs are already available, so you can do e.g.:
(substitute "foo"
  (("'grep'") (which "grep))
  (("'gawk'") (which "gawk")))

...instead of referencing the inputs explicitly.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26770: [PATCH] gnu: tailon: Use absolute paths for commands.

[Christopher Baines]

* gnu/packages/logging.scm (tailon)[arguments]: Patch commands.py to reference
  grep, awk, sed and tail by absolute paths.
---
 gnu/packages/logging.scm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/gnu/packages/logging.scm b/gnu/packages/logging.scm
index 2523d65f6..11bbfef52 100644
--- a/gnu/packages/logging.scm
+++ b/gnu/packages/logging.scm
@@ -109,6 +109,22 @@ command line.")
      `(("python-pyyaml" ,python-pyyaml)
        ("python-sockjs-tornado" ,python-sockjs-tornado)
        ("python-tornado" ,python-tornado)))
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-after 'install 'wrap-tailon-path
+                    (lambda* (#:key inputs outputs #:allow-other-keys)
+                      (let ((out (assoc-ref outputs "out")))
+                        (substitute* (find-files out "commands.py")
+                          (("self\\.first_in_path\\('grep'\\)")
+                           (string-append"'" (which "grep") "'"))
+                          (("self\\.first_in_path\\('gawk', 'awk'\\)")
+                           (string-append"'" (which "gawk") "'"))
+                          (("self\\.first_in_path\\('gsed', 'sed'\\)")
+                           (string-append"'" (which "sed") "'"))
+                          (("self\\.first_in_path\\('gtail', 'tail'\\)")
+                           (string-append"'" (which "tail") "'")))
+                        #t))))))
     (home-page "https://tailon.readthedocs.io/")
     (synopsis
      "Webapp for looking at and searching through log files")
-- 
2.12.0

bug#26770: [PATCH] gnu: tailon: Add missing inputs.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

On Fri, 05 May 2017 18:33:41 +0200
Marius Bakke <mbakke@fastmail.com> wrote:

> Christopher Baines <mail@cbaines.net> writes:
> 
> > * gnu/packages/logging.scm (tailon)[inputs]: Add grep, gawk, sed
> > and coreutils as inputs.
> >   [arguments]: Wrap bin/tailon to include some inputs in the PATH.  
> 
> Generally it's better to fully qualify the paths to these programs in
> the code. Is that difficult here?

Good point, I've send an updated patch that does this.

> I think all of these inputs are already available, so you can do e.g.:
> (substitute "foo"
>   (("'grep'") (which "grep))
>   (("'gawk'") (which "gawk")))
> 
> ...instead of referencing the inputs explicitly.

That works, I've changed that in the latest patch also.

Thanks for your review :)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

bug#26770: [PATCH] gnu: tailon: Use absolute paths for commands.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

Christopher Baines <mail@cbaines.net> writes:

> * gnu/packages/logging.scm (tailon)[arguments]: Patch commands.py to reference
>   grep, awk, sed and tail by absolute paths.

Thanks for this!

[...]

> +         (add-after 'install 'wrap-tailon-path
> +                    (lambda* (#:key inputs outputs #:allow-other-keys)
> +                      (let ((out (assoc-ref outputs "out")))
> +                        (substitute* (find-files out "commands.py")
> +                          (("self\\.first_in_path\\('grep'\\)")
> +                           (string-append"'" (which "grep") "'"))
> +                          (("self\\.first_in_path\\('gawk', 'awk'\\)")
> +                           (string-append"'" (which "gawk") "'"))
> +                          (("self\\.first_in_path\\('gsed', 'sed'\\)")
> +                           (string-append"'" (which "sed") "'"))
> +                          (("self\\.first_in_path\\('gtail', 'tail'\\)")
> +                           (string-append"'" (which "tail") "'")))
> +                        #t))))))

Is there any particular reason this phase runs after 'install'? I think
we should try to avoid modifying files after they have been copied to
the store, but if doing this substitution earlier is difficult I guess
it's okay with a comment.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26770: [PATCH] gnu: tailon: Use absolute paths for commands.

[Christopher Baines]

* gnu/packages/logging.scm (tailon)[arguments]: Patch commands.py to reference
  grep, awk, sed and tail by absolute paths.
---
 gnu/packages/logging.scm | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/gnu/packages/logging.scm b/gnu/packages/logging.scm
index 2523d65f6..203279f33 100644
--- a/gnu/packages/logging.scm
+++ b/gnu/packages/logging.scm
@@ -109,6 +109,21 @@ command line.")
      `(("python-pyyaml" ,python-pyyaml)
        ("python-sockjs-tornado" ,python-sockjs-tornado)
        ("python-tornado" ,python-tornado)))
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'patch-commands.py
+                     (lambda args
+                       (substitute* (find-files "." "commands\\.py")
+                         (("self\\.first_in_path\\('grep'\\)")
+                          (string-append"'" (which "grep") "'"))
+                         (("self\\.first_in_path\\('gawk', 'awk'\\)")
+                          (string-append"'" (which "gawk") "'"))
+                         (("self\\.first_in_path\\('gsed', 'sed'\\)")
+                          (string-append"'" (which "sed") "'"))
+                         (("self\\.first_in_path\\('gtail', 'tail'\\)")
+                          (string-append"'" (which "tail") "'")))
+                       #t)))))
     (home-page "https://tailon.readthedocs.io/")
     (synopsis
      "Webapp for looking at and searching through log files")
-- 
2.12.0

bug#26770: [PATCH] gnu: tailon: Use absolute paths for commands.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]

On Mon, 15 May 2017 17:59:42 +0200
Marius Bakke <mbakke@fastmail.com> wrote:

> Christopher Baines <mail@cbaines.net> writes:
> > +         (add-after 'install 'wrap-tailon-path
> > +                    (lambda* (#:key inputs outputs
> > #:allow-other-keys)
> > +                      (let ((out (assoc-ref outputs "out")))
> > +                        (substitute* (find-files out "commands.py")
> > +                          (("self\\.first_in_path\\('grep'\\)")
> > +                           (string-append"'" (which "grep") "'"))
> > +                          (("self\\.first_in_path\\('gawk',
> > 'awk'\\)")
> > +                           (string-append"'" (which "gawk") "'"))
> > +                          (("self\\.first_in_path\\('gsed',
> > 'sed'\\)")
> > +                           (string-append"'" (which "sed") "'"))
> > +                          (("self\\.first_in_path\\('gtail',
> > 'tail'\\)")
> > +                           (string-append"'" (which "tail") "'")))
> > +                        #t))))))  
> 
> Is there any particular reason this phase runs after 'install'? I
> think we should try to avoid modifying files after they have been
> copied to the store, but if doing this substitution earlier is
> difficult I guess it's okay with a comment.

No, I just put it there by default, but I can see why doing the
substitution earlier would be better. I've sent another patch that
moves it to after the unpack phase. 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

bug#26770: [PATCH] gnu: tailon: Use absolute paths for commands.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1820 bytes --]

Christopher Baines <mail@cbaines.net> writes:

> On Mon, 15 May 2017 17:59:42 +0200
> Marius Bakke <mbakke@fastmail.com> wrote:
>
>> Christopher Baines <mail@cbaines.net> writes:
>> > +         (add-after 'install 'wrap-tailon-path
>> > +                    (lambda* (#:key inputs outputs
>> > #:allow-other-keys)
>> > +                      (let ((out (assoc-ref outputs "out")))
>> > +                        (substitute* (find-files out "commands.py")
>> > +                          (("self\\.first_in_path\\('grep'\\)")
>> > +                           (string-append"'" (which "grep") "'"))
>> > +                          (("self\\.first_in_path\\('gawk',
>> > 'awk'\\)")
>> > +                           (string-append"'" (which "gawk") "'"))
>> > +                          (("self\\.first_in_path\\('gsed',
>> > 'sed'\\)")
>> > +                           (string-append"'" (which "sed") "'"))
>> > +                          (("self\\.first_in_path\\('gtail',
>> > 'tail'\\)")
>> > +                           (string-append"'" (which "tail") "'")))
>> > +                        #t))))))  
>> 
>> Is there any particular reason this phase runs after 'install'? I
>> think we should try to avoid modifying files after they have been
>> copied to the store, but if doing this substitution earlier is
>> difficult I guess it's okay with a comment.
>
> No, I just put it there by default, but I can see why doing the
> substitution earlier would be better. I've sent another patch that
> moves it to after the unpack phase. 

Applied, thank you! My rationale was that the store may be on slow
storage (say NFS), whereas the build directory is probably a tmpfs
or local storage.

Note: I replaced the "find-files" invocation with just the one file
path. Hope that was okay!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#26770: [PATCH] gnu: tailon: Use absolute paths for commands.

[Christopher Baines]

[-- Attachment #1.1: Type: text/plain, Size: 1940 bytes --]

On 17/05/17 15:53, Marius Bakke wrote:
> Christopher Baines <mail@cbaines.net> writes:
> 
>> On Mon, 15 May 2017 17:59:42 +0200
>> Marius Bakke <mbakke@fastmail.com> wrote:
>>
>>> Christopher Baines <mail@cbaines.net> writes:
>>>> +         (add-after 'install 'wrap-tailon-path
>>>> +                    (lambda* (#:key inputs outputs
>>>> #:allow-other-keys)
>>>> +                      (let ((out (assoc-ref outputs "out")))
>>>> +                        (substitute* (find-files out "commands.py")
>>>> +                          (("self\\.first_in_path\\('grep'\\)")
>>>> +                           (string-append"'" (which "grep") "'"))
>>>> +                          (("self\\.first_in_path\\('gawk',
>>>> 'awk'\\)")
>>>> +                           (string-append"'" (which "gawk") "'"))
>>>> +                          (("self\\.first_in_path\\('gsed',
>>>> 'sed'\\)")
>>>> +                           (string-append"'" (which "sed") "'"))
>>>> +                          (("self\\.first_in_path\\('gtail',
>>>> 'tail'\\)")
>>>> +                           (string-append"'" (which "tail") "'")))
>>>> +                        #t))))))  
>>>
>>> Is there any particular reason this phase runs after 'install'? I
>>> think we should try to avoid modifying files after they have been
>>> copied to the store, but if doing this substitution earlier is
>>> difficult I guess it's okay with a comment.
>>
>> No, I just put it there by default, but I can see why doing the
>> substitution earlier would be better. I've sent another patch that
>> moves it to after the unpack phase. 
> 
> Applied, thank you! My rationale was that the store may be on slow
> storage (say NFS), whereas the build directory is probably a tmpfs
> or local storage.
> 
> Note: I replaced the "find-files" invocation with just the one file
> path. Hope that was okay!

Yep, looks good. Thanks for your review :)



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]