GNU bug report logs - #26112
[PATCH 2/7] gnu: Add niftilib.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26112 in the body.
You can then email your comments to 26112 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: John Darrington <jmd <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: John Darrington <jmd <at> gnu.org>
Subject: [PATCH 2/7] gnu: Add niftilib.
Date: Wed, 15 Mar 2017 21:05:19 +0100

* gnu/packages/image.scm (niftilib): New variable.
---
 gnu/packages/image.scm | 58 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/image.scm b/gnu/packages/image.scm
index 53ed69a..82ea89c 100644
--- a/gnu/packages/image.scm
+++ b/gnu/packages/image.scm
@@ -6,7 +6,7 @@
 ;;; Copyright © 2014, 2016 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2015 Taylan Ulrich Bayırlı/Kammer <taylanbayirli <at> gmail.com>
 ;;; Copyright © 2015 Amirouche Boubekki <amirouche <at> hypermove.net>
-;;; Copyright © 2014 John Darrington <jmd <at> gnu.org>
+;;; Copyright © 2014, 2017 John Darrington <jmd <at> gnu.org>
 ;;; Copyright © 2016 Leo Famulari <leo <at> famulari.name>
 ;;; Copyright © 2016, 2017 Leo Famulari <leo <at> famulari.name>
 ;;; Copyright © 2016, 2017 Efraim Flashner <efraim <at> flashner.co.il>
@@ -1114,3 +1114,59 @@ interface.  It supports color space extensions that allow it to compress from
 and decompress to 32-bit and big-endian pixel buffers (RGBX, XBGR, etc.).")
     (license (list license:bsd-3        ; jsimd*.[ch] and most of simd/
                    license:ijg))))      ; the rest
