[PATCH] gnu: tailon: Add missing inputs.

  • Done
  • quality assurance status badge
Details
2 participants
  • Christopher Baines
  • Marius Bakke
Owner
unassigned
Submitted by
Christopher Baines
Severity
normal
C
C
Christopher Baines wrote on 4 May 2017 08:47
(address . guix-patches@gnu.org)
20170504064715.9779-1-mail@cbaines.net
* 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(-)

Toggle diff (42 lines)
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
M
M
Marius Bakke wrote on 5 May 2017 18:33
877f1v1cwq.fsf@fastmail.com
Christopher Baines <mail@cbaines.net> writes:

Toggle quote (4 lines)
> * 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.
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAlkMqWUACgkQoqBt8qM6
VPrPyQf9G1aB5JMvAJTn6/fzSPn8BcfY8qQn4ON0oTm5wyMOYm3uAjfUZ7OxuKIm
qWR8Le4tE8YRCY1lAwxQZXSR2wAgn4dMhjRhHOXqaXdoZdN1fcjNO5tZf7nR716H
V7M2S3jtGEH3JFFtj7c5Snvbbl6sUBQrmShwEnaBl5SfvxpwwrPxgkOaRpJwLNSc
BxfmHX5y7ZpN2ZBMwcLT3FhGBBjLAxw84oX0DLvrpP61ym9uNDYkzihamKjJWfWf
eig8YduCKb1BoSknk8NpHxNdPoyBlESklZ5T2FxYCq+q2a6r1MnHz8J86vrZE35x
hPzkUFNa5gttbij5wa2lg6lyhKshPw==
=gmdb
-----END PGP SIGNATURE-----

C
C
Christopher Baines wrote on 15 May 2017 08:11
[PATCH] gnu: tailon: Use absolute paths for commands.
(address . 26770@debbugs.gnu.org)
20170515061155.1380-1-mail@cbaines.net
* 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(+)

Toggle diff (29 lines)
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
C
C
Christopher Baines wrote on 15 May 2017 08:25
Re: bug#26770: [PATCH] gnu: tailon: Add missing inputs.
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 26770@debbugs.gnu.org)
20170515072549.5c933ddc@cbaines.net
On Fri, 05 May 2017 18:33:41 +0200
Marius Bakke <mbakke@fastmail.com> wrote:

Toggle quote (9 lines)
> 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.

Toggle quote (7 lines)
> 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 :)
-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlkZSe1fFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE
9Xfl6g//fos/HeQyntt6j1RVDsLZ1SCtfxHPiT2EvpSdS5z488MoXQ9aWPNtd+zV
KVuc5ZU70wRz/iCVYcTI6d4iee0nttOhQBkTcY/5KT0rYiYtij62hdPy65ZCduHv
+xDFqpmr0Zq3U4WRqhgUJji/UAF1/dZmC9NK7pRmJKGskciCAe0adU4DwJX4e4b7
+dn356FzAqx4WR00IMMH4I+V1PO6lurgWPY45uCfD+gUmuD8+By0QfhGD8n1jfMr
QGLeNM2Ce1lABYv+qUvEcrlrm+16Nu6Ut/j9fHh92MaPIlA4Lhl8pXKdAqgfcTvW
KecUCXjdPeBXww3CkukXAE5b6E5rVgNQSdvT7CxQfiwImd5gOg4VyOOkE/tKz2iw
xi+UtMSmIRe/uNodzLY9ejfxOtzVQNahl38Y9xqhw+uoVb+z3gmydkl6QipthgaJ
VXPJ9i28VrfOuqfOcGAmuqd9erZ0MlFdbTSG9Vdw30eSouhjoa5pIzl9txKZurBS
bRP4tv0vtpCeqO35BTsnw8bkUyka+cVvSfM6YpOtcIt9QEbPb561zCSHjAyN7UEE
nar1cE/W38liZfWWNzr+NRZWxtSpf3vSqywaotO8DRzaqA6vGpFJN5dg2t9o7JHX
P1SRq4JbMZEKIjmU7Nb6JxzgU1bL6f+HtNUzo19IZiqojsRLv1g=
=yviO
-----END PGP SIGNATURE-----


M
M
Marius Bakke wrote on 15 May 2017 17:59
Re: bug#26770: [PATCH] gnu: tailon: Use absolute paths for commands.
87o9uuf6vl.fsf@fastmail.com
Christopher Baines <mail@cbaines.net> writes:

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

Thanks for this!

[...]

