unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH 1/3] gnu: Add cfitsio
@ 2016-08-30 19:21 John Darrington
  2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: John Darrington @ 2016-08-30 19:21 UTC (permalink / raw)
  To: guix-devel; +Cc: John Darrington

* gnu/packages/astronomy.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
---
 gnu/local.mk               |  1 +
 gnu/packages/astronomy.scm | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
 create mode 100644 gnu/packages/astronomy.scm

diff --git a/gnu/local.mk b/gnu/local.mk
index d75ab54..26c4151 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -42,6 +42,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/packages/apr.scm				\
   %D%/packages/aspell.scm			\
   %D%/packages/assembly.scm			\
+  %D%/packages/astronomy.scm			\
   %D%/packages/attr.scm				\
   %D%/packages/audacity.scm			\
   %D%/packages/audio.scm			\
diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
new file mode 100644
index 0000000..881e549
--- /dev/null
+++ b/gnu/packages/astronomy.scm
@@ -0,0 +1,53 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 John Darrington <jmd@gnu.org>
+;;;
+;;; 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 astronomy)
+  #:use-module (guix packages)
+  #:use-module (guix licenses)
+  #:use-module (guix download)
+  #:use-module (guix build-system gnu))
+
+(define-public cfitsio
+  (package
+    (name "cfitsio")
+    (version "3390")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append
+        "http://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/" name version
+        ".tar.gz")) 
+       (sha256
+        (base32 "02gllydm63irwbqqisa3mrskw1fphm5rlplglz3mq9whi3rxilv2"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ; no tests
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'replace-slash-bin
+           (lambda _
+             (substitute* "Makefile.in" (("/bin/") "")))))))
+    (home-page "http://heasarc.gsfc.nasa.gov/fitsio/fitsio.html")
+    (synopsis "Library for reading and writing FITS files")
+    (description "CFITSIO provides simple high-level routines for reading and
+writing FITS (Flexible Image Transport System) files that insulate the
+programmer from the internal complexities of the FITS format. CFITSIO also
+provides many advanced features for manipulating and filtering the information
+in FITS files.")
+    (license (license:non-copyleft "file://License.txt"
+                          "See License.txt in the distribution."))))
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] gnu: Add wcslib
  2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
@ 2016-08-30 19:21 ` John Darrington
  2016-08-30 20:47   ` Leo Famulari
  2016-08-31  6:42   ` Alex Kost
  2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: John Darrington @ 2016-08-30 19:21 UTC (permalink / raw)
  To: guix-devel; +Cc: John Darrington

* gnu/packages/astronomy.scm (wcslib): New variable.
---
 gnu/packages/astronomy.scm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 881e549..53e86c8 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -51,3 +51,30 @@ provides many advanced features for manipulating and filtering the information
 in FITS files.")
     (license (license:non-copyleft "file://License.txt"
                           "See License.txt in the distribution."))))
+
+(define-public wcslib
+  (package
+    (name "wcslib")
+    (version "5.15")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append
+             "ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))
+       (sha256
+        (base32 "1s2nig327g4bimd9xshlk11ww09a7mrjmsbpdcd8smsmn2kl1glb"))))
+    (inputs
+     `(("cfitsio" ,cfitsio)))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:phases (modify-phases %standard-phases
+                  (add-before 'configure 'patch-/bin/sh
+                    (lambda _
+                      (substitute* "makedefs.in"
+                        (("/bin/sh") "sh")))))))
+    (home-page "http://www.atnf.csiro.au/people/mcalabre/WCS")
+    (synopsis "An implementation of the FITS WCS standard")
+    (description "The FITS \"World Coordinate System\" (WCS) standard defines
+keywords and usage that provide for the description of astronomical coordinate
+systems in a FITS image header.")  
+    (license license:lgpl3+)))
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] gnu: Add gnuastro
  2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
  2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
