guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Robin Templeton
Subject: [bug#51179] [PATCH] gnu: Add yt-dlp.
Date: Sun, 24 Oct 2021 23:12:06 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux)

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

-- 
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!





reply via email to

[Prev in Thread] Current Thread [Next in Thread]