Toggle quote (14 lines)
> + (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.
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAlkZ0G4ACgkQoqBt8qM6
VPrDJwf9FbeXvpaba8qDRjZTl7G1Ko5JngZHMP/ZiD/x2kxZAqrnnDxIqiAWvFd0
YFFMmHmA6NvTGDTI4y8mwStjGHjLZdazEak2GvYQ4+pCsGngNdIFei2Z0+w62a54
jzx+P6hO8w2f0DAJSK/AhDL1tRCJz9h1l/Fq0eygDcWRdJP/sBEOgL3M+ZTGRl9Y
qRLwm967p6mtSsm4F2GXFjeYRqRM/wK94jJcsfbLiEuL4LSA+W3wdmxkaBkJpHnt
/q6Oh28RcaSouI27Tv0WIGa203V1RwSudA08Y2j/IT910eZGIoUabu7KGlgLhPc/
KrMIlMIOqTJnwM7eQui1Ao/nSMVryA==
=LkGr
-----END PGP SIGNATURE-----

C
C
Christopher Baines wrote on 16 May 2017 21:40
[PATCH] gnu: tailon: Use absolute paths for commands.
(address . 26770@debbugs.gnu.org)
20170516194001.2591-1-mail@cbaines.net
* 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(+)

Toggle diff (28 lines)
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
C
C
Christopher Baines wrote on 16 May 2017 21:49
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 26770@debbugs.gnu.org)
20170516204928.4edd18b0@cbaines.net
On Mon, 15 May 2017 17:59:42 +0200
Marius Bakke <mbakke@fastmail.com> wrote:

Toggle quote (24 lines)
> 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.
-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlkbV8lfFIAAAAAALgAo
aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF
ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE
9Xc5ww//V2TnDL6epWFX1sj5AgxRl8EqU6J+VftD+t9k59kcJnKZr9dUExSnDxYy
hWnfDjcTbTmiVHNfh0PNWHZw7dPwgXO6hj59+LcaSbAcMgjpjCZs0uXNRBPUlBjp
SCrCBLKtXAU2A9CusefxYgn3Pyb301jblMCvR3zxykZd65hMoTyGm9oE62cqqiec
UYp44F4BiFyG6BZKKWqKTunSLCLFfv2ZYzPnxPwjeMaVFWecTAPUnJFDvfLn4oLg
cAlHVvPPHyBY9ZKioqzxwzYSMEz3yTZUAqWH0S7GUPZWoNPXrBViDU/NIYFUmUwm
2y65AdzdmmYgFRcsoKNe/hhNH+5U5gviEZaP/Rv1H2FCEI0FBqeHPQRKeMMJ0Rfm
pAmogrgkLztTQjhEAj8XWVkaGYkHQrkvBdOSEdQIaax/dxKzGrZFx8EgBNl6mXpo
jDLRaZ3ZKVHYuBsUze00gYhie7Kskdixekg7gsxVU4OMQ57eKX/P2WktK8LJF/HR
McQKhrXmqNYT62Y/N6fldWQwBD0xhdeDlZp7NWJbEund5mNK53WezrKxHGQbqu8L
GFDpfxrf1MA8OYmqFm+SIIf1KcW1lVDumGvm+bhYFO91ed5zBoDhR0JWcBhpD3xU
3Y3WDlRuIRuoB3iCFcVJ6VdKy2LBZbxJD95vFfzQpLHXhxRFcfQ=
=TMdt
-----END PGP SIGNATURE-----


M
M
Marius Bakke wrote on 17 May 2017 16:53
(name . Christopher Baines)(address . mail@cbaines.net)(address . 26770-done@debbugs.gnu.org)
87o9urcz5y.fsf@fastmail.com
Christopher Baines <mail@cbaines.net> writes:

Toggle quote (31 lines)
> 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!
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEu7At3yzq9qgNHeZDoqBt8qM6VPoFAlkcY/oACgkQoqBt8qM6
VPrE5ggAtbFqLbfMa9DfAlwJ9n9TJEcr+Te5BbtbzqS5d56kz4nKKuPjaD14uFYx
qi7i+BPLeRZFaYy5vt9WNgrNo7g71rKrNlAGW4P2McG16dmHzOF3JuR+Bakhn1Jo
bSwWdopczfa+dEFMDImMD6TZ+pFDCHIJ7eDok/TIVc1A43Blj/1CtZ8fsyMvljcn
6KJV97OgzkXfWTHdx/h5DPhjEMzJBiAt9ISKLsLX8/mFRK7iYvvlpg/DS8xvP6uB
xUU7vr6RpupESWPlfd4HFQyG9KChcOj3X5gw0+p2CFW723qdZDMuFQ7O02h9lqUa
WknpMgAGbp9dBYaPabY32gzxKS81nA==
=txVo
-----END PGP SIGNATURE-----

Closed
C
C
Christopher Baines wrote on 18 May 2017 20:50
(name . Marius Bakke)(address . mbakke@fastmail.com)(address . 26770-done@debbugs.gnu.org)
bd3f0095-de16-60c7-c649-4d4b89a9ad9f@cbaines.net
On 17/05/17 15:53, Marius Bakke wrote:
Toggle quote (40 lines)
> 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: signature.asc
Closed
?