@ 2016-08-30 19:21 ` John Darrington
  2016-08-30 20:45   ` Leo Famulari
  2016-08-31  6:47   ` Alex Kost
  2016-08-30 20:47 ` [PATCH 1/3] gnu: Add cfitsio Leo Famulari
  2016-08-31  6:45 ` Alex Kost
  3 siblings, 2 replies; 13+ messages in thread
From: John Darrington @ 2016-08-30 19:21 UTC (permalink / raw)
  To: guix-devel; +Cc: John Darrington

* gnu/packages/astronomy.scm (gnuastro): New variable.
---
 gnu/packages/astronomy.scm | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
index 53e86c8..24c6d79 100644
--- a/gnu/packages/astronomy.scm
+++ b/gnu/packages/astronomy.scm
@@ -18,8 +18,10 @@
 
 (define-module (gnu packages astronomy)
   #:use-module (guix packages)
-  #:use-module (guix licenses)
+  #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix download)
+  #:use-module (gnu packages image)
+  #:use-module (gnu packages maths)
   #:use-module (guix build-system gnu))
 
 (define-public cfitsio
@@ -78,3 +80,27 @@ in FITS files.")
 keywords and usage that provide for the description of astronomical coordinate
 systems in a FITS image header.")  
     (license license:lgpl3+)))
+
+(define-public gnuastro
+  (package
+    (name "gnuastro")
+    (version "0.1")
+    (source
+     (origin
+       (method url-fetch)
+      (uri (string-append "mirror://gnu/gnuastro/gnuastro-"
+                          version ".tar.gz"))
+       (sha256
+        (base32
+         "105s007kw8l3jwwhvh8k9lgbpfbf7sqh2wpxmvpv3qdr6nh9lnjg"))))
+    (inputs
+     `(("cfitsio" ,cfitsio)
+       ("gsl" ,gsl)
+       ("libjpeg" ,libjpeg-8)
+       ("wcslib" ,wcslib)))
+    (build-system gnu-build-system)
+    (home-page "http://www.gnu.org/software/gnuastro")
+    (synopsis "Astronomical data manipulation programs")
+    (description "The GNU Astronomy Utilities (Gnuastro) is a suite of
+programs for the manipulation and analysis of astronomical data.")
+    (license license:gpl3+)))
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] gnu: Add gnuastro
  2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
@ 2016-08-30 20:45   ` Leo Famulari
  2016-08-31  6:47   ` Alex Kost
  1 sibling, 0 replies; 13+ messages in thread
From: Leo Famulari @ 2016-08-30 20:45 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

On Tue, Aug 30, 2016 at 09:21:12PM +0200, John Darrington wrote:
> * gnu/packages/astronomy.scm (gnuastro): New variable.

