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

[Leo Famulari]

On Tue, Mar 22, 2016 at 11:20:37PM +0300, Alex Kost wrote:
> 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'.

Sure, I'll take your advice!

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

I've been using this program for a few months without problems (yes, I
do restore from time to time).

It's being developed very rapidly for something so critical as a backup
program, so I think that it's still a bit experimental. I could allude
to that in the description. What do you think?