GNU bug report logs - #29099
[PATCH] gnu: Add kodi-cli.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 29099 in the body.
You can then email your comments to 29099 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add kodi-cli.
Date: Wed, 01 Nov 2017 09:48:31 +0300

[0001-gnu-Add-kodi-cli.patch (text/x-patch, inline)]

From 741284b1619490859910fb402802ab10b31f0fd2 Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust <at> gmail.com>
Date: Wed, 1 Nov 2017 09:39:05 +0300
Subject: [PATCH] gnu: Add kodi-cli.

* gnu/packages/kodi.scm (kodi-cli): New variable.
---
 gnu/packages/kodi.scm | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/gnu/packages/kodi.scm b/gnu/packages/kodi.scm
index c8a65af79..65621e152 100644
--- a/gnu/packages/kodi.scm
+++ b/gnu/packages/kodi.scm
@@ -25,11 +25,13 @@
   #:use-module (guix git-download)
   #:use-module (guix build-system cmake)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system trivial)
   #:use-module (gnu packages algebra)
   #:use-module (gnu packages audio)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages avahi)
   #:use-module (gnu packages base)
+  #:use-module (gnu packages bash)
   #:use-module (gnu packages cdrom)
   #:use-module (gnu packages cmake)
   #:use-module (gnu packages compression)
@@ -426,3 +428,49 @@ plug-in system.")
                    license:public-domain          ;cpluff/examples
                    license:bsd-3                  ;misc, gtest
                    license:bsd-2)))))             ;xbmc/freebsd
+
+(define-public kodi-cli
+  (let ((commit "104dc23b2a993c8e6db8c46f4f8bec24b146549b") ; Add support for
+        (revision "1"))                                     ; `$HOME/.kodirc'.
+    (package
+      (name "kodi-cli")
+      (version (string-append "1.1-" revision "." (string-take commit 7)))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference (url "https://github.com/nawar/kodi-cli")
+                                    (commit commit)))
+                (sha256
+                 (base32
+                  "1xjhasc5gngfxpr1dlzy6q24w0wpdfjx12p43fanjppxw4i49n5p"))
+                (file-name (string-append name "-" version "-checkout"))))
+      (build-system trivial-build-system)
+      (inputs `(("bash" ,bash)))
+      (propagated-inputs `(("curl" ,curl)))
+      (arguments
+       `(#:modules ((guix build utils))
+         #:builder
+         (begin
+           (use-modules (guix build utils))
+           (copy-recursively (assoc-ref %build-inputs "source") ".")
+           (substitute* "kodi-cli"
+             (("/bin/bash") (string-append (assoc-ref %build-inputs "bash")
+                                           "/bin/bash")))
+           (install-file "kodi-cli" (string-append %output "/bin")))))
+      (home-page "https://github.com/nawar/kodi-cli")
+      (synopsis "Bash script to send commands to Kodi using JSON RPC")
+      (description "@code{kodi-cli} provides the Bash script to send commands to
+Kodi using JSON RPC.
+
+Feautures:
+
+@itemize
+@item Play, pause, stop the current played video.
+@item Skip forward or backward in the current played video.
+@item Play or queue to the current list YouTube video.
+@item Interactive and noninteractive volume control.
+@item Interactive navigation.
+@item Send text.
+@item Toggle fullscreen.
+@item Update or clean Kodi libraries.
+@end itemize\n")
+      (license license:gpl2+))))
-- 
2.14.3

[signature.asc (application/pgp-signature, inline)]

