unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Robin Templeton <robin@terpri.org>
To: Liliana Marie Prikler <liliana.prikler@gmail.com>
Cc: 51179@debbugs.gnu.org
Subject: [bug#51179] [PATCH] gnu: Add yt-dlp.
Date: Sun, 24 Oct 2021 23:12:06 -0400	[thread overview]
Message-ID: <8735opvlvd.fsf@terpri.org> (raw)
In-Reply-To: <87pms9mf7a.fsf@terpri.org>

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	other threads:[~2021-10-25  3:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-13  9:44 [bug#51179] [PATCH] gnu: Add yt-dlp Robin Templeton
2021-10-13 10:15 ` Liliana Marie Prikler
2021-10-13 12:46   ` [bug#51179] [PATCH v2] Re: bug#51179: " Robin Templeton
2021-10-13 14:53     ` Liliana Marie Prikler
2021-10-25  3:12       ` Robin Templeton [this message]
2021-10-25  3:36         ` [bug#51179] " Robin Templeton
2021-10-26 12:54           ` bug#51179: " Liliana Marie Prikler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8735opvlvd.fsf@terpri.org \
    --to=robin@terpri.org \
    --cc=51179@debbugs.gnu.org \
    --cc=liliana.prikler@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).