[bug#31017] [PATCH] gnu: Add subdl

[Leo Famulari <leo@famulari.name>]

[-- Attachment #1.1: Type: text/plain, Size: 2358 bytes --]

On Sun, Apr 01, 2018 at 08:45:02PM +0530, Pierre Neidhardt wrote:
> * gnu/packages/video.scm (subdl): New variable.

Thanks!

> +(define-public subdl
> +  (let
> +      ((commit "4cf5789b11f0ff3f863b704b336190bf968cd471")
> +       (revision "1"))

It's okay to put 'commit' on the same line as 'let' :)

> +    (package
> +      (name "subdl")
> +      (version (string-append "1.0.3-" revision "." (string-take commit 7)))

Here we can use the git-version procedure and forget about how many
characters of the Git hash we are supposed to use:

(version (git-version "1.0.3" revision commit))

I was curious where the "1.0.3" comes from, since the upstream repo
lacks any version tags, but I found it in the subdl source code.

> +      (source (origin
> +                (method git-fetch)

And likewise in here, one should use git-file-name so that the source
archive is named descriptively:

(file-name (git-file-name name version))

> +      (build-system trivial-build-system)
> +      (arguments
> +       `(#:modules ((guix build utils))
> +         #:builder (begin
> +                     (use-modules (guix build utils))
> +                     (let* ((out (assoc-ref %outputs "out"))
> +                            (bin (string-append out "/bin"))
> +                            (source (assoc-ref %build-inputs "source"))
> +                            (python (assoc-ref %build-inputs "python")))
> +                       (mkdir-p bin)
> +                       (with-directory-excursion bin
> +                         (copy-file (string-append source "/subdl") "subdl")
> +                         (patch-shebang "subdl"
> +                                        (list (string-append python "/bin")))
> +                         (chmod "subdl" #o555))))))

You asked about this chmod in your followup message. In this case it's
unnecessary since the file is installed #o555 regardless. And we can
even simplify the code to this:

(install-file (string-append source "/subdl") bin) 
(patch-shebang (string-append bin "/subdl")
               (list (string-append python "/bin")))

The (install-file) procedure conveniently combines (mkdir-p) and
(copy-file).

I can commit with the changes in the attached diff, but which name and
email should I use for your copyright statement?

[-- Attachment #1.2: diff --]
[-- Type: text/plain, Size: 2922 bytes --]

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index fe1badee9..6ccced8fa 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -2824,18 +2824,20 @@ changed.  Or in other words, it can detect motion.")
     (license license:gpl2)))
 
 (define-public subdl
-  (let
-      ((commit "4cf5789b11f0ff3f863b704b336190bf968cd471")
-       (revision "1"))
+  (let ((commit "4cf5789b11f0ff3f863b704b336190bf968cd471")
+        (revision "1"))
     (package
       (name "subdl")
-      (version (string-append "1.0.3-" revision "." (string-take commit 7)))
+      (version (git-version "1.0.3" revision commit))
       (source (origin
                 (method git-fetch)
                 (uri (git-reference
                       (url "https://github.com/alexanderwink/subdl.git")
                       (commit commit)))
-                (sha256 (base32 "0kmk5ck1j49q4ww0lvas2767kwnzhkq0vdwkmjypdx5zkxz73fn8"))))
+                (file-name (git-file-name name version))
+                (sha256
+                 (base32
+                  "0kmk5ck1j49q4ww0lvas2767kwnzhkq0vdwkmjypdx5zkxz73fn8"))))
       (build-system trivial-build-system)
       (arguments
        `(#:modules ((guix build utils))
@@ -2845,18 +2847,14 @@ changed.  Or in other words, it can detect motion.")
                             (bin (string-append out "/bin"))
                             (source (assoc-ref %build-inputs "source"))
                             (python (assoc-ref %build-inputs "python")))
-                       (mkdir-p bin)
-                       (with-directory-excursion bin
-                         (copy-file (string-append source "/subdl") "subdl")
-                         (patch-shebang "subdl"
-                                        (list (string-append python "/bin")))
-                         (chmod "subdl" #o555))))))
+                       (install-file (string-append source "/subdl") bin)
+                       (patch-shebang (string-append bin "/subdl")
+                                      (list (string-append python "/bin")))))))
       (inputs `(("python" ,python)))
       (synopsis "Command-line tool for downloading subtitles from opensubtitles.org")
-      (description
-       "Subdl is a command-line tool for downloading subtitles from opensubtitles.org.
-By default, it will search for English subtitles, display the results,
-download the highest-rated result in the requested language and save it to the
-appropriate filename.")
+      (description "Subdl is a command-line tool for downloading subtitles from
+opensubtitles.org.  By default, it will search for English subtitles, display
+the results, download the highest-rated result in the requested language and
+save it to the appropriate filename.")
       (license license:gpl3+)
       (home-page "https://github.com/alexanderwink/subdl"))))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]