+
+(define-public niftilib
+  (package
+   (name "niftilib")
+   (version "2.0.0")
+   (source (origin
+            (method url-fetch)
+            (uri (list (string-append "mirror://sourceforge/niftilib/"
+                                      "nifticlib/nifticlib_2_0_0/"
+                                      "/nifticlib-" version ".tar.gz")))
+            (sha256
+             (base32 "123z9bwzgin5y8gi5ni8j217k7n683whjsvg0lrpii9flgk8isd3"))))
+   (build-system gnu-build-system)
+   (arguments
+    '(#:tests? #f
+      #:parallel-build? #f
+      #:phases
+      (modify-phases %standard-phases
+        (replace 'install
+                 (lambda _
+                   (for-each
+                    (lambda (dir)
+                      (let ((directory (assoc-ref %outputs "out")))
+                        (mkdir-p (string-append directory "/" dir))
+                        (zero? (system* "cp" "-a" dir directory))))
+                    '("bin" "lib" "include"))))
+        (replace 'configure
+                 (lambda _
+                   (substitute* "Makefile"
+                     (("^SHELL[ \t]*=[ \t]*csh")
+                      (string-append "SHELL = "
+                                     (assoc-ref %build-inputs "bash")
+                                     "/bin/sh"))
+
+                     (("^CFLAGS[ \t]*=[ \t]\\$\\(ANSI_FLAGS\\)")
+                      "CFLAGS = $(ANSI_FLAGS) -fPIC")
+
+                     (("^ZLIB_INC[ \t]*=[ \t]*-I/usr/include")
+                      (string-append "ZLIB_INC = -I"
+                                     (assoc-ref %build-inputs "zlib")
+                                     "/include"))
+
+                     (("^CP[ \t]*=[ \t]*cp")
+                      (string-append "CP = "
+                                     (assoc-ref %build-inputs "coreutils")
+                                     "/bin/cp")))
+                   #t)))))
+   (inputs
+    `(("zlib" ,zlib)))
+   (synopsis "Library for reading and writing files in the nifti-1 format")
+   (description   "Niftilib is a set of i/o libraries for reading and writing
+files in the nifti-1 data format - a binary file format for storing
+medical image data, e.g. magnetic resonance image (MRI) and functional MRI
+(fMRI) brain images.")
+   (home-page "http://niftilib.sourceforge.net")
+   (license license:public-domain)))
-- 
2.1.4

Message #8 received at 26112 <at> debbugs.gnu.org (full text, mbox):

From: Kei Kebreau <kei <at> openmailbox.org>
To: John Darrington <jmd <at> gnu.org>
Cc: 26112 <at> debbugs.gnu.org
Subject: Re: bug#26112: [PATCH 2/7] gnu: Add niftilib.
Date: Fri, 17 Mar 2017 16:18:31 -0400

[Message part 1 (text/plain, inline)]

John Darrington <jmd <at> gnu.org> writes:

> * gnu/packages/image.scm (niftilib): New variable.
> ---
>  gnu/packages/image.scm | 58 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/image.scm b/gnu/packages/image.scm
> index 53ed69a..82ea89c 100644
> --- a/gnu/packages/image.scm
> +++ b/gnu/packages/image.scm
> @@ -6,7 +6,7 @@
>  ;;; Copyright © 2014, 2016 Ricardo Wurmus <rekado <at> elephly.net>
>  ;;; Copyright © 2015 Taylan Ulrich Bayırlı/Kammer <taylanbayirli <at> gmail.com>
>  ;;; Copyright © 2015 Amirouche Boubekki <amirouche <at> hypermove.net>
> -;;; Copyright © 2014 John Darrington <jmd <at> gnu.org>
> +;;; Copyright © 2014, 2017 John Darrington <jmd <at> gnu.org>
>  ;;; Copyright © 2016 Leo Famulari <leo <at> famulari.name>
>  ;;; Copyright © 2016, 2017 Leo Famulari <leo <at> famulari.name>
>  ;;; Copyright © 2016, 2017 Efraim Flashner <efraim <at> flashner.co.il>
> @@ -1114,3 +1114,59 @@ interface.  It supports color space extensions that allow it to compress from
>  and decompress to 32-bit and big-endian pixel buffers (RGBX, XBGR, etc.).")
>      (license (list license:bsd-3        ; jsimd*.[ch] and most of simd/
>                     license:ijg))))      ; the rest
> +
> +(define-public niftilib
> +  (package
> +   (name "niftilib")
> +   (version "2.0.0")
> +   (source (origin
> +            (method url-fetch)
> +            (uri (list (string-append "mirror://sourceforge/niftilib/"
> +                                      "nifticlib/nifticlib_2_0_0/"

Instead of 2_0_0, you could use the trusty

(string-join (string-split version #\.) "_")

trick. It's less error-prone when updates come around.

> +                                      "/nifticlib-" version ".tar.gz")))
> +            (sha256
> +             (base32 "123z9bwzgin5y8gi5ni8j217k7n683whjsvg0lrpii9flgk8isd3"))))
> +   (build-system gnu-build-system)
> +   (arguments
> +    '(#:tests? #f
> +      #:parallel-build? #f
> +      #:phases
> +      (modify-phases %standard-phases
> +        (replace 'install
> +                 (lambda _
> +                   (for-each
> +                    (lambda (dir)
> +                      (let ((directory (assoc-ref %outputs "out")))
> +                        (mkdir-p (string-append directory "/" dir))
> +                        (zero? (system* "cp" "-a" dir directory))))
> +                    '("bin" "lib" "include"))))
> +        (replace 'configure
> +                 (lambda _
> +                   (substitute* "Makefile"
> +                     (("^SHELL[ \t]*=[ \t]*csh")
> +                      (string-append "SHELL = "
> +                                     (assoc-ref %build-inputs "bash")
> +                                     "/bin/sh"))
> +
> +                     (("^CFLAGS[ \t]*=[ \t]\\$\\(ANSI_FLAGS\\)")
> +                      "CFLAGS = $(ANSI_FLAGS) -fPIC")
> +
> +                     (("^ZLIB_INC[ \t]*=[ \t]*-I/usr/include")
> +                      (string-append "ZLIB_INC = -I"
> +                                     (assoc-ref %build-inputs "zlib")
> +                                     "/include"))
> +
> +                     (("^CP[ \t]*=[ \t]*cp")
> +                      (string-append "CP = "
> +                                     (assoc-ref %build-inputs "coreutils")
> +                                     "/bin/cp")))
> +                   #t)))))
> +   (inputs
> +    `(("zlib" ,zlib)))
> +   (synopsis "Library for reading and writing files in the nifti-1 format")
> +   (description   "Niftilib is a set of i/o libraries for reading and writing

Why is there a triple space between "description" and the double quote?
Am I correct in assuming that it's accidental?

> +files in the nifti-1 data format - a binary file format for storing
> +medical image data, e.g. magnetic resonance image (MRI) and functional MRI
> +(fMRI) brain images.")
> +   (home-page "http://niftilib.sourceforge.net")
> +   (license license:public-domain)))

Other than that, LGTM.

[signature.asc (application/pgp-signature, inline)]

Message #13 received at 26112-done <at> debbugs.gnu.org (full text, mbox):

From: John Darrington <john <at> darrington.wattle.id.au>
To: 26112-done <at> debbugs.gnu.org
Subject: Done
Date: Sat, 18 Mar 2017 15:35:38 +0100

I pushed this.  Thanks.

Message #16 received at 26112 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: John Darrington <jmd <at> gnu.org>
Cc: 26112 <at> debbugs.gnu.org
Subject: Re: bug#26112: [PATCH 2/7] gnu: Add niftilib.
Date: Sun, 19 Mar 2017 14:11:51 +0100

Hi John,

thanks for the contribution.

> * gnu/packages/image.scm (niftilib): New variable.

I realize this has been pushed already, but here are some comments for
the future.

> +
> +(define-public niftilib
> +  (package
> +   (name "niftilib")
> +   (version "2.0.0")
> +   (source (origin
> +            (method url-fetch)
> +            (uri (list (string-append "mirror://sourceforge/niftilib/"
> +                                      "nifticlib/nifticlib_2_0_0/"
> +                                      "/nifticlib-" version ".tar.gz")))
> +            (sha256
> +             (base32 "123z9bwzgin5y8gi5ni8j217k7n683whjsvg0lrpii9flgk8isd3"))))
> +   (build-system gnu-build-system)
> +   (arguments
> +    '(#:tests? #f
> +      #:parallel-build? #f

Please add simple comments explaining why this is required.

> +      #:phases
> +      (modify-phases %standard-phases
> +        (replace 'install
> +                 (lambda _
> +                   (for-each
> +                    (lambda (dir)
> +                      (let ((directory (assoc-ref %outputs "out")))
> +                        (mkdir-p (string-append directory "/" dir))
> +                        (zero? (system* "cp" "-a" dir directory))))
> +                    '("bin" "lib" "include"))))

Instead of starting a shell process to copy things please use
“copy-recursively” or “install-file” instead.  It’s also better style to
pull the let binding out of the lambda.

Finally, please end the phase on “#t”.

> +        (replace 'configure
> +                 (lambda _
> +                   (substitute* "Makefile"
> +                     (("^SHELL[ \t]*=[ \t]*csh")
> +                      (string-append "SHELL = "
> +                                     (assoc-ref %build-inputs "bash")
> +                                     "/bin/sh"))
> +
> +                     (("^CFLAGS[ \t]*=[ \t]\\$\\(ANSI_FLAGS\\)")
> +                      "CFLAGS = $(ANSI_FLAGS) -fPIC")
> +
> +                     (("^ZLIB_INC[ \t]*=[ \t]*-I/usr/include")
> +                      (string-append "ZLIB_INC = -I"
> +                                     (assoc-ref %build-inputs "zlib")
> +                                     "/include"))
> +
> +                     (("^CP[ \t]*=[ \t]*cp")
> +                      (string-append "CP = "
> +                                     (assoc-ref %build-inputs "coreutils")
> +                                     "/bin/cp")))
> +                   #t)))))

It is preferrable to use #:make-flags instead of patching the Makefile.
When referencing inputs, please use “lambda* (#:key inputs
#:allow-other-keys)” instead of “lambda _” and “%build-inputs”.

It would also be good to check the indentation, which can be done with
“etc/indent-code.el”.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

Message #19 received at 26112 <at> debbugs.gnu.org (full text, mbox):

From: John Darrington <john <at> darrington.wattle.id.au>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26112 <at> debbugs.gnu.org, John Darrington <jmd <at> gnu.org>
Subject: Re: bug#26112: [PATCH 2/7] gnu: Add niftilib.
Date: Sun, 19 Mar 2017 14:33:17 +0100

[Message part 1 (text/plain, inline)]

Thanks for the heads up.  

I'm not sure that all your suggestions will work, but I will try what you suggest
and push follow-up commit.

Regarding indentation - I normally let emacs handle it. Shouldn't that get everything
correct?

J'


On Sun, Mar 19, 2017 at 02:11:51PM +0100, Ricardo Wurmus wrote:
     
     Hi John,
     
     thanks for the contribution.
     
     > * gnu/packages/image.scm (niftilib): New variable.
     
     I realize this has been pushed already, but here are some comments for
     the future.
     
     > +
     > +(define-public niftilib
     > +  (package
     > +   (name "niftilib")
     > +   (version "2.0.0")
     > +   (source (origin
     > +            (method url-fetch)
     > +            (uri (list (string-append "mirror://sourceforge/niftilib/"
     > +                                      "nifticlib/nifticlib_2_0_0/"
     > +                                      "/nifticlib-" version ".tar.gz")))
     > +            (sha256
     > +             (base32 "123z9bwzgin5y8gi5ni8j217k7n683whjsvg0lrpii9flgk8isd3"))))
     > +   (build-system gnu-build-system)
     > +   (arguments
     > +    '(#:tests? #f
     > +      #:parallel-build? #f
     
     Please add simple comments explaining why this is required.
     
     > +      #:phases
     > +      (modify-phases %standard-phases
     > +        (replace 'install
     > +                 (lambda _
     > +                   (for-each
     > +                    (lambda (dir)
     > +                      (let ((directory (assoc-ref %outputs "out")))
     > +                        (mkdir-p (string-append directory "/" dir))
     > +                        (zero? (system* "cp" "-a" dir directory))))
     > +                    '("bin" "lib" "include"))))
     
     Instead of starting a shell process to copy things please use
     ???copy-recursively??? or ???install-file??? instead.  It???s also better style to
     pull the let binding out of the lambda.
     
     Finally, please end the phase on ???#t???.
     
     > +        (replace 'configure
     > +                 (lambda _
     > +                   (substitute* "Makefile"
     > +                     (("^SHELL[ \t]*=[ \t]*csh")
     > +                      (string-append "SHELL = "
     > +                                     (assoc-ref %build-inputs "bash")
     > +                                     "/bin/sh"))
     > +
     > +                     (("^CFLAGS[ \t]*=[ \t]\\$\\(ANSI_FLAGS\\)")
     > +                      "CFLAGS = $(ANSI_FLAGS) -fPIC")
     > +
     > +                     (("^ZLIB_INC[ \t]*=[ \t]*-I/usr/include")
     > +                      (string-append "ZLIB_INC = -I"
     > +                                     (assoc-ref %build-inputs "zlib")
     > +                                     "/include"))
     > +
     > +                     (("^CP[ \t]*=[ \t]*cp")
     > +                      (string-append "CP = "
     > +                                     (assoc-ref %build-inputs "coreutils")
     > +                                     "/bin/cp")))
     > +                   #t)))))
     
     It is preferrable to use #:make-flags instead of patching the Makefile.
     When referencing inputs, please use ???lambda* (#:key inputs
     #:allow-other-keys)??? instead of ???lambda _??? and ???%build-inputs???.
     
     It would also be good to check the indentation, which can be done with
     ???etc/indent-code.el???.
     
     --
     Ricardo
     
     GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
     https://elephly.net

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.

[signature.asc (application/pgp-signature, inline)]

Message #22 received at 26112 <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: John Darrington <john <at> darrington.wattle.id.au>
Cc: 26112 <at> debbugs.gnu.org, John Darrington <jmd <at> gnu.org>
Subject: Re: bug#26112: [PATCH 2/7] gnu: Add niftilib.
Date: Sun, 19 Mar 2017 15:03:08 +0100

John Darrington <john <at> darrington.wattle.id.au> writes:

> I'm not sure that all your suggestions will work, but I will try what you suggest
> and push follow-up commit.

I’ve already done that with commit
f729a4d8b274aebd578f45b2ca55cded31001b85.  It builds fine for me.

> Regarding indentation - I normally let emacs handle it. Shouldn't that get everything
> correct?

We give a couple of hints to Emacs to indent our special forms
differently.  This requires use of the “emacs-guix” package, if I’m not
mistaken.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

Message #25 received at 26112 <at> debbugs.gnu.org (full text, mbox):

From: John Darrington <john <at> darrington.wattle.id.au>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 26112 <at> debbugs.gnu.org, John Darrington <john <at> darrington.wattle.id.au>,
 John Darrington <jmd <at> gnu.org>
Subject: Re: bug#26112: [PATCH 2/7] gnu: Add niftilib.
Date: Sun, 19 Mar 2017 15:47:22 +0100

[Message part 1 (text/plain, inline)]

On Sun, Mar 19, 2017 at 03:03:08PM +0100, Ricardo Wurmus wrote:
     
     John Darrington <john <at> darrington.wattle.id.au> writes:
     
     > I'm not sure that all your suggestions will work, but I will try what you suggest
     > and push follow-up commit.
     
     I???ve already done that with commit
     f729a4d8b274aebd578f45b2ca55cded31001b85.  It builds fine for me.

Hmm ok.  I have a nasty feeling that this may have broken some of my yet-to-be-committed, 
dependent packages.  But if it has, we'll cross that bridge when we come to it.  Thanks
for the fixup anyway.

     
     > Regarding indentation - I normally let emacs handle it. Shouldn't that get everything
     > correct?
     
     We give a couple of hints to Emacs to indent our special forms
     differently.  This requires use of the ???emacs-guix??? package, if I???m not
     mistaken.
     
Ok I will install that.  Thanks for the hint.

J'

-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.