all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [PATCH] Add fyba to GNU Guix
@ 2016-03-20  2:19 Dennis Mungai
  2016-03-21 10:53 ` Ricardo Wurmus
  0 siblings, 1 reply; 2+ messages in thread
From: Dennis Mungai @ 2016-03-20  2:19 UTC (permalink / raw)
  To: guix-devel

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

Hello,

This patch adds Fyba, a library required by GDAL to process Norwegian
statistical datums, to GNU Guix.

From the package description:

FYBA is the source code release of the FYBA library, distributed by
the National Mapping Authority of Norway (Kartverket) to read and
write files in the National geodata standard format SOSI.

Thanks and regards,

Dennis.

[-- Attachment #2: 0001-Ported-the-fyba-package-to-GNU-Guix.patch --]
[-- Type: text/x-patch, Size: 2958 bytes --]

From 5212e686078f73211182a4fe52d81529faccefba Mon Sep 17 00:00:00 2001
From: Dennis Mungai <dmngaie@gmail.com>
Date: Sun, 20 Mar 2016 04:48:27 +0300
Subject: [PATCH] Ported the fyba package to GNU Guix

---
 fyba.scm | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 fyba.scm

diff --git a/fyba.scm b/fyba.scm
new file mode 100644
index 0000000..a667a73
--- /dev/null
+++ b/fyba.scm
@@ -0,0 +1,60 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 Dennis Mungai <dmngaie@gmail.com>
+;;;
+;;; 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 (gn packages fyba)
+  #:use-module ((guix licenses))
+  #:use-module (gnu packages)
+  #:use-module (gnu packages autotools)
+  #:use-module (guix packages)
+  #:use-module (guix download)
+  #:use-module (guix build-system gnu)
+  #:use-module (gnu packages linux)
+  #:use-module (gnu packages textutils)
+  #:use-module (gnu packages base)
+  #:use-module (gnu packages zip)
+  #:use-module (gnu packages gnupg)
+  #:use-module (gnu packages bootstrap)
+  #:use-module (guix git-download))
+
+(define-public fyba
+  (package
+   (name "fyba")
+   (version "4.1.1")
+   (source (origin
+             (method url-fetch)
+             (uri (string-append "https://github.com/kartverket/fyba/archive/"
+                                 version ".tar.gz"))
+             (file-name (string-append name "-" version ".tar.gz"))
+             (sha256
+              (base32
+               "0ya1agi78d386skq353dk400fl11q6whfqmv31qrkn4g5vamixlr"))))
+    (inputs `(("zip" ,zip)
+             ("autoconf" ,autoconf)
+             ("automake" ,automake)
+             ("libtool" ,libtool)
+             ("libgcrypt" ,libgcrypt)))                                              
+    (build-system gnu-build-system)
+     (arguments
+     '(#:phases (modify-phases %standard-phases
+                    (add-after 'unpack `bootstrap
+                      (lambda _
+                        (zero? (system* "autoreconf" "-vfi")))))))    
+    (home-page "http://labs.kartverket.no/sos/")
+    (synopsis "source code release of the FYBA library")
+    (description "OpenFYBA is the source code release of the FYBA library.")
+    (license (list gpl2))))
-- 
1.9.1


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

* Re: [PATCH] Add fyba to GNU Guix
  2016-03-20  2:19 [PATCH] Add fyba to GNU Guix Dennis Mungai
@ 2016-03-21 10:53 ` Ricardo Wurmus
  0 siblings, 0 replies; 2+ messages in thread
From: Ricardo Wurmus @ 2016-03-21 10:53 UTC (permalink / raw)
  To: Dennis Mungai; +Cc: guix-devel


Hi Dennis,

> This patch adds Fyba, a library required by GDAL to process Norwegian
> statistical datums, to GNU Guix.

Thanks for your patch!

> From the package description:
>
> FYBA is the source code release of the FYBA library, distributed by
> the National Mapping Authority of Norway (Kartverket) to read and
> write files in the National geodata standard format SOSI.

This is a much better description than the one you chose to use for the
package below.


> From 5212e686078f73211182a4fe52d81529faccefba Mon Sep 17 00:00:00 2001
> From: Dennis Mungai <dmngaie@gmail.com>
> Date: Sun, 20 Mar 2016 04:48:27 +0300
> Subject: [PATCH] Ported the fyba package to GNU Guix

Please see my comments on commit messages earlier.

> +(define-module (gn packages fyba)

Is it really necessary to create a new module?  Could this package be
added to an existing module instead?  Also, our modules are called “gnu
packages ...” not “gn packages ...”.

> +(define-public fyba
> +  (package
> +   (name "fyba")
> +   (version "4.1.1")
> +   (source (origin
> +             (method url-fetch)
> +             (uri (string-append "https://github.com/kartverket/fyba/archive/"
> +                                 version ".tar.gz"))
> +             (file-name (string-append name "-" version ".tar.gz"))
> +             (sha256
> +              (base32
> +               "0ya1agi78d386skq353dk400fl11q6whfqmv31qrkn4g5vamixlr"))))

OK.  Have you checked that there is no other release tarball?

> +    (inputs `(("zip" ,zip)
> +             ("autoconf" ,autoconf)
> +             ("automake" ,automake)
> +             ("libtool" ,libtool)
> +             ("libgcrypt" ,libgcrypt)))                                              

Most packages place the “inputs” field after “arguments”.  I think its
better to do the same.

Most of these, however, look like they should be “native-inputs”.

Are the autotools really required?  Or is there a bootstrapped release
tarball somewhere that we could use instead?

> +    (build-system gnu-build-system)
> +     (arguments
> +     '(#:phases (modify-phases %standard-phases
> +                    (add-after 'unpack `bootstrap
> +                      (lambda _
> +                        (zero? (system* "autoreconf" "-vfi")))))))    
> +    (home-page "http://labs.kartverket.no/sos/")
> +    (synopsis "source code release of the FYBA library")

This is not a helpful synopsis.  It should tell me succintly what this
library does, not that this is a release.

> +    (description "OpenFYBA is the source code release of the FYBA library.")

Is it OpenFYBA or just FYBA?  What is the FYBA library?  Your
explanation in the email is a lot better than this.

> +    (license (list gpl2))))

No “list” required when you only have one license.  Is this really just
“GPLv2 only” or “GPLv2 or later”?

Could you please resend this patch after addressing these comments and
running “guix lint fyba”?

Thanks again!

~~ Ricardo

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

end of thread, other threads:[~2016-03-21 10:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-20  2:19 [PATCH] Add fyba to GNU Guix Dennis Mungai
2016-03-21 10:53 ` Ricardo Wurmus

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.