> +    (source
> +     (origin

I would shift this one column to the right, and the rest of (source)
accordingly.

> +       (method url-fetch)
> +      (uri (string-append "mirror://gnu/gnuastro/gnuastro-"
> +                          version ".tar.gz"))

The (uri) should be aligned with (method url-fetch). Otherwise LGTM.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] gnu: Add cfitsio
  2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
  2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
  2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
@ 2016-08-30 20:47 ` Leo Famulari
  2016-08-31  6:45 ` Alex Kost
  3 siblings, 0 replies; 13+ messages in thread
From: Leo Famulari @ 2016-08-30 20:47 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

On Tue, Aug 30, 2016 at 09:21:10PM +0200, John Darrington wrote:
> +       (uri (string-append
> +        "http://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/" name version
> +        ".tar.gz")) 

I would indent this based on (string-append).

> +         (add-after 'unpack 'replace-slash-bin

I would call this "patch-paths" or something like that, in order to
communicate the intention.

Otherwise LGTM.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] gnu: Add wcslib
  2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
@ 2016-08-30 20:47   ` Leo Famulari
  2016-08-31  6:42   ` Alex Kost
  1 sibling, 0 replies; 13+ messages in thread
From: Leo Famulari @ 2016-08-30 20:47 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

On Tue, Aug 30, 2016 at 09:21:11PM +0200, John Darrington wrote:
> * gnu/packages/astronomy.scm (wcslib): New variable.

LGTM

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] gnu: Add wcslib
  2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
  2016-08-30 20:47   ` Leo Famulari
@ 2016-08-31  6:42   ` Alex Kost
  2016-09-12 13:44     ` Alex Kost
  1 sibling, 1 reply; 13+ messages in thread
From: Alex Kost @ 2016-08-31  6:42 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

John Darrington (2016-08-30 22:21 +0300) wrote:

> * gnu/packages/astronomy.scm (wcslib): New variable.
> ---
>  gnu/packages/astronomy.scm | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
>
> diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
> index 881e549..53e86c8 100644
> --- a/gnu/packages/astronomy.scm
> +++ b/gnu/packages/astronomy.scm
> @@ -51,3 +51,30 @@ provides many advanced features for manipulating and filtering the information
>  in FITS files.")
>      (license (license:non-copyleft "file://License.txt"
>                            "See License.txt in the distribution."))))
> +
> +(define-public wcslib
> +  (package
> +    (name "wcslib")
> +    (version "5.15")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +             "ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))

As for me, this line is too long, I would write:

          (uri (string-append
                "ftp://ftp.atnf.csiro.au/pub/software/wcslib/"
                name "-" version ".tar.bz2"))

> +       (sha256
> +        (base32 "1s2nig327g4bimd9xshlk11ww09a7mrjmsbpdcd8smsmn2kl1glb"))))
> +    (inputs
> +     `(("cfitsio" ,cfitsio)))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:phases (modify-phases %standard-phases
> +                  (add-before 'configure 'patch-/bin/sh
> +                    (lambda _
> +                      (substitute* "makedefs.in"
> +                        (("/bin/sh") "sh")))))))

This phase should end with #t.

> +    (home-page "http://www.atnf.csiro.au/people/mcalabre/WCS")
> +    (synopsis "An implementation of the FITS WCS standard")

I think we don't begin synopses with articles ("guix lint" should report
about it).

> +    (description "The FITS \"World Coordinate System\" (WCS) standard defines
> +keywords and usage that provide for the description of astronomical coordinate
> +systems in a FITS image header.")  
                                    ^^
Please remove trailing spaces

> +    (license license:lgpl3+)))

-- 
Alex

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] gnu: Add cfitsio
  2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
                   ` (2 preceding siblings ...)
  2016-08-30 20:47 ` [PATCH 1/3] gnu: Add cfitsio Leo Famulari
@ 2016-08-31  6:45 ` Alex Kost
  3 siblings, 0 replies; 13+ messages in thread
From: Alex Kost @ 2016-08-31  6:45 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

John Darrington (2016-08-30 22:21 +0300) wrote:

> * gnu/packages/astronomy.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> ---
>  gnu/local.mk               |  1 +
>  gnu/packages/astronomy.scm | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
>  create mode 100644 gnu/packages/astronomy.scm
>
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index d75ab54..26c4151 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -42,6 +42,7 @@ GNU_SYSTEM_MODULES =				\
>    %D%/packages/apr.scm				\
>    %D%/packages/aspell.scm			\
>    %D%/packages/assembly.scm			\
> +  %D%/packages/astronomy.scm			\
>    %D%/packages/attr.scm				\
>    %D%/packages/audacity.scm			\
>    %D%/packages/audio.scm			\
> diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
> new file mode 100644
> index 0000000..881e549
> --- /dev/null
> +++ b/gnu/packages/astronomy.scm
> @@ -0,0 +1,53 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2016 John Darrington <jmd@gnu.org>
> +;;;
> +;;; 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 astronomy)
> +  #:use-module (guix packages)
> +  #:use-module (guix licenses)
> +  #:use-module (guix download)
> +  #:use-module (guix build-system gnu))
> +
> +(define-public cfitsio
> +  (package
> +    (name "cfitsio")
> +    (version "3390")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +        "http://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/" name version
> +        ".tar.gz")) 
                      ^
Please indent the above lines properly and remove the trailing space, I
would write it like this:

          (uri (string-append
                "http://heasarc.gsfc.nasa.gov/FTP/software/fitsio/c/"
                name version ".tar.gz"))


> +       (sha256
> +        (base32 "02gllydm63irwbqqisa3mrskw1fphm5rlplglz3mq9whi3rxilv2"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:tests? #f ; no tests
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'replace-slash-bin
> +           (lambda _
> +             (substitute* "Makefile.in" (("/bin/") "")))))))

Please add #t in the end of this phase.

> +    (home-page "http://heasarc.gsfc.nasa.gov/fitsio/fitsio.html")
> +    (synopsis "Library for reading and writing FITS files")
> +    (description "CFITSIO provides simple high-level routines for reading and
> +writing FITS (Flexible Image Transport System) files that insulate the
> +programmer from the internal complexities of the FITS format. CFITSIO also
> +provides many advanced features for manipulating and filtering the information
> +in FITS files.")
> +    (license (license:non-copyleft "file://License.txt"
> +                          "See License.txt in the distribution."))))
                             ^^^^^^^^^
Indentation here

-- 
Alex

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] gnu: Add gnuastro
  2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
  2016-08-30 20:45   ` Leo Famulari