Message #8 received at 29099 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Oleg Pykhalov <go.wigust <at> gmail.com>, 29099 <at> debbugs.gnu.org
Subject: Re: [bug#29099] [PATCH] gnu: Add kodi-cli.
Date: Wed, 01 Nov 2017 22:32:34 +0100

[Message part 1 (text/plain, inline)]

Hello Oleg!

Oleg Pykhalov <go.wigust <at> gmail.com> writes:

> From 741284b1619490859910fb402802ab10b31f0fd2 Mon Sep 17 00:00:00 2001
> From: Oleg Pykhalov <go.wigust <at> gmail.com>
> Date: Wed, 1 Nov 2017 09:39:05 +0300
> Subject: [PATCH] gnu: Add kodi-cli.
>
> * gnu/packages/kodi.scm (kodi-cli): New variable.

Cool!

[...]

> +(define-public kodi-cli
> +  (let ((commit "104dc23b2a993c8e6db8c46f4f8bec24b146549b") ; Add support for
> +        (revision "1"))                                     ; `$HOME/.kodirc'.
> +    (package
> +      (name "kodi-cli")
> +      (version (string-append "1.1-" revision "." (string-take commit 7)))
> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference (url "https://github.com/nawar/kodi-cli")
> +                                    (commit commit)))
> +                (sha256
> +                 (base32
> +                  "1xjhasc5gngfxpr1dlzy6q24w0wpdfjx12p43fanjppxw4i49n5p"))
> +                (file-name (string-append name "-" version "-checkout"))))
> +      (build-system trivial-build-system)
> +      (inputs `(("bash" ,bash)))
> +      (propagated-inputs `(("curl" ,curl)))

There is only one reference to `curl` in the script, can you try to
substitute it with the absolute path and make it a regular input?  It's
good to avoid propagation when we can.

> +      (arguments
> +       `(#:modules ((guix build utils))
> +         #:builder
> +         (begin
> +           (use-modules (guix build utils))
> +           (copy-recursively (assoc-ref %build-inputs "source") ".")
> +           (substitute* "kodi-cli"
> +             (("/bin/bash") (string-append (assoc-ref %build-inputs "bash")
> +                                           "/bin/bash")))
> +           (install-file "kodi-cli" (string-append %output "/bin")))))

(install-file ...) has an unspecified return value, so please return #t here.

> +      (home-page "https://github.com/nawar/kodi-cli")
> +      (synopsis "Bash script to send commands to Kodi using JSON RPC")

Maybe just "Control Kodi from the command line".

> +      (description "@code{kodi-cli} provides the Bash script to send commands to
> +Kodi using JSON RPC.

Similarly, this sentence can be reduced to something like
"@code{kodi-cli} is a tool for sending commands to a Kodi server using
JSON-RPC.".  Bash is an boring implementation detail IMO. :-)

> +
> +Feautures:
      ^ Stray 'u' character.

> +
> +@itemize
> +@item Play, pause, stop the current played video.

"currently playing"

> +@item Skip forward or backward in the current played video.

Same here.  Maybe it should be "currently playing item", since I assume
this works for music as well.

> +@item Play or queue to the current list YouTube video.

"List of YouTube videos"?

> +@item Interactive and noninteractive volume control.
> +@item Interactive navigation.
> +@item Send text.

What does this mean?  Arbitrary commands, or text notifications?

> +@item Toggle fullscreen.
> +@item Update or clean Kodi libraries.
> +@end itemize\n")
> +      (license license:gpl2+))))

LGTM with some cosmetic improvements.  And welcome!

