Re: [PATCH 5/6] gnu: Add borg.

[Alex Kost]

Leo Famulari (2016-03-22 01:52 +0300) wrote:

> On Mon, Mar 21, 2016 at 06:35:19PM -0400, Leo Famulari wrote:
>> I asked upstream about some warnings during the docs generation, and
>> they pointed out that I needed to add a command line. I've attached a
>> revised patch.
>>
>> Can I get some feedback on it?
>
> Based on feedback on #guix, I've revised it again. Attached...

> From: Leo Famulari <address@hidden>
> Date: Mon, 14 Mar 2016 19:34:25 -0400
> Subject: [PATCH 5/6] gnu: Add borg.
>
> * gnu/packages/backup.scm (borg): New variable.

[...]
> +(define-public borg
> +  (package
> +    (name "borg")
> +    (version "1.0.0")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (pypi-uri "borgbackup" version))
> +              (sha256
> +               (base32
> +                "0wa6cvqs3rni5nwrgagigchcly8a53rxk56z0zn8iaii2cqrw2sh"))))
> +    (build-system python-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'set-env
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (let ((openssl (assoc-ref inputs "openssl"))
> +                   (lz4 (assoc-ref inputs "lz4")))
> +               (setenv "BORG_OPENSSL_PREFIX" openssl)
> +               (setenv "BORG_LZ4_PREFIX" lz4)
> +               (setenv "PYTHON_EGG_CACHE" "/tmp")
> +             #t)))

If you keep #t inside 'let', then align it to (setenv ...), or move it
away from 'let':

           (lambda* (#:key inputs #:allow-other-keys)
             (let ((openssl (assoc-ref inputs "openssl"))
                   (lz4     (assoc-ref inputs "lz4")))
               (setenv "BORG_OPENSSL_PREFIX" openssl)
               (setenv "BORG_LZ4_PREFIX" lz4)
               (setenv "PYTHON_EGG_CACHE" "/tmp"))
             #t)

Note the difference: another ')' after the last (setenv ...).

> +         (add-after 'install 'docs

Perhaps it is better to call this phase 'install-doc'.  It's a usual
name that can be met in other recipes.  There is also
'install-man-pages' phase name, but I prefer 'install-doc'.

The rest looks good to me, so if you checked that this program works, I
think you can push this patchset.

-- 
Alex