@ 2016-08-31  6:47   ` Alex Kost
  1 sibling, 0 replies; 13+ messages in thread
From: Alex Kost @ 2016-08-31  6:47 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

John Darrington (2016-08-30 22:21 +0300) wrote:

> * gnu/packages/astronomy.scm (gnuastro): New variable.
> ---
>  gnu/packages/astronomy.scm | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
>
> diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
> index 53e86c8..24c6d79 100644
> --- a/gnu/packages/astronomy.scm
> +++ b/gnu/packages/astronomy.scm
> @@ -18,8 +18,10 @@
>  
>  (define-module (gnu packages astronomy)
>    #:use-module (guix packages)
> -  #:use-module (guix licenses)
> +  #:use-module ((guix licenses) #:prefix license:)
>    #:use-module (guix download)
> +  #:use-module (gnu packages image)
> +  #:use-module (gnu packages maths)
>    #:use-module (guix build-system gnu))
>  
>  (define-public cfitsio
> @@ -78,3 +80,27 @@ in FITS files.")
>  keywords and usage that provide for the description of astronomical coordinate
>  systems in a FITS image header.")  
>      (license license:lgpl3+)))
> +
> +(define-public gnuastro
> +  (package
> +    (name "gnuastro")
> +    (version "0.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +      (uri (string-append "mirror://gnu/gnuastro/gnuastro-"
> +                          version ".tar.gz"))
         ^^
Please shift this (uri ...) one character to the right.

> +       (sha256
> +        (base32
> +         "105s007kw8l3jwwhvh8k9lgbpfbf7sqh2wpxmvpv3qdr6nh9lnjg"))))
> +    (inputs
> +     `(("cfitsio" ,cfitsio)
> +       ("gsl" ,gsl)
> +       ("libjpeg" ,libjpeg-8)
> +       ("wcslib" ,wcslib)))
> +    (build-system gnu-build-system)
> +    (home-page "http://www.gnu.org/software/gnuastro")
> +    (synopsis "Astronomical data manipulation programs")
> +    (description "The GNU Astronomy Utilities (Gnuastro) is a suite of
> +programs for the manipulation and analysis of astronomical data.")
> +    (license license:gpl3+)))

