[PATCH] mps-youtube

[PATCH] mps-youtube

[ng0]

I am not sure about how the non-python runtime dependencies are supposed to be handled in python packages.

python-pafy needs youtube-dl to function.
mps-youtube needs at least mpv and optionally ffmpeg to deal with conversion of formats.

So far they are in (inputs). Are those supposed to be installed by the users themselves?

[PATCH 1/2] gnu: Add python-pafy.

[ng0]

* gnu/packages/python.scm (python-pafy): New variable.
---
 gnu/packages/python.scm | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index b7c9e11..32caf20 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -86,6 +86,7 @@
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages tls)
   #:use-module (gnu packages version-control)
+  #:use-module (gnu packages video)
   #:use-module (gnu packages web)
   #:use-module (gnu packages base)
   #:use-module (gnu packages xml)
@@ -1583,6 +1584,26 @@ standard library.")
      `(#:python ,python-2
        #:tests? #f)))) ; no setup.py test command
 
+(define-public python-pafy
+  (package
+    (name "python-pafy")
+    (version "0.5.2")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (pypi-uri "pafy" version))
+       (sha256
+        (base32
+         "1ckvrypyvb7jbqlgwdz0y337ajagjv7dgxyns326nqwypn1wpq0i"))))
+    (build-system python-build-system)
+    (inputs
+     `(("youtube-dl" ,youtube-dl)))
+    (home-page "http://np1.github.io/pafy/")
+    (synopsis "Retrieve YouTube content and metadata")
+    (description
+     "@code{pafy} is a python library to retrieve YouTube content and metadata.")
+    (license license:lgpl3)))
+
 (define-public python-py
   (package
     (name "python-py")
-- 
2.10.1

Re: [PATCH 1/2] gnu: Add python-pafy.

[Hartmut Goebel]

Am 02.11.2016 um 11:40 schrieb ng0:
> +    (inputs
> +     `(("youtube-dl" ,youtube-dl)))

This needs to be a propagated input since python-pafy is a package itself.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

Re: [PATCH 1/2] gnu: Add python-pafy.

[ng0]

Hartmut Goebel <h.goebel@crazy-compilers.com> writes:

> Am 02.11.2016 um 11:40 schrieb ng0:
>> +    (inputs
>> +     `(("youtube-dl" ,youtube-dl)))
>
> This needs to be a propagated input since python-pafy is a package itself.

Can you be more specific? I know now that other python inputs
should be propagated, but why non-python software? Or is
youtube-dl just another python software?

Thanks

Re: [PATCH 1/2] gnu: Add python-pafy.

[Hartmut Goebel]

Am 04.11.2016 um 19:18 schrieb ng0:
> Hartmut Goebel <h.goebel@crazy-compilers.com> writes:
>
>> Am 02.11.2016 um 11:40 schrieb ng0:
>>> +    (inputs
>>> +     `(("youtube-dl" ,youtube-dl)))
>> This needs to be a propagated input since python-pafy is a package itself.
> Can you be more specific? I know now that other python inputs
> should be propagated, but why non-python software? Or is
> youtube-dl just another python software?

Oh, sorry, I thought you know that youtube-dl is a python tool. (I not
it since I'm using it myself).

Basically you were right: tools do not need to be propagated (at least
as far as I understood).

But the reason this needs to be propagated is: it is not used as a tool
(via some system-call or such), but the python package is imported, see
<https://github.com/mps-youtube/pafy/blob/develop/pafy/backend_youtube_dl.py#L11>.
I have to admit that this is not obvious.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

Re: [PATCH 1/2] gnu: Add python-pafy.

[ng0]

Hartmut Goebel <h.goebel@crazy-compilers.com> writes:

> Am 04.11.2016 um 19:18 schrieb ng0:
>> Hartmut Goebel <h.goebel@crazy-compilers.com> writes:
>>
>>> Am 02.11.2016 um 11:40 schrieb ng0:
>>>> +    (inputs
>>>> +     `(("youtube-dl" ,youtube-dl)))
>>> This needs to be a propagated input since python-pafy is a package itself.
>> Can you be more specific? I know now that other python inputs
>> should be propagated, but why non-python software? Or is
>> youtube-dl just another python software?
>
> Oh, sorry, I thought you know that youtube-dl is a python tool. (I not
> it since I'm using it myself).

Ah, thanks. I use it, but I have forgotten which language it is
written in as it is not relevant for me.

>
> Basically you were right: tools do not need to be propagated (at least
> as far as I understood).
>
> But the reason this needs to be propagated is: it is not used as a tool
> (via some system-call or such), but the python package is imported, see
> <https://github.com/mps-youtube/pafy/blob/develop/pafy/backend_youtube_dl.py#L11>.
> I have to admit that this is not obvious.

Ok, thanks for the explanation. I will add this with a comment to
a new revision of the patch series.

[PATCH] mps-youtube, revision 3

[ng0]

[PATCH 1/2] gnu: Add python-pafy.

- made youtube-dl propagated-input as suggested by harmut

[PATCH 2/2] gnu: Add mps-youtube.

- no changes

[PATCH 1/2] gnu: Add python-pafy.

[ng0]

* gnu/packages/python.scm (python-pafy): New variable.
---
 gnu/packages/python.scm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index 8987450..4c1f9db 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -86,6 +86,7 @@
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages tls)
   #:use-module (gnu packages version-control)
+  #:use-module (gnu packages video)
   #:use-module (gnu packages web)
   #:use-module (gnu packages base)
   #:use-module (gnu packages xml)
@@ -1583,6 +1584,28 @@ standard library.")
      `(#:python ,python-2
        #:tests? #f)))) ; no setup.py test command
 
+(define-public python-pafy
+  (package
+    (name "python-pafy")
+    (version "0.5.2")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (pypi-uri "pafy" version))
+       (sha256
+        (base32
+         "1ckvrypyvb7jbqlgwdz0y337ajagjv7dgxyns326nqwypn1wpq0i"))))
+    (build-system python-build-system)
+    (propagated-inputs
+     ;; Youtube-dl is a python package which is imported in the file
+     ;; "backend_youtube_dl.py", therefore it needs to be propagated.
+     `(("youtube-dl" ,youtube-dl)))
+    (home-page "http://np1.github.io/pafy/")
+    (synopsis "Retrieve YouTube content and metadata")
+    (description
+     "@code{pafy} is a python library to retrieve YouTube content and metadata.")
+    (license license:lgpl3)))
+
 (define-public python-py
   (package
     (name "python-py")
-- 
2.10.2

[PATCH 2/2] gnu: Add mps-youtube.

[ng0]

* gnu/packages/video.scm (mps-youtube): New variable.
---
 gnu/packages/video.scm | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 6ce03f7..50dcb5d 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2016 Kei Kebreau <kei@openmailbox.org>
 ;;; Copyright © 2016 Dmitry Nikolaev <cameltheman@gmail.com>
 ;;; Copyright © 2016 Andy Patterson <ajpatter@uwaterloo.ca>
+;;; Copyright © 2016 ng0 <ng0@we.make.ritual.n0.is>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1683,3 +1684,35 @@ specifications.")
     (description "libaacs is a library which implements the Advanced Access
 Content System specification.")
     (license license:lgpl2.1+)))
+
+(define-public mps-youtube
+  (package
+    (name "mps-youtube")
+    (version "0.2.7.1")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "https://github.com/mps-youtube/mps-youtube/"
+                           "archive/v" version ".tar.gz"))
+       (file-name (string-append name "-" version ".tar.gz"))
+       (sha256
+        (base32
+         "1s7h35yx6f0szf8mm8612ic913w3v05m2kwphjfcxnpq0ammhyci"))))
+    (build-system python-build-system)
+    (propagated-inputs
+     `(("python-pafy" ,python-pafy)
+       ("python-pygobject" ,python-pygobject))) ; For mpris2 support
+    ;; These are runtime requirements.
+    (inputs
+     `(("mpv" ,mpv)
+       ("ffmpeg" ,ffmpeg))) ; For the convertion feature
+    (home-page "http://github.com/np1/mps-youtube")
+    (synopsis "Terminal based YouTube player and downloader")
+    (description
+     "@code{mps-youtube} is based on mps, a terminal based program to
+search, stream and download music.  This implementation uses YouTube as
+a source of content and can play and download video as well as audio.
+It can use either mpv or mplayer for playback, and for conversion of
+formats ffmpeg or libav is used.  The choice is up to users which one
+they like to go with.")
+    (license license:gpl3)))
-- 
2.10.2

