Message ID | 87pms9mf7a.fsf@terpri.org |
---|---|
State | Accepted |
Headers | show |
Series | [bug#51179] gnu: Add yt-dlp. | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
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@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
Liliana Marie Prikler <liliana.prikler@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@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
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")