all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#31017] [PATCH] gnu: Add subdl
@ 2018-04-01 15:15 Pierre Neidhardt
  2018-04-01 15:24 ` Peter Neidhardt
  2018-04-02 21:02 ` Leo Famulari
  0 siblings, 2 replies; 6+ messages in thread
From: Pierre Neidhardt @ 2018-04-01 15:15 UTC (permalink / raw)
  To: 31017

* 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [bug#31017] [PATCH] gnu: Add subdl
  2018-04-01 15:15 [bug#31017] [PATCH] gnu: Add subdl Pierre Neidhardt
@ 2018-04-01 15:24 ` Peter Neidhardt
  2018-04-02 21:02 ` Leo Famulari
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Neidhardt @ 2018-04-01 15:24 UTC (permalink / raw)
  To: 31017

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#31017] [PATCH] gnu: Add subdl
  2018-04-01 15:15 [bug#31017] [PATCH] gnu: Add subdl Pierre Neidhardt
  2018-04-01 15:24 ` Peter Neidhardt
@ 2018-04-02 21:02 ` Leo Famulari
  2018-04-03  4:19   ` Pierre Neidhardt
  1 sibling, 1 reply; 6+ messages in thread
From: Leo Famulari @ 2018-04-02 21:02 UTC (permalink / raw)
  To: Pierre Neidhardt; +Cc: 31017


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [bug#31017] [PATCH] gnu: Add subdl
  2018-04-02 21:02 ` Leo Famulari
@ 2018-04-03  4:19   ` Pierre Neidhardt
  2018-04-04 11:43     ` Ludovic Courtès
  2018-04-05  6:30     ` bug#31017: " Leo Famulari
  0 siblings, 2 replies; 6+ messages in thread
From: Pierre Neidhardt @ 2018-04-03  4:19 UTC (permalink / raw)
  To: Leo Famulari; +Cc: 31017

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [bug#31017] [PATCH] gnu: Add subdl
  2018-04-03  4:19   ` Pierre Neidhardt
@ 2018-04-04 11:43     ` Ludovic Courtès
  2018-04-05  6:30     ` bug#31017: " Leo Famulari
  1 sibling, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2018-04-04 11:43 UTC (permalink / raw)
  To: Pierre Neidhardt; +Cc: 31017

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’.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#31017: [PATCH] gnu: Add subdl
  2018-04-03  4:19   ` Pierre Neidhardt
  2018-04-04 11:43     ` Ludovic Courtès
@ 2018-04-05  6:30     ` Leo Famulari
  1 sibling, 0 replies; 6+ messages in thread
From: Leo Famulari @ 2018-04-05  6:30 UTC (permalink / raw)
  To: Pierre Neidhardt; +Cc: 31017-done

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-04-05  6:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-01 15:15 [bug#31017] [PATCH] gnu: Add subdl Pierre Neidhardt
2018-04-01 15:24 ` Peter Neidhardt
2018-04-02 21:02 ` Leo Famulari
2018-04-03  4:19   ` Pierre Neidhardt
2018-04-04 11:43     ` Ludovic Courtès
2018-04-05  6:30     ` bug#31017: " Leo Famulari

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.