-- 
Alex

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] gnu: Add wcslib
  2016-08-31  6:42   ` Alex Kost
@ 2016-09-12 13:44     ` Alex Kost
  2016-09-12 15:40       ` John Darrington
  0 siblings, 1 reply; 13+ messages in thread
From: Alex Kost @ 2016-09-12 13:44 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel

Alex Kost (2016-08-31 09:42 +0300) wrote:

> John Darrington (2016-08-30 22:21 +0300) wrote:
>
>> * gnu/packages/astronomy.scm (wcslib): New variable.
>> ---
>>  gnu/packages/astronomy.scm | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>>
>> diff --git a/gnu/packages/astronomy.scm b/gnu/packages/astronomy.scm
>> index 881e549..53e86c8 100644
>> --- a/gnu/packages/astronomy.scm
>> +++ b/gnu/packages/astronomy.scm
>> @@ -51,3 +51,30 @@ provides many advanced features for manipulating and filtering the information
>>  in FITS files.")
>>      (license (license:non-copyleft "file://License.txt"
>>                            "See License.txt in the distribution."))))
>> +
>> +(define-public wcslib
>> +  (package
>> +    (name "wcslib")
>> +    (version "5.15")
>> +    (source
>> +     (origin
>> +       (method url-fetch)
>> +       (uri (string-append
>> +             "ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))
>
> As for me, this line is too long, I would write:
>
>           (uri (string-append
>                 "ftp://ftp.atnf.csiro.au/pub/software/wcslib/"
>                 name "-" version ".tar.bz2"))
>
>> +       (sha256
>> +        (base32 "1s2nig327g4bimd9xshlk11ww09a7mrjmsbpdcd8smsmn2kl1glb"))))
>> +    (inputs
>> +     `(("cfitsio" ,cfitsio)))
>> +    (build-system gnu-build-system)
>> +    (arguments
>> +     `(#:phases (modify-phases %standard-phases
>> +                  (add-before 'configure 'patch-/bin/sh
>> +                    (lambda _
>> +                      (substitute* "makedefs.in"
>> +                        (("/bin/sh") "sh")))))))
>
> This phase should end with #t.

I've noticed that you didn't fix these things (long line and #t after
substitute*).  Could please do it next time :-)

The same for 'cfitsio' package.

-- 
Alex

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] gnu: Add wcslib
  2016-09-12 13:44     ` Alex Kost
@ 2016-09-12 15:40       ` John Darrington
  2016-09-13  8:10         ` Alex Kost
  0 siblings, 1 reply; 13+ messages in thread
From: John Darrington @ 2016-09-12 15:40 UTC (permalink / raw)
  To: Alex Kost; +Cc: guix-devel, John Darrington

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

On Mon, Sep 12, 2016 at 04:44:44PM +0300, Alex Kost wrote:

     I've noticed that you didn't fix these things (long line and #t after
     substitute*).  Could please do it next time :-)
     
     The same for 'cfitsio' package.
     
Done.

If this is important why doesn't guix build and/or guix lint check for it?



-- 
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.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] gnu: Add wcslib
  2016-09-12 15:40       ` John Darrington
@ 2016-09-13  8:10         ` Alex Kost
  2016-09-13 12:44           ` John Darrington
  0 siblings, 1 reply; 13+ messages in thread
From: Alex Kost @ 2016-09-13  8:10 UTC (permalink / raw)
  To: John Darrington; +Cc: guix-devel, John Darrington

John Darrington (2016-09-12 17:40 +0200) wrote:

> On Mon, Sep 12, 2016 at 04:44:44PM +0300, Alex Kost wrote:
>
>      I've noticed that you didn't fix these things (long line and #t after
>      substitute*).  Could please do it next time :-)
>
>      The same for 'cfitsio' package.
>
> Done.
>
> If this is important why doesn't guix build and/or guix lint check for it?

"guix lint" can't check if a phase should end with #t or not, it's up to
you check if it is needed.  The thing is: if a phase fails, it should
return false value, and if it succeeds, it should return non-false
value.  A returned value of 'substitute*' is unspecified, so here you
should add #t to the end of the phase.  It works without it, but I would
say it happens "by chance" because #<unspecified> is considered to be
non-false, but we shouldn't rely on it, so adding #t to such phases is
the right thing.

As for the long line, it is 89 chars long, while "guix lint" reports
about exceeding 90 chars (see 'report-long-line' in (guix scripts lint)
module).  BTW I think this is too loose, I would limit it to 80.

But your line could be easily shorten, as I wrote at
<http://lists.gnu.org/archive/html/guix-devel/2016-08/msg02070.html>,
so instead of the current long line:

       (uri (string-append
             "ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))

it could be:

       (uri (string-append
             "ftp://ftp.atnf.csiro.au/pub/software/wcslib/"
             name "-" version ".tar.bz2"))

which fits any screen and thus is more readable I think.

I just felt a bit of a letdown that you ignored my comments and
didn't send a message why.

-- 
Alex

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] gnu: Add wcslib
  2016-09-13  8:10         ` Alex Kost
