[bug#53388] New package: gallery-dl

Hello,

The attached patch adds a new package: gallery-dl in
gnu/packages/video.scm

Thanks!

Hi,

Bird schreef op do 20-01-2022 om 11:48 [+0000]:
> +    (propagated-inputs (list python-requests))

The README says it can use ‘youtube-dl’ or ‘yt-dlp’ for vide downloads
and ‘ffmpeg‘ for some conversion, so maybe these could be added?

Also, propagation can lead to profile collisions which can be
complicated to resolve, so can this be made a regular input instead of
a propagated input?  Some wrap-program might be necessary.

> +    (home-page "https://github.com/mikf/gallery-dl")
> +    (synopsis "Command-line program to download images from several
> sites")
> +    (description "Command-line program to download image galleries
> + and collections from several image hosting sites")

Does it only support images, or also videos?  From the ‘ffmpeg’ and
‘youtube-dl’ dependency, I would assume the latter.  Also, when I'm
reading this description, it reminds me a lot of youtube-dl -- perhaps
the description could explain it supports more video sites, or less
video sites, or not only video but also images, or something along
these lines.

> +    (license gpl2+)
> +    (arguments
> +     '(#:tests? #f))))

Why are tests disabled here?

Also, 'gallery-dl' (indirectly) uses openssl for the S in HTTPS,
so you may need to add SSL_CERT_DIR (or SSL_CERT_FILE, not sure) to
native-search-paths to make "guix shell --pure yt-dlp nss-certs" work.

Although there appears to be some disagreement in that area, see e.g.
some discussion in #53324, so maybe not.

Greetings,
Maxime.

Maxime Devos <maximedevos@telenet.be> writes:

> Hi,
>
> Bird schreef op do 20-01-2022 om 11:48 [+0000]:
>> +    (propagated-inputs (list python-requests))
>
> The README says it can use ‘youtube-dl’ or ‘yt-dlp’ for vide downloads
> and ‘ffmpeg‘ for some conversion, so maybe these could be added?
>
> Also, propagation can lead to profile collisions which can be
> complicated to resolve, so can this be made a regular input instead of
> a propagated input?  Some wrap-program might be necessary.

Can programs call to regular inputs that are not propagated?  Since
gallery-dl will need ffmpeg and python-requests (and possibly yt-dlp),
would it need to be in user's profile?

>
>> +    (home-page "https://github.com/mikf/gallery-dl")
>> +    (synopsis "Command-line program to download images from several
>> sites")
>> +    (description "Command-line program to download image galleries
>> + and collections from several image hosting sites")
>
> Does it only support images, or also videos?  From the ‘ffmpeg’ and
> ‘youtube-dl’ dependency, I would assume the latter.  Also, when I'm
> reading this description, it reminds me a lot of youtube-dl -- perhaps
> the description could explain it supports more video sites, or less
> video sites, or not only video but also images, or something along
> these lines.
>

It's really similar to youtube-dl but targets images primarily, i'll
make that clearer in the next patch

>
>> +    (license gpl2+)
>> +    (arguments
>> +     '(#:tests? #f))))
>
> Why are tests disabled here?
>

Sorry, I just assumed it will fail the tests due to network-lessness of
build environment, it actually passes the tests.

>
> Also, 'gallery-dl' (indirectly) uses openssl for the S in HTTPS,
> so you may need to add SSL_CERT_DIR (or SSL_CERT_FILE, not sure) to
> native-search-paths to make "guix shell --pure yt-dlp nss-certs" work.
>
> Although there appears to be some disagreement in that area, see e.g.
> some discussion in #53324, so maybe not.
>

I did a quick grep of SSL_CERT_FILE and SSL_CERT_DIR in gnu/packages/
but didn't find anything, could you please explain what i should do
here? 

>
> Greetings,
> Maxime.
>

Thanks,
Bird

On Thu, Jan 20, 2022 at 11:48:25AM +0000, Bird wrote:
> The attached patch adds a new package: gallery-dl in
> gnu/packages/video.scm

> From 799d589e6b5d76b4cac6272d22f87603729fb9e1 Mon Sep 17 00:00:00 2001
> From: Bird <birdsite@airmail.cc>
> Date: Thu, 20 Jan 2022 11:42:24 +0000
> Subject: [PATCH] gnu: gallery-dl: Add a new package. *
>  gnu/packages/video.scm(gallery-dl): Add package.

Hello, and thanks for the patch!

I applied it to a Git checkout of the Guix source code, and tried
rebuilding Guix, as described here in the Contributing chapter of the
manual:

https://guix.gnu.org/manual/devel/en/html_node/Building-from-Git.html

However, it crashes like this:

------
ice-9/eval.scm:293:34: error: gpl2+: unbound variable
hint: Did you forget `(use-modules (guix licenses))'?

make[2]: *** [Makefile:7380: make-packages-go] Error 1
make[2]: Leaving directory '/home/leo/work/guix'
make[1]: *** [Makefile:6453: all-recursive] Error 1
make[1]: Leaving directory '/home/leo/work/guix'
make: *** [Makefile:3982: all] Error 2
------

The reason for this error is that licenses in the gnu/packages/video.scm
module are "prefixed" to avoid a namespace collision between packages
and licenses with the same name, such as zlib and expat:

https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/video.scm?id=f223535e1c2f052f671e44a6c546d0ebde3591b1#n80

You can check other packages in that module for examples of how to
prefix the license.

> +    (license gpl2+)

I checked several of the .py files in the source code, and setup.py, and
they all seem to specify that the program is made available under the
terms of version 2 of the GPL, but no later versions. So, this package
should use "gpl2", not "gpl2+".

> +    (arguments
> +     '(#:tests? #f))))

We always run upstream test suites in Guix packages, because it helps
validate our packaging and finds bugs to assist upstream development.
If we do not run the tests, we add a code comment explaining why. In
Scheme, comments begin with the ; (semicolon) character.

So, can you send a revised patch that adjusts the license and either
runs the test suite or explains why not?

Thanks in advance!

On Thu, Jan 20, 2022 at 05:42:17PM +0000, Bird wrote:
> > The README says it can use ‘youtube-dl’ or ‘yt-dlp’ for vide downloads
> > and ‘ffmpeg‘ for some conversion, so maybe these could be added?
> >
> > Also, propagation can lead to profile collisions which can be
> > complicated to resolve, so can this be made a regular input instead of
> > a propagated input?  Some wrap-program might be necessary.
> 
> Can programs call to regular inputs that are not propagated?  Since
> gallery-dl will need ffmpeg and python-requests (and possibly yt-dlp),
> would it need to be in user's profile?

They can, but they need to learn how to find them. Usually that happens
by making the package record the "store path" of the input at
build-time.

On the other hand, propagated-inputs appear on $PATH, so if the calling
program does the normal thing it will find them.

But, the drawback of propagating, for example, FFmpeg, is that it makes
it impossible for users to `guix install` a custom FFmpeg package
alongside the propagated FFmpeg.

> >> +    (home-page "https://github.com/mikf/gallery-dl")

I looked at the source code, and this program respects some
configuration options to find the location of youtube-dl and ffmpeg:

https://github.com/mikf/gallery-dl/blob/master/docs/configuration.rst#ugoiraffmpeg-location

It's a matter of opinion but, to me, both of those programs are things
that users may customize or choose particular versions of, so it's okay
to expect users to install them and configure gallery-dl properly.

If we want to improve this package in that regard... that's for a
followup patch!