Re: [PATCH] Add wayback-machine-downloader

[Ben Woodcroft]

On 01/04/16 10:05, address@hidden wrote:

Hi everyone!

This is my first patch, please double check it. It adds a package with a self-explanatory name: wayback-machine-downloader. It still uses the original package name in the command line (wayback_machine_downloader), for compatibility with upstream. I have tested it with Guix on top of Debian.

I'd like to thank Ben Woodcroft for answering all the questions I had! :)

Hi again Vincent, and no problem.

Getting there, but still a few things to tweak.

From a43ed2194a17d8e97b4f0cd41d0bc0b4873dcb42 Mon Sep 17 00:00:00 2001
From: Vincent Cloutier <address@hidden>
Date: Thu, 31 Mar 2016 20:02:56 -0400
Subject: [PATCH] Add wayback-machine-downloader

---
 gnu/packages/web.scm | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index 516e623..cc65dc9 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -35,12 +35,14 @@
   #:use-module (guix build-system perl)
   #:use-module (guix build-system cmake)
   #:use-module (guix build-system r)
+  #:use-module (guix build-system ruby)
   #:use-module (gnu packages)
   #:use-module (gnu packages apr)
   #:use-module (gnu packages asciidoc)
   #:use-module (gnu packages docbook)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages compression)
+  #:use-module (gnu packages ruby)
   #:use-module (gnu packages cyrus-sasl)
   #:use-module (gnu packages databases)
   #:use-module (gnu packages mit-krb5)
@@ -3109,3 +3111,33 @@ callback or connection interfaces.")
      "Gumbo is an implementation of the HTML5 parsing algorithm implemented as
 a pure C99 library.")
     (license l:asl2.0)))
+
+ 
+ (define-public wayback-machine-downloader
+   (package
+     (name "wayback-machine-downloader")
+     (version "0.2.1")
+     (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+              (url "https://github.com/hartator/wayback-machine-downloader.git")
+              (commit "7000035a304")))
+         (sha256
+           (base32
+             "0p8q0qpfq6b9akapypyxyvdj12kyz3xw3c2fpg4nxrzq0f7lq6dl"))))
+     (build-system ruby-build-system)
+     (arguments
+       `(#:tests? #f )) ; no test, tests need to access archive.org 

Ah, so we went to the git-fetch because the tests were not included, but then it seems the tests require networking so they get disabled anyway. So then I think it might be best to revert your original method of downloading via rubygems, because then updates can be auto-detected. However, it is good to do at least some rudimentary testing so I'd include something like this:

    (arguments
     `(#:phases
       (modify-phases %standard-phases
         (replace 'check
           (lambda _
             (zero? (system* "wayback_machine_downloader" "-h")))))))

However, this will fail: read Ricardo's advice on wrapping the program.

It seems that lint reports a few things:

$ ./pre-inst-env guix lint wayback-machine-downloader
gnu/packages/web.scm:3121:7: wayback-machine-downloader-0.2.1: the source file name should contain the package name
gnu/packages/web.scm:3117:3: wayback-machine-downloader-0.2.1: trailing white space on line 3131
gnu/packages/web.scm:3117:3: wayback-machine-downloader-0.2.1: trailing white space on line 3137
gnu/packages/web.scm:3117:3: wayback-machine-downloader-0.2.1: trailing white space on line 3138

The first of these will go away when the switch back to downloading from rubygems is done. I also notice that there is a leading space in the package definition which should be removed as well as leading and trailing newlines.

+     (native-inputs
+       `(("ruby-rake-compiler" ,ruby-rake-compiler)))

I don't see why this is needed still? It compiled fine for me without it.

+     (synopsis "Download websites from archive.org's Wayback Machine")
+     (description
+       "Download any website from the Wayback Machine from the command line.
+Wayback Machine by Internet Archive (archive.org) is an awesome tool to view 
+any website at any point of time but lacks an export feature.  Wayback Machine 
+Downloader brings exactly this.")

WDYT of my previous comments about the description?

+     (home-page
+       "https://github.com/hartator/wayback-machine-downloader")
+     (license l:expat)))
+
-- 
2.6.3

Would you mind sending an updated patch please?
Thanks.
ben