@ 2016-09-13 12:44           ` John Darrington
  0 siblings, 0 replies; 13+ messages in thread
From: John Darrington @ 2016-09-13 12:44 UTC (permalink / raw)
  To: Alex Kost; +Cc: guix-devel

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

On Tue, Sep 13, 2016 at 11:10:57AM +0300, Alex Kost wrote:
     John Darrington (2016-09-12 17:40 +0200) wrote:
     
     > On Mon, Sep 12, 2016 at 04:44:44PM +0300, Alex Kost wrote:
     >
     >      I've noticed that you didn't fix these things (long line and #t after
     >      substitute*).  Could please do it next time :-)
     >
     >      The same for 'cfitsio' package.
     >
     > Done.
     >
     > If this is important why doesn't guix build and/or guix lint check for it?
     
     "guix lint" can't check if a phase should end with #t or not, it's up to
     you check if it is needed.  The thing is: if a phase fails, it should
     return false value, and if it succeeds, it should return non-false
     value.  A returned value of 'substitute*' is unspecified, so here you
     should add #t to the end of the phase.  It works without it, but I would
     say it happens "by chance" because #<unspecified> is considered to be
     non-false, but we shouldn't rely on it, so adding #t to such phases is
     the right thing.


1.  Presumably guix build somewhere calls something like:

   (if (run-phase x) (run-next-phase) (error))

We could change this to:

   (if (let ((result (run-phase x)))
          (if (unspecfied? result) (error "Result of phase is unspecified"))
           result)
        (run-next-phase)
	(error))

.... or something similar ...


2. It wouldn't be a bad idea to change subsitute* to return something.  For
   example, the number of substitutions performed.
     


     As for the long line, it is 89 chars long, while "guix lint" reports
     about exceeding 90 chars (see 'report-long-line' in (guix scripts lint)
     module).  BTW I think this is too loose, I would limit it to 80.
     
     But your line could be easily shorten, as I wrote at
     <http://lists.gnu.org/archive/html/guix-devel/2016-08/msg02070.html>,
     so instead of the current long line:
     
            (uri (string-append
                  "ftp://ftp.atnf.csiro.au/pub/software/wcslib/" name "-" version ".tar.bz2"))
     
     it could be:
     
            (uri (string-append
                  "ftp://ftp.atnf.csiro.au/pub/software/wcslib/"
                  name "-" version ".tar.bz2"))
     
     which fits any screen and thus is more readable I think.
     

I think all the lines are less than 80 right now aren't they?


     I just felt a bit of a letdown that you ignored my comments and
     didn't send a message why.
     

I didn't mean to shun you.   I appreciate the time and effort you took to 
review my code.

Unfortunately I find the Guix workflow awkward to manage so sometimes I
have omitted corrections which ought to have been made.   Part of my
problem is that currently (due to lack of many services) I cannot send or
receive mail from the machine on which I have GuixSD running.  Hopefully
this will change soon.

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.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-09-13 12:44 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-30 19:21 [PATCH 1/3] gnu: Add cfitsio John Darrington
2016-08-30 19:21 ` [PATCH 2/3] gnu: Add wcslib John Darrington
2016-08-30 20:47   ` Leo Famulari
2016-08-31  6:42   ` Alex Kost
2016-09-12 13:44     ` Alex Kost
2016-09-12 15:40       ` John Darrington
2016-09-13  8:10         ` Alex Kost
2016-09-13 12:44           ` John Darrington
2016-08-30 19:21 ` [PATCH 3/3] gnu: Add gnuastro John Darrington
2016-08-30 20:45   ` Leo Famulari
2016-08-31  6:47   ` Alex Kost
2016-08-30 20:47 ` [PATCH 1/3] gnu: Add cfitsio Leo Famulari
2016-08-31  6:45 ` Alex Kost

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).