Re: [PATCH] envstore-2.1

[Matthew Jordan]

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 <address@hidden>
Date: Thu, 12 May 2016 14:57:34 -0400
Subject: [PATCH] gnu: Add shellutils.

* gnu/package/shellutils.scm: New file.
---
 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 <address@hidden>
+;;;
+;;; 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 <address@hidden> writes:
>
>> From 8de06b6e26d9e1eb7bb7ef6df163f54a46db3d89 Mon Sep 17 00:00:00 2001
>> From: Matthew Jordan <address@hidden>
>> 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