Re: [PATCH 3/3] gnu: Duplicity: Update to 0.7.10

[Eric Bavier <ericbavier@openmailbox.org>]

On Tue, 20 Sep 2016 21:36:02 +1000
Brendan Tildesley <brendan.tildesley@openmailbox.org> wrote:

> * gnu/packages/backup.scm (duplicity): Update to 0.7.10.
> * gnu/packages/patches/duplicity-piped-password.patch: Deleted.
> * gnu/packages/patches/duplicity-test_selection-tmp.patch: Deleted.

These patches need to also be removed from gnu/local.mk
(dist_patch_DATA).  And there should be a [source] note that mentions
the patch removal.

> ---
>  gnu/packages/backup.scm                            | 82 +++++++++++++---------
>  .../patches/duplicity-piped-password.patch         | 20 ------
>  .../patches/duplicity-test_selection-tmp.patch     | 18 -----
>  3 files changed, 47 insertions(+), 73 deletions(-)
>  delete mode 100644 gnu/packages/patches/duplicity-piped-password.patch
>  delete mode 100644 gnu/packages/patches/duplicity-test_selection-tmp.patch
> 
> diff --git a/gnu/packages/backup.scm b/gnu/packages/backup.scm
> index e77c4f2..408389a 100644
> --- a/gnu/packages/backup.scm
> +++ b/gnu/packages/backup.scm
> @@ -2,6 +2,7 @@
>  ;;; Copyright © 2014, 2015 Eric Bavier <bavier@member.fsf.org>
>  ;;; Copyright © 2014 Ian Denhardt <ian@zenhack.net>
>  ;;; Copyright © 2015, 2016 Leo Famulari <leo@famulari.name>
> +;;; Copyright © 2016 Brendan Tildesley <brendan.tildesley@openmailbox.org>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -33,6 +34,7 @@
>    #:use-module (gnu packages compression)
>    #:use-module (gnu packages databases)
>    #:use-module (gnu packages dejagnu)
> +  #:use-module (gnu packages ftp)
>    #:use-module (gnu packages glib)
>    #:use-module (gnu packages gnupg)
>    #:use-module (gnu packages gperf)
> @@ -51,53 +53,63 @@
>  (define-public duplicity
>    (package
>      (name "duplicity")
> -    (version "0.6.26")
> +    (version "0.7.10")
>      (source
>       (origin
> -      (method url-fetch)
> -      (uri (string-append "https://code.launchpad.net/duplicity/"
> -                          (version-major+minor version)
> -                          "-series/" version "/+download/duplicity-"
> -                          version ".tar.gz"))
> -      (sha256
> -       (base32
> -        "0jh79syhr8n3l81jxlwsmwm1pklb4d923m2lgqbswyavh1fqmvwb"))
> -      (patches (search-patches "duplicity-piped-password.patch"
> -                               "duplicity-test_selection-tmp.patch"))))
> +       (method url-fetch)
> +       (uri (string-append "https://code.launchpad.net/duplicity/"
> +                           (version-major+minor version)
> +                           "-series/" version "/+download/duplicity-"
> +                           version ".tar.gz"))
> +       (sha256
> +        (base32
> +         "0hj8451gpndyqdfxwhbxwr9ddxvc47g0iv003q33q5kda8ayyps8"))))

This includes a lot of unnecessary whitespace changes.

>      (build-system python-build-system)
>      (native-inputs
>       `(("python2-setuptools" ,python2-setuptools)
> -       ("util-linux" ,util-linux)))     ;setsid command, for the tests
> +       ("python2-mock" ,python2-mock)
> +       ("util-linux" ,util-linux))) ; setsid command, for the tests
>      (inputs
>       `(("python" ,python-2)
>         ("librsync" ,librsync)
> -       ("mock" ,python2-mock)           ;for testing
> -       ("lockfile" ,python2-lockfile)
> -       ("gnupg" ,gnupg-1)               ;gpg executable needed
> -       ("util-linux" ,util-linux)       ;for setsid
> -       ("tzdata" ,tzdata)))
> +       ("python2-lockfile" ,python2-lockfile)
> +       ("python2-pexpect" ,python2-pexpect)
> +       ("python2-paramiko" ,python2-paramiko)
> +       ("python2-pycrypto" ,python2-pycrypto)
> +       ("python2-botocore" ,python2-botocore)
> +       ("python2-dropbox" ,python2-dropbox)

Could you move the new inputs to a separate commit?  I.e. upgrade
duplicity in one commit, then followup with another commit that adds
the new backend inputs?

> +       ("lftp" ,lftp)
> +       ("gnupg" ,gnupg-1)
> +       ("ncftp" ,ncftp)
> +       ("tzdata" ,tzdata)
> +       ("par2cmdline" ,par2cmdline)))
>      (arguments
> -     `(#:python ,python-2               ;setup assumes Python 2
> +     `(#:python ,python-2

Why remove this comment?

>         #:test-target "test"
> -       #:phases (alist-cons-before
> -                 'check 'check-setup
> -                 (lambda* (#:key inputs #:allow-other-keys)
> -                   (substitute* "testing/functional/__init__.py"
> -                     (("/bin/sh") (which "sh")))
> -                   (setenv "HOME" (getcwd)) ;gpg needs to write to $HOME
> -                   (setenv "TZDIR"          ;some timestamp checks need TZDIR
> -                           (string-append (assoc-ref inputs "tzdata")
> -                                          "/share/zoneinfo")))
> -                 %standard-phases)))
> +       #:phases
> +       (modify-phases %standard-phases

Could you make the transition to modify-phases in a separate commit?

> +         (add-before 'check 'check-setup
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (substitute* '("testing/functional/__init__.py"
> +                            "testing/overrides/bin/lftp")
> +               (("/bin/sh") (which "sh")))
> +             (setenv "HOME" (getcwd))
                                          ^
Again, why remove the comment?

> +             ;; RUN_CODE_TESTS can be set to enable unessential PEP8, 2to3 and pylint
> +             ;; tests that currently fail due to lacking dependencies
> +             ;; (setenv "RUN_CODE_TESTS" "1")
> +             ;;

Let's leave this out then, or just leave a small "TODO" comment.

> +             ;; Backup filenames embed TZ data.
> +             ;; See testing/unit/test_statistics.py test_get_stats_string(self)
> +             (setenv "TZDIR" (string-append (assoc-ref inputs "tzdata")
> +                                            "/share/zoneinfo")))))))

This comment change seems gratuitous.

>      (home-page "http://duplicity.nongnu.org/index.html")
>      (synopsis "Encrypted backup using rsync algorithm")
> -    (description
> -     "Duplicity backs up directories by producing encrypted tar-format volumes
> -and uploading them to a remote or local file server.  Because duplicity uses
> -librsync, the incremental archives are space efficient and only record the
> -parts of files that have changed since the last backup.  Because duplicity
> -uses GnuPG to encrypt and/or sign these archives, they will be safe from
> -spying and/or modification by the server.")
> +    (description "Duplicity backs up directories by producing encrypted
> +tar-format volumes and uploading them to a remote or local file server.  
> +Because duplicity uses librsync, the incremental archives are space efficient
> +and only record the parts of files that have changed since the last backup.  
> +Because duplicity uses GnuPG to encrypt and/or sign these archives, they will
> +be safe from spying and/or modification by the server.")

Let's leave out the nnecessary whitespace changes.

Could you send an updated patch?
Thanks,
`~Eric