Re: [PATCH] envstore-2.1

[Efraim Flashner <efraim@flashner.co.il>]

[-- Attachment #1: Type: text/plain, Size: 7707 bytes --]

On Sun, May 15, 2016 at 07:16:33AM -0400, Matthew Jordan wrote:
> 
> Good day Mark H Weaver,
> 
> I have updated the patch in accordance with your suggestions. And thank
> you for the feed back you provided in your previous email. This only the
> second package I have submitted so far.
> 
> Also I renamed to the source file to shellutils.scm, let me know if this
> is acceptable. As I didn't find any other files with general names
> implying a reference to command line utilities.
> 

> From 7c85692c7e844d84c568fc779846b5492a98bc2b Mon Sep 17 00:00:00 2001
> From: Matthew Jordan <matthewjordandevops@yandex.com>
> Date: Thu, 12 May 2016 14:57:34 -0400
> Subject: [PATCH] gnu: Add shellutils.
> 
> * gnu/package/shellutils.scm: New file.

Don't forget to register the file in gnu/local.mk

> ---
>  gnu/packages/shellutils.scm | 50 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>  create mode 100644 gnu/packages/shellutils.scm
> 
> diff --git a/gnu/packages/shellutils.scm b/gnu/packages/shellutils.scm
> new file mode 100644
> index 0000000..2c60607
> --- /dev/null
> +++ b/gnu/packages/shellutils.scm
> @@ -0,0 +1,50 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2015 Taylan Ulrich Bayırlı/Kammer <taylanbayirli@gmail.com>
                    ^^^
One of the fun bits of getting your patches into guix is putting _your_
name (and the current year) in the header ;)

> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (gnu packages shellutils)
> +  #:use-module (guix licenses)
> +  #:use-module (guix packages)
> +  #:use-module (guix download)
> +  #:use-module (guix utils)
> +  #:use-module (guix build-system gnu))
> +
> +(define-public envstore
> +  (package
> +    (name "envstore")
> +    (version "2.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "https://finalrewind.org/projects/"
> +                           name "/" name "-" version ".tar.bz2"))
> +       (sha256
> +        (base32 "1x97lxad80m5blhdfanl5v2qzjwcgbij2i23701bn8mpyxsrqszi"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:test-target "test"
> +       #:make-flags (list "CC=gcc"
> +                          (string-append "PREFIX=" (assoc-ref %outputs "out")))
> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure))))
> +    (home-page "https://finalrewind.org/projects/envstore/")
> +    (synopsis "Save and restore environment variables")
> +    (description "Envstore is a program for sharing environment variables
> +between various shells or commands.")
> +    (license
> +     (non-copyleft "http://www.wtfpl.net/txt/copying/"))))
> -- 
> 2.7.4
> 

> 
> Sincerely,
> 
> -- 
> Matthew Jordan
> Sent with my mu4e
> 
> 
> Mark H Weaver writes:
> 
> > Hi,
> >
> > Matthew Jordan <matthewjordandevops@yandex.com> writes:
> >
> >> From 8de06b6e26d9e1eb7bb7ef6df163f54a46db3d89 Mon Sep 17 00:00:00 2001
> >> From: Matthew Jordan <matthewjordandevops@yandex.com>
> >> Date: Thu, 12 May 2016 14:57:34 -0400
> >> Subject: [PATCH] gnu: Added envstore package.
> >
> > The summary line should be "gnu: Add envstore."
> >
> >>
> >> * gnu/package/enstore.scm: New file.
> >
> > You misspelled "envstore.scm", but it would be better to find an
> > existing file in gnu/package/*.scm that would be appropriate for this.
> >
> >> diff --git a/gnu/packages/envstore.scm b/gnu/packages/envstore.scm
> >> new file mode 100644
> >> index 0000000..e3ec99d
> >> --- /dev/null
> >> +++ b/gnu/packages/envstore.scm
> >> @@ -0,0 +1,42 @@
> >> +(define-module (gnu packages envstore)
> >
> > When adding a new *.scm file, it needs to contain a copyright notice and
> > header at the top, as with our other source files.
> >
> >> +  #:use-module (guix)
> >> +  #:use-module (guix packages)
> >> +  #:use-module (guix build-system gnu)
> >> +  #:use-module (gnu packages)
> >> +  #:use-module (guix download)
> >> +  #:use-module (guix utils)
> >> +  #:use-module (guix licenses))
> >> +
> >> +(define-public envstore
> >> +  (package
> >> +    (name "envstore")
> >> +    (version "2.1")
> >> +    (source
> >> +     (origin
> >> +       (method url-fetch)
> >> +       (uri (string-append "https://github.com/derf/" name "/archive/"
> >> +                           version ".tar.gz"))
> >
> > How about using
> >
> >   https://finalrewind.org/projects/envstore/envstore-2.1.tar.bz2
> >
> > instead?  That's the tarball linked from the project's home page, and
> > unlike the github tarball, it's digitally signed.
> >
> >> +       (sha256
> >> +        (base32 "097yd6w0fql8a3xh0gmz8bf40w61j4893rp8c28rngrrk80bk9a8"))))
> >> +    (build-system gnu-build-system)
> >> +    (arguments
> >> +     `(#:test-target "test"
> >> +       #:phases
> >> +       (modify-phases %standard-phases
> >> +         (delete 'configure)
> >> +         (replace 'build
> >> +           (lambda _
> >> +             (setenv "CC" (which "gcc"))
> >> +             (system* "make")))
> >
> > Instead of replacing the 'build' phase, it would be better to add this
> > to the 'arguments':
> >
> >   #:make-flags (list "CC=gcc")
> >
> > See 'dvtm' in dvtm.scm for an example.
> >
> >> +         (replace 'install
> >> +           (lambda* (#:key outputs #:allow-other-keys)
> >> +             (let ((out (assoc-ref outputs "out")))
> >> +               (setenv "PREFIX" "/")
> >> +               (setenv "DESTDIR" out)
> >> +               (system* "make" "install")))))))
> >
> > These are incorrect settings for PREFIX and DESTDIR.  In general, PREFIX
> > tells where the installed files will be located when the program is run,
> > and DESTDIR names a temporary staging directory where "make install"
> > will put the files, on the assumption that they will later be moved to
> > PREFIX before they are run.
> >
> > So, PREFIX should be set to (assoc-ref outputs "out"), and DESTDIR
> > should be left alone.
> >
> > Also, as with the 'build' phase, it would be better to simply add these
> > to make-flags, like this:
> >
> >   #:make-flags (list "CC=gcc"
> >                      (string-append "PREFIX=" (assoc-ref %outputs "out")))
> >
> >> +    (home-page "https://finalrewind.org/projects/envstore/")
> >> +    (synopsis "Save and restore environment variables")
> >> +    (description "Envstore is a program for sharing environment variables
> >> +between various shells or commands.")
> >> +    (license
> >> +     (non-copyleft "http://www.wtfpl.net/txt/copying/"))))
> >
> > Can you send an updated patch?
> >
> >      Thanks,
> >        Mark


-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]