By the way, if you use Kodi from Guix, feel free to update the 18
snapshot if you can.  My media PC died a while back, and I didn't get a
replacement yet, so I have not been able to maintain the Kodi package.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 29099 <at> debbugs.gnu.org (full text, mbox):

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 29099 <at> debbugs.gnu.org
Subject: Re: [bug#29099] [PATCH] gnu: Add kodi-cli.
Date: Thu, 02 Nov 2017 12:14:54 +0300

[Message part 1 (text/plain, inline)]

Hello Marius,

Thank you for review!

Marius Bakke <mbakke <at> fastmail.com> writes:

>> +(define-public kodi-cli
>> +  (let ((commit "104dc23b2a993c8e6db8c46f4f8bec24b146549b") ; Add support for
>> +        (revision "1"))                                     ; `$HOME/.kodirc'.
>> +    (package
>> +      (name "kodi-cli")
>> +      (version (string-append "1.1-" revision "." (string-take commit 7)))
>> +      (source (origin
>> +                (method git-fetch)
>> +                (uri (git-reference (url "https://github.com/nawar/kodi-cli")
>> +                                    (commit commit)))
>> +                (sha256
>> +                 (base32
>> +                  "1xjhasc5gngfxpr1dlzy6q24w0wpdfjx12p43fanjppxw4i49n5p"))
>> +                (file-name (string-append name "-" version "-checkout"))))
>> +      (build-system trivial-build-system)
>> +      (inputs `(("bash" ,bash)))
>> +      (propagated-inputs `(("curl" ,curl)))
>
> There is only one reference to `curl` in the script, can you try to
> substitute it with the absolute path and make it a regular input?  It's
> good to avoid propagation when we can.

Done.  I also added input mps-youtube and did the same.

>> +      (arguments
>> +       `(#:modules ((guix build utils))
>> +         #:builder
>> +         (begin
>> +           (use-modules (guix build utils))
>> +           (copy-recursively (assoc-ref %build-inputs "source") ".")
>> +           (substitute* "kodi-cli"
>> +             (("/bin/bash") (string-append (assoc-ref %build-inputs "bash")
>> +                                           "/bin/bash")))
>> +           (install-file "kodi-cli" (string-append %output "/bin")))))
>
> (install-file ...) has an unspecified return value, so please return #t here.

Done.

>> +      (home-page "https://github.com/nawar/kodi-cli")
>> +      (synopsis "Bash script to send commands to Kodi using JSON RPC")
>
> Maybe just "Control Kodi from the command line".

Done.

>> +      (description "@code{kodi-cli} provides the Bash script to send commands to
>> +Kodi using JSON RPC.
>
> Similarly, this sentence can be reduced to something like
> "@code{kodi-cli} is a tool for sending commands to a Kodi server using
> JSON-RPC.".  Bash is an boring implementation detail IMO. :-)

Done.

>> +
>> +Feautures:
>       ^ Stray 'u' character.

Done.

>> +
>> +@itemize
>> +@item Play, pause, stop the current played video.
>
> "currently playing"

Done.

>> +@item Skip forward or backward in the current played video.
>
> Same here.  Maybe it should be "currently playing item", since I assume
> this works for music as well.

Done.

>> +@item Play or queue to the current list YouTube video.
>
> "List of YouTube videos"?

Done.

>> +@item Interactive and noninteractive volume control.
>> +@item Interactive navigation.
>> +@item Send text.
>
> What does this mean?  Arbitrary commands, or text notifications?

From my experience this allows you do the following:

1. Open Videos.
2. Add videos…
3. Select <none>.
4. Enter path by sending a command via 'kodi-cli -t PATH'.

Maybe call this item of the feature list as
“Send text to the Kodi keyboard”?


New patch is attached.

[0001-gnu-Add-kodi-cli.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

>> +@item Toggle fullscreen.
>> +@item Update or clean Kodi libraries.
>> +@end itemize\n")
>> +      (license license:gpl2+))))
>
> LGTM with some cosmetic improvements.  And welcome!
>
> By the way, if you use Kodi from Guix, feel free to update the 18
> snapshot if you can.  My media PC died a while back, and I didn't get a
> replacement yet, so I have not been able to maintain the Kodi package.

OK.  I do, but not much.  Primarily use it on a “media tablet”.  :-)

Oleg.

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 29099 <at> debbugs.gnu.org (full text, mbox):