Re: [PATCH] mps-youtube, revision 3

[Hartmut Goebel]

Am 04.11.2016 um 20:23 schrieb ng0:
> [PATCH 1/2] gnu: Add python-pafy.
>
> - made youtube-dl propagated-input as suggested by harmut
>
> [PATCH 2/2] gnu: Add mps-youtube.
>
> - no changes
>

Both LGTM.

Adding the comment about youtube_dl was a good idea :-)

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

Re: [PATCH] mps-youtube, revision 3

[Marius Bakke]

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

Hartmut Goebel <h.goebel@crazy-compilers.com> writes:

> Am 04.11.2016 um 20:23 schrieb ng0:
>> [PATCH 1/2] gnu: Add python-pafy.
>>
>> - made youtube-dl propagated-input as suggested by harmut
>>
>> [PATCH 2/2] gnu: Add mps-youtube.
>>
>> - no changes
>>
>
> Both LGTM.
>
> Adding the comment about youtube_dl was a good idea :-)

Both of these packages have 'or later' in the source license header.
Also mps-youtube does not need mpv or ffmpeg in inputs, since they are
optional runtime dependencies. Additionally home-page should be https.

Other than that this works great, neat program!

I pushed the patches with these changes, and a minor edit to the
mps-youtube description to make it clear that mplayer or mpv is
required.

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

