GNU bug report logs - #51179
[PATCH] gnu: Add yt-dlp.

Previous Next

Package: guix-patches;

Reported by: Robin Templeton <robin <at> terpri.org>

Date: Wed, 13 Oct 2021 09:51:01 UTC

Severity: normal

Tags: patch

Merged with 51363

Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#51179; Package guix-patches. (Wed, 13 Oct 2021 09:51:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Robin Templeton <robin <at> terpri.org>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Wed, 13 Oct 2021 09:51:01 GMT) Full text and rfc822 format available.

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

From: Robin Templeton <robin <at> terpri.org>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add yt-dlp.
Date: Wed, 13 Oct 2021 05:44:41 -0400
* gnu/packages/video.scm (yt-dlp): New variable.

Suggested-by: bdju <bdju <at> tilde.team>
---
 gnu/packages/video.scm | 71 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index fcac369f60..1aceac25eb 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -2360,6 +2360,77 @@ YouTube.com and many more sites.")
     (home-page "https://yt-dl.org")
     (license license:public-domain)))
 
+(define-public yt-dlp
+  (package
+    (name "yt-dlp")
+    (version "2021.10.10")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "https://github.com/yt-dlp/yt-dlp/"
+                                  "releases/download/"
+                                  version "/yt-dlp.tar.gz"))
+              (sha256
+               (base32
+                "1ywld4qhvsik970gbac1h3kvxb74r7150m5axq9r5nffdw5sz3vd"))
+              (snippet
+               '(begin
+                  ;; Delete the pre-generated files, except for the man page
+                  ;; which requires 'pandoc' to build.
+                  (for-each delete-file '("yt-dlp"
+                                          ;;pandoc is needed to generate
+                                          ;;"yt-dlp.1"
+                                          "completions/bash/yt-dlp"
+                                          "completions/fish/yt-dlp.fish"
+                                          "completions/zsh/_yt-dlp"))
+                  #t))))
+    (build-system python-build-system)
+    (arguments
+     ;; The problem here is that the directory for the man page and completion
+     ;; files is relative, and for some reason, setup.py uses the
+     ;; auto-detected sys.prefix instead of the user-defined "--prefix=FOO".
+     ;; So, we need pass the prefix directly.
+     `(#:tests? #f ; Many tests fail. The test suite can be run with pytest.
+       #:phases (modify-phases %standard-phases
+                  (add-after 'unpack 'default-to-the-ffmpeg-input
+                    (lambda _
+                      ;; See <https://issues.guix.gnu.org/43418#5>.
+                      ;; ffmpeg is big but required to request free formats
+                      ;; from, e.g., YouTube so pull it in unconditionally.
+                      ;; Continue respecting the --ffmpeg-location argument.
+                      (substitute* "yt_dlp/postprocessor/ffmpeg.py"
+                        (("\\.get\\('ffmpeg_location'\\)" match)
+                         (format #f "~a or '~a'" match (which "ffmpeg"))))
+                      #t))
+                  (add-before 'build 'build-generated-files
+                    (lambda _
+                      ;; Avoid the make targets that require pandoc.
+                      (invoke "make"
+                              "PYTHON=python"
+                              "yt-dlp"
+                              ;;"youtube-dl.1"   ; needs pandoc
+                              "completions")))
+                  (add-before 'install 'fix-the-data-directories
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (let ((prefix (assoc-ref outputs "out")))
+                        (substitute* "setup.py"
+                          (("'etc/")
+                           (string-append "'" prefix "/etc/"))
+                          (("'share/")
+                           (string-append "'" prefix "/share/")))
+                        #t))))))
+    (native-inputs
+     `(("zip" ,zip)))
+    (inputs
+     `(("ffmpeg" ,ffmpeg)))
+    (synopsis "Download videos from YouTube.com and other sites")
+    (description
+     "yt-dlp is a small command-line program to download videos from
+YouTube.com and many more sites.  It is a fork of youtube-dl with a
+focus on adding new features while keeping up-to-date with the
+original project.")
+    (home-page "https://github.com/yt-dlp/yt-dlp")
+    (license license:public-domain)))
+
 (define-public youtube-dl-gui
   (package
     (name "youtube-dl-gui")
-- 
2.33.0






Information forwarded to guix-patches <at> gnu.org:
bug#51179; Package guix-patches. (Wed, 13 Oct 2021 10:16:01 GMT) Full text and rfc822 format available.

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Robin Templeton <robin <at> terpri.org>, 51179 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: Add yt-dlp.
Date: Wed, 13 Oct 2021 12:15:39 +0200
Hi Robin,

Am Mittwoch, den 13.10.2021, 05:44 -0400 schrieb Robin Templeton:
> * gnu/packages/video.scm (yt-dlp): New variable.
> 
> Suggested-by: bdju <bdju <at> tilde.team>
> ---
>  gnu/packages/video.scm | 71
> ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
> index fcac369f60..1aceac25eb 100644
> --- a/gnu/packages/video.scm
> +++ b/gnu/packages/video.scm
> @@ -2360,6 +2360,77 @@ YouTube.com and many more sites.")
>      (home-page "https://yt-dl.org")
>      (license license:public-domain)))
>  
> +(define-public yt-dlp
> +  (package
> +    (name "yt-dlp")
> +    (version "2021.10.10")
I think yt-dlp can (and ought to) inherit from youtube-dl, which might
simplify some of the below.
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/yt-dlp/yt-dlp/
> "
> +                                  "releases/download/"
> +                                  version "/yt-dlp.tar.gz"))
> +              (sha256
> +               (base32
> +                "1ywld4qhvsik970gbac1h3kvxb74r7150m5axq9r5nffdw5sz3v
> d"))
> +              (snippet
> +               '(begin
> +                  ;; Delete the pre-generated files, except for the
> man page
> +                  ;; which requires 'pandoc' to build.
> +                  (for-each delete-file '("yt-dlp"
> +                                          ;;pandoc is needed to
> generate
> +                                          ;;"yt-dlp.1"
> +                                          "completions/bash/yt-dlp"
> +                                          "completions/fish/yt-
> dlp.fish"
> +                                          "completions/zsh/_yt-
> dlp"))
> +                  #t))))
Is this the same as for youtube-dl?  If not, we might want to give
pandoc as native-input.  That will increase build times, but it ought
not to increase closure size.
> +    (build-system python-build-system)
> +    (arguments
> +     ;; The problem here is that the directory for the man page and
> completion
> +     ;; files is relative, and for some reason, setup.py uses the
> +     ;; auto-detected sys.prefix instead of the user-defined "
> --prefix=FOO".
> +     ;; So, we need pass the prefix directly.
> +     `(#:tests? #f ; Many tests fail. The test suite can be run with
> pytest.
> +       #:phases (modify-phases %standard-phases
> +                  (add-after 'unpack 'default-to-the-ffmpeg-input
> +                    (lambda _
> +                      ;; See <https://issues.guix.gnu.org/43418#5>;.
> +                      ;; ffmpeg is big but required to request free
> formats
> +                      ;; from, e.g., YouTube so pull it in
> unconditionally.
> +                      ;; Continue respecting the --ffmpeg-location
> argument.
> +                      (substitute* "yt_dlp/postprocessor/ffmpeg.py"
> +                        (("\\.get\\('ffmpeg_location'\\)" match)
> +                         (format #f "~a or '~a'" match (which
> "ffmpeg"))))
> +                      #t))
> +                  (add-before 'build 'build-generated-files
> +                    (lambda _
> +                      ;; Avoid the make targets that require pandoc.
> +                      (invoke "make"
> +                              "PYTHON=python"
> +                              "yt-dlp"
> +                              ;;"youtube-dl.1"   ; needs pandoc
> +                              "completions")))
> +                  (add-before 'install 'fix-the-data-directories
> +                    (lambda* (#:key outputs #:allow-other-keys)
> +                      (let ((prefix (assoc-ref outputs "out")))
> +                        (substitute* "setup.py"
> +                          (("'etc/")
> +                           (string-append "'" prefix "/etc/"))
> +                          (("'share/")
> +                           (string-append "'" prefix "/share/")))
> +                        #t))))))
Horizontal space is at a premium and you can save some.
> +    (native-inputs
> +     `(("zip" ,zip)))
> +    (inputs
> +     `(("ffmpeg" ,ffmpeg)))
> +    (synopsis "Download videos from YouTube.com and other sites")
> +    (description
> +     "yt-dlp is a small command-line program to download videos from
> +YouTube.com and many more sites.  It is a fork of youtube-dl with a
> +focus on adding new features while keeping up-to-date with the
> +original project.")
> +    (home-page "https://github.com/yt-dlp/yt-dlp")
> +    (license license:public-domain)))
Otherwise LGTM, but haven't tested.

Regards,
Liliana





Information forwarded to guix-patches <at> gnu.org:
bug#51179; Package guix-patches. (Wed, 13 Oct 2021 12:47:01 GMT) Full text and rfc822 format available.

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

From: Robin Templeton <robin <at> terpri.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 51179 <at> debbugs.gnu.org
Subject: [PATCH v2] Re: bug#51179: [PATCH] gnu: Add yt-dlp.
Date: Wed, 13 Oct 2021 08:46:16 -0400
Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:

> Am Mittwoch, den 13.10.2021, 05:44 -0400 schrieb Robin Templeton:
>> * gnu/packages/video.scm (yt-dlp): New variable.
[...]
>> +(define-public yt-dlp
>> +  (package
>> +    (name "yt-dlp")
>> +    (version "2021.10.10")
> I think yt-dlp can (and ought to) inherit from youtube-dl, which might
> simplify some of the below.

Agreed, changed in the revised patch below (yt-dlp can't, e.g., directly
use youtube-dl's extra phases at the moment, but it should be more
maintainable in the future).

[...]
>> +              (snippet
>> +               '(begin
>> +                  ;; Delete the pre-generated files, except for the
>> man page
>> +                  ;; which requires 'pandoc' to build.
[...]
> Is this the same as for youtube-dl?  If not, we might want to give
> pandoc as native-input.  That will increase build times, but it ought
> not to increase closure size.

This is analogous to youtube-dl's pandoc avoidance; IMHO youtube-dl
ought to use pandoc as a native input, but I wanted to keep the
packaging as close to youtube-dl's as possible.

[...]
>> +                  (add-before 'build 'build-generated-files
>> +                    (lambda _
>> +                      ;; Avoid the make targets that require pandoc.
>> +                      (invoke "make"
>> +                              "PYTHON=python"
>> +                              "yt-dlp"
>> +                              ;;"youtube-dl.1"   ; needs pandoc
>> +                              "completions")))
>> +                  (add-before 'install 'fix-the-data-directories
>> +                    (lambda* (#:key outputs #:allow-other-keys)
>> +                      (let ((prefix (assoc-ref outputs "out")))
>> +                        (substitute* "setup.py"
>> +                          (("'etc/")
>> +                           (string-append "'" prefix "/etc/"))
>> +                          (("'share/")
>> +                           (string-append "'" prefix "/share/")))
>> +                        #t))))))
> Horizontal space is at a premium and you can save some.

I'm not sure where exactly this should use fewer columns, but I squeezed
the make invocation onto a single line.

v2 also adds three inputs needed for the program to run correctly
(updated based on efraim's yt-dlp package, <https://bpa.st/FJDA>; in
that package, the extra libraries are propagated inputs, but adding them
as regular inputs appears to be sufficient).

[...]
> Otherwise LGTM, but haven't tested.

Thanks for the quick review! Patch v2 below. (I tested it on a few
YouTube videos and it worked as expected, at least for simple "yt-dlp
$URL"-type usage.)

--8<---------------cut here---------------start------------->8---

* gnu/packages/video.scm (yt-dlp): New variable.

Suggested-by: bdju <bdju <at> tilde.team>
---
 gnu/packages/video.scm | 61 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index fcac369f60..94f5e85e77 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -142,6 +142,7 @@
   #:use-module (gnu packages man)
   #:use-module (gnu packages markup)
   #:use-module (gnu packages maths)
+  #:use-module (gnu packages music)
   #:use-module (gnu packages mp3)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages networking)
@@ -2360,6 +2361,66 @@ YouTube.com and many more sites.")
     (home-page "https://yt-dl.org")
     (license license:public-domain)))
 
+(define-public yt-dlp
+  (package/inherit youtube-dl
+    (name "yt-dlp")
+    (version "2021.10.10")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "https://github.com/yt-dlp/yt-dlp/"
+                                  "releases/download/"
+                                  version "/yt-dlp.tar.gz"))
+              (sha256
+               (base32
+                "1ywld4qhvsik970gbac1h3kvxb74r7150m5axq9r5nffdw5sz3vd"))
+              (snippet
+               '(begin
+                  ;; Delete the pre-generated files, except for the man page
+                  ;; which requires 'pandoc' to build.
+                  (for-each delete-file '("yt-dlp"
+                                          ;;pandoc is needed to generate
+                                          ;;"yt-dlp.1"
+                                          "completions/bash/yt-dlp"
+                                          "completions/fish/yt-dlp.fish"
+                                          "completions/zsh/_yt-dlp"))
+                  #t))))
+    (arguments
+     (substitute-keyword-arguments (package-arguments youtube-dl)
+       ((#:phases phases)
+        `(modify-phases ,phases
+           ;; See the comment for the corresponding phase in youtube-dl.
+           (replace 'default-to-the-ffmpeg-input
+             (lambda _
+               (substitute* "yt_dlp/postprocessor/ffmpeg.py"
+                 (("\\.get\\('ffmpeg_location'\\)" match)
+                  (format #f "~a or '~a'" match (which "ffmpeg"))))
+               #t))
+           (replace 'build-generated-files
+             (lambda _
+               ;; Avoid the yt-dlp.1 target, which requires pandoc.
+               (invoke "make" "PYTHON=python" "yt-dlp" "completions")))
+           (replace 'fix-the-data-directories
+             (lambda* (#:key outputs #:allow-other-keys)
+               (let ((prefix (assoc-ref outputs "out")))
+                 (substitute* "setup.py"
+                   (("'etc/")
+                    (string-append "'" prefix "/etc/"))
+                   (("'share/")
+                    (string-append "'" prefix "/share/"))))
+               #t))
+           (delete 'install-completion)))))
+    (inputs
+     `(("python-mutagen" ,python-mutagen)
+       ("python-pycryptodomex" ,python-pycryptodomex)
+       ("python-websockets" ,python-websockets)
+       ,@(package-inputs youtube-dl)))
+    (description
+     "yt-dlp is a small command-line program to download videos from
+YouTube.com and many more sites.  It is a fork of youtube-dl with a
+focus on adding new features while keeping up-to-date with the
+original project.")
+    (home-page "https://github.com/yt-dlp/yt-dlp")))
+
 (define-public youtube-dl-gui
   (package
     (name "youtube-dl-gui")
-- 
2.33.0




Information forwarded to guix-patches <at> gnu.org:
bug#51179; Package guix-patches. (Wed, 13 Oct 2021 14:54:01 GMT) Full text and rfc822 format available.

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

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Robin Templeton <robin <at> terpri.org>
Cc: 51179 <at> debbugs.gnu.org
Subject: Re: [PATCH v2] Re: bug#51179: [PATCH] gnu: Add yt-dlp.
Date: Wed, 13 Oct 2021 16:53:38 +0200
Hi Robin,

when pasting other stuff into a patch, one usually ought to do so after
the "---" line before the summary and the first diff.

Am Mittwoch, den 13.10.2021, 08:46 -0400 schrieb Robin Templeton:
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:
> 
> [...]
> > > +              (snippet
> > > +               '(begin
> > > +                  ;; Delete the pre-generated files, except for
> > > the man page
> > > +                  ;; which requires 'pandoc' to build.
> [...]
> > Is this the same as for youtube-dl?  If not, we might want to give
> > pandoc as native-input.  That will increase build times, but it
> > ought not to increase closure size.
> 
> This is analogous to youtube-dl's pandoc avoidance; IMHO youtube-dl
> ought to use pandoc as a native input, but I wanted to keep the
> packaging as close to youtube-dl's as possible.
Hence why I asked whether youtube-dl does the same.  Making that change
would probably be out of scope for now.

> [...]
> > > +                  (add-before 'build 'build-generated-files
> > > +                    (lambda _
> > > +                      ;; Avoid the make targets that require
> > > pandoc.
> > > +                      (invoke "make"
> > > +                              "PYTHON=python"
> > > +                              "yt-dlp"
> > > +                              ;;"youtube-dl.1"   ; needs pandoc
> > > +                              "completions")))
> > > +                  (add-before 'install 'fix-the-data-directories
> > > +                    (lambda* (#:key outputs #:allow-other-keys)
> > > +                      (let ((prefix (assoc-ref outputs "out")))
> > > +                        (substitute* "setup.py"
> > > +                          (("'etc/")
> > > +                           (string-append "'" prefix "/etc/"))
> > > +                          (("'share/")
> > > +                           (string-append "'" prefix
> > > "/share/")))
> > > +                        #t))))))
> > Horizontal space is at a premium and you can save some.
> 
> I'm not sure where exactly this should use fewer columns, but I
> squeezed the make invocation onto a single line.
You fixed it by accident, given that you're now required to use
substitute-keyword-arguments*, putting #:phases on an extra line as it
ought to be.

> v2 also adds three inputs needed for the program to run correctly
> (updated based on efraim's yt-dlp package, <https://bpa.st/FJDA>;; in
> that package, the extra libraries are propagated inputs, but adding
> them as regular inputs appears to be sufficient).
Can we somehow verify that this is indeed the case?  Normally we would
wrap PYTHONPATH in such a case.

> [...]
> > Otherwise LGTM, but haven't tested.
> 
> Thanks for the quick review! Patch v2 below. (I tested it on a few
> YouTube videos and it worked as expected, at least for simple "yt-dlp
> $URL"-type usage.)
> 
> --8<---------------cut here---------------start------------->8---
> 
> * gnu/packages/video.scm (yt-dlp): New variable.
> 
> Suggested-by: bdju <bdju <at> tilde.team>
> ---
>  gnu/packages/video.scm | 61
> ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
> 
> diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
> index fcac369f60..94f5e85e77 100644
> --- a/gnu/packages/video.scm
> +++ b/gnu/packages/video.scm
> @@ -142,6 +142,7 @@
>    #:use-module (gnu packages man)
>    #:use-module (gnu packages markup)
>    #:use-module (gnu packages maths)
> +  #:use-module (gnu packages music)
>    #:use-module (gnu packages mp3)
>    #:use-module (gnu packages ncurses)
>    #:use-module (gnu packages networking)
> @@ -2360,6 +2361,66 @@ YouTube.com and many more sites.")
>      (home-page "https://yt-dl.org")
>      (license license:public-domain)))
>  
> +(define-public yt-dlp
> +  (package/inherit youtube-dl
> +    (name "yt-dlp")
> +    (version "2021.10.10")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/yt-dlp/yt-dlp/
> "
> +                                  "releases/download/"
> +                                  version "/yt-dlp.tar.gz"))
> +              (sha256
> +               (base32
> +                "1ywld4qhvsik970gbac1h3kvxb74r7150m5axq9r5nffdw5sz3v
> d"))
> +              (snippet
> +               '(begin
> +                  ;; Delete the pre-generated files, except for the
> man page
> +                  ;; which requires 'pandoc' to build.
> +                  (for-each delete-file '("yt-dlp"
> +                                          ;;pandoc is needed to
> generate
> +                                          ;;"yt-dlp.1"
> +                                          "completions/bash/yt-dlp"
> +                                          "completions/fish/yt-
> dlp.fish"
> +                                          "completions/zsh/_yt-
> dlp"))
> +                  #t))))
> +    (arguments
> +     (substitute-keyword-arguments (package-arguments youtube-dl)
> +       ((#:phases phases)
> +        `(modify-phases ,phases
> +           ;; See the comment for the corresponding phase in
> youtube-dl.
> +           (replace 'default-to-the-ffmpeg-input
> +             (lambda _
> +               (substitute* "yt_dlp/postprocessor/ffmpeg.py"
> +                 (("\\.get\\('ffmpeg_location'\\)" match)
> +                  (format #f "~a or '~a'" match (which "ffmpeg"))))
> +               #t))
> +           (replace 'build-generated-files
> +             (lambda _
> +               ;; Avoid the yt-dlp.1 target, which requires pandoc.
> +               (invoke "make" "PYTHON=python" "yt-dlp"
> "completions")))
> +           (replace 'fix-the-data-directories
> +             (lambda* (#:key outputs #:allow-other-keys)
> +               (let ((prefix (assoc-ref outputs "out")))
> +                 (substitute* "setup.py"
> +                   (("'etc/")
> +                    (string-append "'" prefix "/etc/"))
> +                   (("'share/")
> +                    (string-append "'" prefix "/share/"))))
> +               #t))
> +           (delete 'install-completion)))))
Why do we not install completions?
> +    (inputs
> +     `(("python-mutagen" ,python-mutagen)
> +       ("python-pycryptodomex" ,python-pycryptodomex)
> +       ("python-websockets" ,python-websockets)
> +       ,@(package-inputs youtube-dl)))
> +    (description
> +     "yt-dlp is a small command-line program to download videos from
> +YouTube.com and many more sites.  It is a fork of youtube-dl with a
> +focus on adding new features while keeping up-to-date with the
> +original project.")
> +    (home-page "https://github.com/yt-dlp/yt-dlp")))
> +
>  (define-public youtube-dl-gui
>    (package
>      (name "youtube-dl-gui")

Regards,
Liliana





Merged 51179 51363. Request was from Leo Famulari <leo <at> famulari.name> to control <at> debbugs.gnu.org. (Sat, 23 Oct 2021 20:52:01 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#51179; Package guix-patches. (Mon, 25 Oct 2021 03:13:01 GMT) Full text and rfc822 format available.

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

From: Robin Templeton <robin <at> terpri.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 51179 <at> debbugs.gnu.org
Subject: Re: bug#51179: [PATCH] gnu: Add yt-dlp.
Date: Sun, 24 Oct 2021 23:12:06 -0400
Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:

> Hi Robin,
>
> when pasting other stuff into a patch, one usually ought to do so after
> the "---" line before the summary and the first diff.

Ah, thanks for the reminder, it's been a minute since I used email for
patch submissions :) So IIUC, it's preferable to write:

> * gnu/packages/video.scm (yt-dlp): New variable. [...]
> ---
> 
> [RANDOM COMMENTS HERE]
> 
>  gnu/packages/video.scm | 61
> ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)

rather than using "cut here" lines and the like.

> Am Mittwoch, den 13.10.2021, 08:46 -0400 schrieb Robin Templeton:
>> Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:
>> 
>> [...]
>> > > +              (snippet
>> > > +               '(begin
>> > > +                  ;; Delete the pre-generated files, except for
>> > > the man page
>> > > +                  ;; which requires 'pandoc' to build.
>> [...]
>> > Is this the same as for youtube-dl?  If not, we might want to give
>> > pandoc as native-input.  That will increase build times, but it
>> > ought not to increase closure size.
>> 
>> This is analogous to youtube-dl's pandoc avoidance; IMHO youtube-dl
>> ought to use pandoc as a native input, but I wanted to keep the
>> packaging as close to youtube-dl's as possible.
> Hence why I asked whether youtube-dl does the same.  Making that change
> would probably be out of scope for now.

+1

>> [...]
>> > > +                  (add-before 'build 'build-generated-files
>> > > +                    (lambda _
>> > > +                      ;; Avoid the make targets that require
>> > > pandoc.
>> > > +                      (invoke "make"
>> > > +                              "PYTHON=python"
>> > > +                              "yt-dlp"
>> > > +                              ;;"youtube-dl.1"   ; needs pandoc
>> > > +                              "completions")))
>> > > +                  (add-before 'install 'fix-the-data-directories
>> > > +                    (lambda* (#:key outputs #:allow-other-keys)
>> > > +                      (let ((prefix (assoc-ref outputs "out")))
>> > > +                        (substitute* "setup.py"
>> > > +                          (("'etc/")
>> > > +                           (string-append "'" prefix "/etc/"))
>> > > +                          (("'share/")
>> > > +                           (string-append "'" prefix
>> > > "/share/")))
>> > > +                        #t))))))
>> > Horizontal space is at a premium and you can save some.
>> 
>> I'm not sure where exactly this should use fewer columns, but I
>> squeezed the make invocation onto a single line.
> You fixed it by accident, given that you're now required to use
> substitute-keyword-arguments*, putting #:phases on an extra line as it
> ought to be.

Oh, I see what you meant now, thanks for clarifying.

>> v2 also adds three inputs needed for the program to run correctly
>> (updated based on efraim's yt-dlp package, <https://bpa.st/FJDA>;; in
>> that package, the extra libraries are propagated inputs, but adding
>> them as regular inputs appears to be sufficient).
> Can we somehow verify that this is indeed the case?  Normally we would
> wrap PYTHONPATH in such a case.

python-build-system wraps the yt-dlc script correctly, although (as
jgart explained to me on IRC) the Python library inputs would need to
become propagated inputs if users or future packages want to use yt-dlc
as a library.

Morgan's patch packages yt-dlc the way I'd package *youtube-dl* (using a
git origin, generating the manpage with pandoc, etc.) but yt-dlc doesn't
inherit from youtube-dl in their patch. The non-downloading-related part
of yt-dlc's test suite (~10% of tests) *does* run without failures,
unlike youtube-dl, so I enabled tests and added the pytest invocation
from Morgan's version; otherwise, no changes in v3 (besides using the
latest yt-dlc release), which I'll post as a follow-up.

I don't have a strong opinion about my patch vs. Morgan's for an initial
version; at some point I think youtube-dl should be packaged similarly
to Morgan's approach, which would allow the yt-dlc definition to be
simplified too, but my patch is written to inherit from youtube-dl as
it's packaged now. IMO the order of the changes doesn't matter much.

Thanks,
Robin

-- 
Inteligenta persono lernas la lingvon Esperanton rapide kaj facile.
Esperanto estas moderna, kultura lingvo por la mondo. Simpla, fleksebla,
belsona, Esperanto estas la praktika solvo de la problemo de universala
interkompreno. Lernu la interlingvon Esperanton!




Information forwarded to guix-patches <at> gnu.org:
bug#51179; Package guix-patches. (Mon, 25 Oct 2021 03:37:01 GMT) Full text and rfc822 format available.

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

From: Robin Templeton <robin <at> terpri.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 51179 <at> debbugs.gnu.org
Subject: Re: bug#51179: [PATCH] gnu: Add yt-dlp.
Date: Sun, 24 Oct 2021 23:36:24 -0400
* gnu/packages/video.scm (yt-dlp): New variable.

Co-authored-by: Morgan Smith <Morgan.J.Smith <at> outlook.com>
Suggested-by: bdju <bdju <at> tilde.team>
---
Add check phase from Morgan's package and use latest yt-dlp release

 gnu/packages/video.scm | 70 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index d2a2a08ee3..a2ae26987a 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -53,6 +53,7 @@
 ;;; Copyright © 2020 Hartmut Goebel <h.goebel <at> crazy-compilers.com>
 ;;; Copyright © 2021 Raghav Gururajan <rg <at> raghavgururajan.name>
 ;;; Copyright © 2021 Petr Hodina <phodina <at> protonmail.com>
+;;; Copyright © 2021 Robin Templeton <robin <at> terpri.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -142,6 +143,7 @@ (define-module (gnu packages video)
   #:use-module (gnu packages man)
   #:use-module (gnu packages markup)
   #:use-module (gnu packages maths)
+  #:use-module (gnu packages music)
   #:use-module (gnu packages mp3)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages networking)
@@ -2360,6 +2362,74 @@ (define-public youtube-dl
     (home-page "https://yt-dl.org")
     (license license:public-domain)))
 
+(define-public yt-dlp
+  (package/inherit youtube-dl
+    (name "yt-dlp")
+    (version "2021.10.22")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "https://github.com/yt-dlp/yt-dlp/"
+                                  "releases/download/"
+                                  version "/yt-dlp.tar.gz"))
+              (sha256
+               (base32
+                "0xh4cwmvx49pxn8x07wj2dy8ynj6xg8977l5493vv0l8zc27wp87"))
+              (snippet
+               '(begin
+                  ;; Delete the pre-generated files, except for the man page
+                  ;; which requires 'pandoc' to build.
+                  (for-each delete-file '("yt-dlp"
+                                          ;;pandoc is needed to generate
+                                          ;;"yt-dlp.1"
+                                          "completions/bash/yt-dlp"
+                                          "completions/fish/yt-dlp.fish"
+                                          "completions/zsh/_yt-dlp"))
+                  #t))))
+    (arguments
+     (substitute-keyword-arguments (package-arguments youtube-dl)
+       ((#:tests? _) #t)
+       ((#:phases phases)
+        `(modify-phases ,phases
+           ;; See the comment for the corresponding phase in youtube-dl.
+           (replace 'default-to-the-ffmpeg-input
+             (lambda _
+               (substitute* "yt_dlp/postprocessor/ffmpeg.py"
+                 (("\\.get\\('ffmpeg_location'\\)" match)
+                  (format #f "~a or '~a'" match (which "ffmpeg"))))
+               #t))
+           (replace 'build-generated-files
+             (lambda _
+               ;; Avoid the yt-dlp.1 target, which requires pandoc.
+               (invoke "make" "PYTHON=python" "yt-dlp" "completions")))
+           (replace 'fix-the-data-directories
+             (lambda* (#:key outputs #:allow-other-keys)
+               (let ((prefix (assoc-ref outputs "out")))
+                 (substitute* "setup.py"
+                   (("'etc/")
+                    (string-append "'" prefix "/etc/"))
+                   (("'share/")
+                    (string-append "'" prefix "/share/"))))
+               #t))
+           (delete 'install-completion)
+           (replace 'check
+             (lambda* (#:key tests? #:allow-other-keys)
+               (when tests?
+                 (invoke "pytest" "-k" "not download"))))))))
+    (inputs
+     `(("python-mutagen" ,python-mutagen)
+       ("python-pycryptodomex" ,python-pycryptodomex)
+       ("python-websockets" ,python-websockets)
+       ,@(package-propagated-inputs youtube-dl)))
+    (native-inputs
+     `(("python-pytest" ,python-pytest)
+       ,@(package-native-inputs youtube-dl)))
+    (description
+     "yt-dlp is a small command-line program to download videos from
+YouTube.com and many more sites.  It is a fork of youtube-dl with a
+focus on adding new features while keeping up-to-date with the
+original project.")
+    (home-page "https://github.com/yt-dlp/yt-dlp")))
+
 (define-public youtube-dl-gui
   (package
     (name "youtube-dl-gui")
-- 
2.33.0





Reply sent to Liliana Marie Prikler <liliana.prikler <at> gmail.com>:
You have taken responsibility. (Tue, 26 Oct 2021 12:55:01 GMT) Full text and rfc822 format available.

Notification sent to Robin Templeton <robin <at> terpri.org>:
bug acknowledged by developer. (Tue, 26 Oct 2021 12:55:02 GMT) Full text and rfc822 format available.

Message #27 received at 51179-done <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Robin Templeton <robin <at> terpri.org>
Cc: 51179-done <at> debbugs.gnu.org
Subject: Re: bug#51179: [PATCH] gnu: Add yt-dlp.
Date: Tue, 26 Oct 2021 14:54:30 +0200
Hi,

I've pushed this patch with the following adjustments,

Am Sonntag, den 24.10.2021, 23:36 -0400 schrieb Robin Templeton:
> * gnu/packages/video.scm (yt-dlp): New variable.
> 
> Co-authored-by: Morgan Smith <Morgan.J.Smith <at> outlook.com>
> Suggested-by: bdju <bdju <at> tilde.team>
> ---
> Add check phase from Morgan's package and use latest yt-dlp release
> 
>  gnu/packages/video.scm | 70
> ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
> index d2a2a08ee3..a2ae26987a 100644
> --- a/gnu/packages/video.scm
> +++ b/gnu/packages/video.scm
> @@ -53,6 +53,7 @@
>  ;;; Copyright © 2020 Hartmut Goebel <h.goebel <at> crazy-compilers.com>
>  ;;; Copyright © 2021 Raghav Gururajan <rg <at> raghavgururajan.name>
>  ;;; Copyright © 2021 Petr Hodina <phodina <at> protonmail.com>
> +;;; Copyright © 2021 Robin Templeton <robin <at> terpri.org>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -142,6 +143,7 @@ (define-module (gnu packages video)
>    #:use-module (gnu packages man)
>    #:use-module (gnu packages markup)
>    #:use-module (gnu packages maths)
> +  #:use-module (gnu packages music)
>    #:use-module (gnu packages mp3)
>    #:use-module (gnu packages ncurses)
>    #:use-module (gnu packages networking)
> @@ -2360,6 +2362,74 @@ (define-public youtube-dl
>      (home-page "https://yt-dl.org")
>      (license license:public-domain)))
>  
> +(define-public yt-dlp
> +  (package/inherit youtube-dl
> +    (name "yt-dlp")
> +    (version "2021.10.22")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/yt-dlp/yt-dlp/
> "
> +                                  "releases/download/"
> +                                  version "/yt-dlp.tar.gz"))
> +              (sha256
> +               (base32
> +                "0xh4cwmvx49pxn8x07wj2dy8ynj6xg8977l5493vv0l8zc27wp8
> 7"))
> +              (snippet
> +               '(begin
> +                  ;; Delete the pre-generated files, except for the
> man page
> +                  ;; which requires 'pandoc' to build.
> +                  (for-each delete-file '("yt-dlp"
> +                                          ;;pandoc is needed to
> generate
> +                                          ;;"yt-dlp.1"
> +                                          "completions/bash/yt-dlp"
> +                                          "completions/fish/yt-
> dlp.fish"
> +                                          "completions/zsh/_yt-
> dlp"))
> +                  #t))))
> +    (arguments
> +     (substitute-keyword-arguments (package-arguments youtube-dl)
> +       ((#:tests? _) #t)
> +       ((#:phases phases)
> +        `(modify-phases ,phases
> +           ;; See the comment for the corresponding phase in
> youtube-dl.
> +           (replace 'default-to-the-ffmpeg-input
> +             (lambda _
> +               (substitute* "yt_dlp/postprocessor/ffmpeg.py"
> +                 (("\\.get\\('ffmpeg_location'\\)" match)
> +                  (format #f "~a or '~a'" match (which "ffmpeg"))))
"get" becomes "get_param" in yt-dlp.
> +               #t))
> +           (replace 'build-generated-files
> +             (lambda _
> +               ;; Avoid the yt-dlp.1 target, which requires pandoc.
> +               (invoke "make" "PYTHON=python" "yt-dlp"
> "completions")))
> +           (replace 'fix-the-data-directories
> +             (lambda* (#:key outputs #:allow-other-keys)
> +               (let ((prefix (assoc-ref outputs "out")))
> +                 (substitute* "setup.py"
> +                   (("'etc/")
> +                    (string-append "'" prefix "/etc/"))
> +                   (("'share/")
> +                    (string-append "'" prefix "/share/"))))
> +               #t))
> +           (delete 'install-completion)
> +           (replace 'check
> +             (lambda* (#:key tests? #:allow-other-keys)
> +               (when tests?
> +                 (invoke "pytest" "-k" "not download"))))))))
> +    (inputs
> +     `(("python-mutagen" ,python-mutagen)
> +       ("python-pycryptodomex" ,python-pycryptodomex)
> +       ("python-websockets" ,python-websockets)
> +       ,@(package-propagated-inputs youtube-dl)))
Used (package-inputs) as it should be.
> +    (native-inputs
> +     `(("python-pytest" ,python-pytest)
> +       ,@(package-native-inputs youtube-dl)))
> +    (description
> +     "yt-dlp is a small command-line program to download videos from
> +YouTube.com and many more sites.  It is a fork of youtube-dl with a
> +focus on adding new features while keeping up-to-date with the
> +original project.")
> +    (home-page "https://github.com/yt-dlp/yt-dlp")))

Thanks





Reply sent to Liliana Marie Prikler <liliana.prikler <at> gmail.com>:
You have taken responsibility. (Tue, 26 Oct 2021 12:55:02 GMT) Full text and rfc822 format available.

Notification sent to Morgan.J.Smith <at> outlook.com:
bug acknowledged by developer. (Tue, 26 Oct 2021 12:55:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 24 Nov 2021 12:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 125 days ago.

Previous Next


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