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

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

[Pierre Neidhardt]

* gnu/packages/video.scm (subdl): New variable.
---
 gnu/packages/video.scm | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 3937c52c0..fe1badee9 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -56,6 +56,7 @@
   #:use-module (guix build-system perl)
   #:use-module (guix build-system python)
   #:use-module (guix build-system waf)
+  #:use-module (guix build-system trivial)
   #:use-module (gnu packages)
   #:use-module (gnu packages algebra)
   #:use-module (gnu packages audio)
@@ -2821,3 +2822,41 @@ changed.  Or in other words, it can detect motion.")
 
     ;; Some files say "version 2" and others "version 2 or later".
     (license license:gpl2)))
+
+(define-public subdl
+  (let
+      ((commit "4cf5789b11f0ff3f863b704b336190bf968cd471")
+       (revision "1"))
+    (package
+      (name "subdl")
+      (version (string-append "1.0.3-" revision "." (string-take commit 7)))
+      (source (origin
+                (method git-fetch)
+                (uri (git-reference
+                      (url "https://github.com/alexanderwink/subdl.git")
+                      (commit commit)))
+                (sha256 (base32 "0kmk5ck1j49q4ww0lvas2767kwnzhkq0vdwkmjypdx5zkxz73fn8"))))
+      (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))))))
+      (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.")
+      (license license:gpl3+)
+      (home-page "https://github.com/alexanderwink/subdl"))))
-- 
2.16.3

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

[Peter Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 406 bytes --]


Since I'm still rather new with Guix packaging API, I've mostly re-used
code from existing packages to write this one.
Hence the strong resemblance with "mb2md" package.

One thing I'm not so sure of however: In mb2md's declaration:

	(chmod "mb2md" #o555)

I'd tend to think it is necessary when we are unsure of upstream's
permissions.
Let me know if I'm wrong here.

-- 
Peter Neidhardt

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

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

[Leo Famulari]

[-- 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 --]

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

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]


Thank you very much for this detailed review!

As for the copyright, the same as this e-mail should show will do:

    Pierre Neidhardt <ambrevar@gmail.com>

I don't know if this matters, but I already have a copyright assignment
for another GNU project, Emms.

Cheers!

-- 
Pierre Neidhardt

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

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

[Ludovic Courtès]

Hello,

Pierre Neidhardt <ambrevar@gmail.com> skribis:

> I don't know if this matters, but I already have a copyright assignment
> for another GNU project, Emms.

Guix contributors remain copyright holders in Guix, we don’t assign
copyright to the FSF, so no worries here!

Ludo’.

bug#31017: [PATCH] gnu: Add subdl

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 318 bytes --]

On Tue, Apr 03, 2018 at 09:49:24AM +0530, Pierre Neidhardt wrote:
> 
> Thank you very much for this detailed review!
> 
> As for the copyright, the same as this e-mail should show will do:
> 
>     Pierre Neidhardt <ambrevar@gmail.com>

Thanks! I pushed the patch as f27f264e51a8de77f01d6798eb9c991cecbcdedb.

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