Re: [PATCH] mps-youtube, revision 3

[ng0]

Marius Bakke <mbakke@fastmail.com> writes:

> Hartmut Goebel <h.goebel@crazy-compilers.com> writes:
>
>> Am 04.11.2016 um 20:23 schrieb ng0:
>>> [PATCH 1/2] gnu: Add python-pafy.
>>>
>>> - made youtube-dl propagated-input as suggested by harmut
>>>
>>> [PATCH 2/2] gnu: Add mps-youtube.
>>>
>>> - no changes
>>>
>>
>> Both LGTM.
>>
>> Adding the comment about youtube_dl was a good idea :-)
>
> Both of these packages have 'or later' in the source license header.
> Also mps-youtube does not need mpv or ffmpeg in inputs, since they are
> optional runtime dependencies.

Oh... I had forgotten about these two changes I wanted to
make. Thanks for fixing it!

> Additionally home-page should be https.
>
> Other than that this works great, neat program!
>
> I pushed the patches with these changes, and a minor edit to the
> mps-youtube description to make it clear that mplayer or mpv is
> required.
>

[PATCH 2/2] gnu: Add mps-youtube.

[ng0]

* gnu/packages/video.scm (mps-youtube): New variable.
---
 gnu/packages/video.scm | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 6ce03f7..298d8d8 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2016 Kei Kebreau <kei@openmailbox.org>
 ;;; Copyright © 2016 Dmitry Nikolaev <cameltheman@gmail.com>
 ;;; Copyright © 2016 Andy Patterson <ajpatter@uwaterloo.ca>
+;;; Copyright © 2016 ng0 <ng0@we.make.ritual.n0.is>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1683,3 +1684,32 @@ specifications.")
     (description "libaacs is a library which implements the Advanced Access
 Content System specification.")
     (license license:lgpl2.1+)))
+
+(define-public mps-youtube
+  (package
+    (name "mps-youtube")
+    (version "0.2.7.1")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "https://github.com/mps-youtube/mps-youtube/"
+                           "archive/v" version ".tar.gz"))
+       (file-name (string-append name "-" version ".tar.gz"))
+       (sha256
+        (base32
+         "1s7h35yx6f0szf8mm8612ic913w3v05m2kwphjfcxnpq0ammhyci"))))
+    (build-system python-build-system)
+    (propagated-inputs
+     `(("python-pafy" ,python-pafy)
+       ("python-pygobject" ,python-pygobject))) ; For mpris2 support
+    ;; These are runtime requirements.
+    (inputs
+     `(("mpv" ,mpv)
+       ("ffmpeg" ,ffmpeg))) ; For the convertion feature
+    (home-page "http://github.com/np1/mps-youtube")
+    (synopsis "Terminal based YouTube player and downloader")
+    (description
+     "@code{mps-youtube} is based on mps, a terminal based program to
+search, stream and download music.  This implementation uses YouTube as
+a source of content and can play and download video as well as audio.")
+    (license license:gpl3)))
-- 
2.10.1