From: Marius Bakke <mbakke <at> fastmail.com>
To: Oleg Pykhalov <go.wigust <at> gmail.com>
Cc: 29099 <at> debbugs.gnu.org
Subject: Re: [bug#29099] [PATCH] gnu: Add kodi-cli.
Date: Thu, 02 Nov 2017 21:00:14 +0100

[Message part 1 (text/plain, inline)]

Oleg Pykhalov <go.wigust <at> gmail.com> writes:

> Hello Marius,
>
> Thank you for review!
>
> Marius Bakke <mbakke <at> fastmail.com> writes:
>
>>> +(define-public kodi-cli
>>> +  (let ((commit "104dc23b2a993c8e6db8c46f4f8bec24b146549b") ; Add support for
>>> +        (revision "1"))                                     ; `$HOME/.kodirc'.
>>> +    (package
>>> +      (name "kodi-cli")
>>> +      (version (string-append "1.1-" revision "." (string-take commit 7)))
>>> +      (source (origin
>>> +                (method git-fetch)
>>> +                (uri (git-reference (url "https://github.com/nawar/kodi-cli")
>>> +                                    (commit commit)))
>>> +                (sha256
>>> +                 (base32
>>> +                  "1xjhasc5gngfxpr1dlzy6q24w0wpdfjx12p43fanjppxw4i49n5p"))
>>> +                (file-name (string-append name "-" version "-checkout"))))
>>> +      (build-system trivial-build-system)
>>> +      (inputs `(("bash" ,bash)))
>>> +      (propagated-inputs `(("curl" ,curl)))
>>
>> There is only one reference to `curl` in the script, can you try to
>> substitute it with the absolute path and make it a regular input?  It's
>> good to avoid propagation when we can.
>
> Done.  I also added input mps-youtube and did the same.

Excellent.

>>> +@item Interactive and noninteractive volume control.
>>> +@item Interactive navigation.
>>> +@item Send text.
>>
>> What does this mean?  Arbitrary commands, or text notifications?
>
> From my experience this allows you do the following:
>
> 1. Open Videos.
> 2. Add videos…
> 3. Select <none>.
> 4. Enter path by sending a command via 'kodi-cli -t PATH'.
>
> Maybe call this item of the feature list as
> “Send text to the Kodi keyboard”?

"Send text to the Kodi keyboard" is much better, thanks!

> New patch is attached.

[...]

> +      (arguments
> +       `(#:modules ((guix build utils))
> +         #:builder
> +         (begin
> +           (use-modules (guix build utils))
> +           (copy-recursively (assoc-ref %build-inputs "source") ".")
> +           (substitute* "kodi-cli"
> +             (("/bin/bash") (string-append (assoc-ref %build-inputs "bash")
> +                                           "/bin/bash"))
> +             (("output=\\$\\((curl)" all curl)
> +              (string-append "output=$("
> +                             (assoc-ref %build-inputs "curl")
> +                             "/bin/" curl))
> +             (("play_youtube `(mpsyt)" all mpsyt)
> +              (string-append "play_youtube `"
> +                             (assoc-ref %build-inputs "mps-youtube")
> +                             "/bin/" mpsyt)))

FYI, you could also use (which "curl") etc from (guix build utils) here,
but I usually prefer this form.

[...]

> +@itemize
> +@item Play, pause, stop the current playing item.
> +@item Skip forward or backward in the current playing item.

These should be "currently playing".

> +@item Play or queue to the current list YouTube videos.

"... current list of YouTube videos."

(note the "of")

LGTM otherwise.  Don't forget to add a copyright line for yourself.

Thanks!

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 29099 <at> debbugs.gnu.org (full text, mbox):

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: Marius Bakke <mbakke <at> fastmail.com>
Cc: 29099-done <at> debbugs.gnu.org, 29099 <at> debbugs.gnu.org
Subject: Re: [bug#29099] [PATCH] gnu: Add kodi-cli.
Date: Fri, 03 Nov 2017 06:57:05 +0300

[Message part 1 (text/plain, inline)]

Marius Bakke <mbakke <at> fastmail.com> writes:

[...]

>> Maybe call this item of the feature list as
>> “Send text to the Kodi keyboard”?
>
> "Send text to the Kodi keyboard" is much better, thanks!
>
>> +      (arguments
>> +       `(#:modules ((guix build utils))
>> +         #:builder
>> +         (begin
>> +           (use-modules (guix build utils))
>> +           (copy-recursively (assoc-ref %build-inputs "source") ".")
>> +           (substitute* "kodi-cli"
>> +             (("/bin/bash") (string-append (assoc-ref %build-inputs "bash")
>> +                                           "/bin/bash"))
>> +             (("output=\\$\\((curl)" all curl)
>> +              (string-append "output=$("
>> +                             (assoc-ref %build-inputs "curl")
>> +                             "/bin/" curl))
>> +             (("play_youtube `(mpsyt)" all mpsyt)
>> +              (string-append "play_youtube `"
>> +                             (assoc-ref %build-inputs "mps-youtube")
>> +                             "/bin/" mpsyt)))
>
> FYI, you could also use (which "curl") etc from (guix build utils) here,
> but I usually prefer this form.

Thanks for notice.  I prefer a form used in attached patch, too.

>> +@itemize
>> +@item Play, pause, stop the current playing item.
>> +@item Skip forward or backward in the current playing item.
>
> These should be "currently playing".

Done.

>> +@item Play or queue to the current list YouTube videos.
>
> "... current list of YouTube videos."
>
> (note the "of")

Done.

> LGTM otherwise.  Don't forget to add a copyright line for yourself.

OK.  I think it's fine to merge it now.


Pushed as 72df48dbad95c3bc70a2962f496420ce3363d0de

Oleg,
Thanks.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.