Re: [PATCH] mps-youtube

[Marius Bakke]

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

ng0 <ng0@we.make.ritual.n0.is> writes:

> I am not sure about how the non-python runtime dependencies are supposed to be handled in python packages.
>
> python-pafy needs youtube-dl to function.
> mps-youtube needs at least mpv and optionally ffmpeg to deal with conversion of formats.

Have you checked whether the inputs are referenced in `guix gc -R
/gnu/store/...item`? If not, we'll need to make it record the absolute
paths of the dependencies somehow. Often this can be done with
'--with-foo=(string-append (assoc-ref inputs "foo") "/bin/foo"' in the
configure step. Another approach is to substitute those commands
with the full path directly in the code.

> So far they are in (inputs). Are those supposed to be installed by the users themselves?

For optional dependencies (i.e. if mps-youtube gracefully handles
missing ffmpeg), leaving it up to the user is good (but then it should
not be an input). For "hard" runtime dependencies, the absolute paths
must be recorded so that they won't get garbage collected.

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

Re: [PATCH] mps-youtube

[ng0]

Marius Bakke <mbakke@fastmail.com> writes:

> ng0 <ng0@we.make.ritual.n0.is> writes:
>
>> I am not sure about how the non-python runtime dependencies are supposed to be handled in python packages.
>>
>> python-pafy needs youtube-dl to function.
>> mps-youtube needs at least mpv and optionally ffmpeg to deal with conversion of formats.
>
> Have you checked whether the inputs are referenced in `guix gc -R
> /gnu/store/...item`?

Well, youtube-dl is in there, ffmpeg and mpv are not.

> If not, we'll need to make it record the absolute
> paths of the dependencies somehow. Often this can be done with
> '--with-foo=(string-append (assoc-ref inputs "foo") "/bin/foo"' in the
> configure step. Another approach is to substitute those commands
> with the full path directly in the code.

You have the choice between mpv and mplayer for mps-youtube, if I
remember correctly you get an informative error message if none
of the binaries is found in your $PATH.
The same applies for ffmpeg, but I never used the ffmpeg feature
so this is just asuming they are consistent.

part of mps_youtube/config.py:

def check_player(player):
    """ Check player exefile exists and get mpv version. """
    if util.has_exefile(player):
        util.load_player_info(player)

        if "mpv" in player:
            version = "%s.%s.%s" % g.mpv_version
            fmt = c.g, c.w, c.g, c.w, version
            msg = "%splayer%s set to %smpv%s (version %s)" % fmt
            return dict(valid=True, message=msg, value=player)

        else:
            msg = "%splayer%s set to %s%s%s" % (c.g, c.w, c.g, player, c.w)
            return dict(valid=True, message=msg, value=player)

    else:
        if mswin and not (player.endswith(".exe") or player.endswith(".com")):
            # Using mpv.exe has issues; use mpv.com
            if "mpv" in player:
                retval = check_player(player + ".com")
                if retval["valid"]:
                    return retval
            return check_player(player + ".exe")

        else:
            msg = "Player application %s%s%s not found" % (c.r, player, c.w)
            return dict(valid=False, message=msg)


>> So far they are in (inputs). Are those supposed to be installed by the users themselves?
>
> For optional dependencies (i.e. if mps-youtube gracefully handles
> missing ffmpeg), leaving it up to the user is good (but then it should
> not be an input). For "hard" runtime dependencies, the absolute paths
> must be recorded so that they won't get garbage collected.
>

With the info I gave above, I'd say I remove the dependencies
which are not found and add information to the description.
For example: … it can be used with either mpv or mplayer, for
optional format conversion you need to use ffmpeg.

What do you think?

Re: [PATCH] mps-youtube

[Marius Bakke]

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

ng0 <ng0@we.make.ritual.n0.is> writes:

>>> So far they are in (inputs). Are those supposed to be installed by the users themselves?
>>
>> For optional dependencies (i.e. if mps-youtube gracefully handles
>> missing ffmpeg), leaving it up to the user is good (but then it should
>> not be an input). For "hard" runtime dependencies, the absolute paths
>> must be recorded so that they won't get garbage collected.
>>
>
> With the info I gave above, I'd say I remove the dependencies
> which are not found and add information to the description.
> For example: … it can be used with either mpv or mplayer, for
> optional format conversion you need to use ffmpeg.
>
> What do you think?

That sounds sensible. It seems to handle the case where those are
missing as well, which is good.

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

[PATCH] mps-youtube, revision 2.

[ng0]

[PATCH 1/2] gnu: Add python-pafy.

no changes.

[PATCH 2/2] gnu: Add mps-youtube.

changes: removed inputs, expanded description

[PATCH 1/2] gnu: Add python-pafy.

[ng0]

* gnu/packages/python.scm (python-pafy): New variable.
---
 gnu/packages/python.scm | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index b7c9e11..32caf20 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -86,6 +86,7 @@
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages tls)
   #:use-module (gnu packages version-control)
+  #:use-module (gnu packages video)
   #:use-module (gnu packages web)
   #:use-module (gnu packages base)
   #:use-module (gnu packages xml)
@@ -1583,6 +1584,26 @@ standard library.")
      `(#:python ,python-2
        #:tests? #f)))) ; no setup.py test command
 
+(define-public python-pafy
+  (package
+    (name "python-pafy")
+    (version "0.5.2")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (pypi-uri "pafy" version))
+       (sha256
+        (base32
+         "1ckvrypyvb7jbqlgwdz0y337ajagjv7dgxyns326nqwypn1wpq0i"))))
+    (build-system python-build-system)
+    (inputs
+     `(("youtube-dl" ,youtube-dl)))
+    (home-page "http://np1.github.io/pafy/")
+    (synopsis "Retrieve YouTube content and metadata")
+    (description
+     "@code{pafy} is a python library to retrieve YouTube content and metadata.")
+    (license license:lgpl3)))
+
 (define-public python-py
   (package
     (name "python-py")
-- 
2.10.1

[PATCH 2/2] gnu: Add mps-youtube.

[ng0]

* gnu/packages/video.scm (mps-youtube): New variable.
---
 gnu/packages/video.scm | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 6ce03f7..50dcb5d 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2016 Kei Kebreau <kei@openmailbox.org>
 ;;; Copyright © 2016 Dmitry Nikolaev <cameltheman@gmail.com>
 ;;; Copyright © 2016 Andy Patterson <ajpatter@uwaterloo.ca>
+;;; Copyright © 2016 ng0 <ng0@we.make.ritual.n0.is>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1683,3 +1684,35 @@ specifications.")
     (description "libaacs is a library which implements the Advanced Access
 Content System specification.")
     (license license:lgpl2.1+)))
+
+(define-public mps-youtube
+  (package
+    (name "mps-youtube")
+    (version "0.2.7.1")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "https://github.com/mps-youtube/mps-youtube/"
+                           "archive/v" version ".tar.gz"))
+       (file-name (string-append name "-" version ".tar.gz"))
+       (sha256
+        (base32
+         "1s7h35yx6f0szf8mm8612ic913w3v05m2kwphjfcxnpq0ammhyci"))))
+    (build-system python-build-system)
+    (propagated-inputs
+     `(("python-pafy" ,python-pafy)
+       ("python-pygobject" ,python-pygobject))) ; For mpris2 support
+    ;; These are runtime requirements.
+    (inputs
+     `(("mpv" ,mpv)
+       ("ffmpeg" ,ffmpeg))) ; For the convertion feature
+    (home-page "http://github.com/np1/mps-youtube")
+    (synopsis "Terminal based YouTube player and downloader")
+    (description
+     "@code{mps-youtube} is based on mps, a terminal based program to
+search, stream and download music.  This implementation uses YouTube as
+a source of content and can play and download video as well as audio.
+It can use either mpv or mplayer for playback, and for conversion of
+formats ffmpeg or libav is used.  The choice is up to users which one
+they like to go with.")
+    (license license:gpl3)))
-- 
2.10.1