diff mbox

gnu: Add hdf-eos5.

Message ID 20160927.222845.1637168386129752210.thomas.danckaert@gmail.com
State New
Headers show

Commit Message

Thomas Danckaert Sept. 27, 2016, 8:28 p.m. UTC
None

Comments

Ludovic Courtès Oct. 3, 2016, 3:59 p.m. UTC | #1
Hi Thomas,

Thomas Danckaert <thomas.danckaert@gmail.com> skribis:

> From f1a3bc9dcfbe9091110aacf353d8224a88788292 Mon Sep 17 00:00:00 2001
> From: Thomas Danckaert <thomas.danckaert@gmail.com>
> Date: Fri, 17 Jun 2016 10:51:38 +0200
> Subject: [PATCH] gnu: Add hdf-eos5.
>
> * gnu/packages/maths.scm (hdf-eos5): New variable.
> * gnu/packages/patches/hdf-eos5-build-shared.patch: New file.
> * gnu/packages/patches/hdf-eos5-remove-gctp.patch: New file.
> * gnu/packages/patches/hdf-eos5-fix-szip.patch: New file.

Thanks for this patch!  Overall it looks pretty good to me; some
comments/suggestions follow.

First, please make sure to list the patches in gnu/local.mk.

Also, for each patch, could you add a word stating what the upstream
status is, such as the URL of the upstream commit or discussion, when it
exists?

> --- a/gnu/packages/maths.scm
> +++ b/gnu/packages/maths.scm

Please add a copyright line for you.

> +    (native-inputs
> +     `(("autoconf" ,autoconf)
> +       ("automake" ,automake)
> +       ("libtool" ,libtool)))
> +    (build-system gnu-build-system)
> +    (inputs
> +     `(("hdf5" ,hdf5)
> +       ("zlib" ,zlib)
> +       ("gctp" ,gctp)))
> +    (arguments
> +     `(#:configure-flags '("--enable-install-include")
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before
> +             'configure 'autogen
> +           (lambda _  ; configure.ac was patched
> +             (and (chmod "./configure" #o755)
> +                  (zero? (system* "autoreconf" "--install"))))))

I wonder if we could avoid running autoreconf, which would mean patching
configure/Makefile.in instead of configure.ac/Makefile.am.

> +    (synopsis
> +     "HDF-EOS5: HDF5-based data format for NASA's Earth Observing System")

Remove “HDF-EOS5:” here.

> +++ b/gnu/packages/patches/hdf-eos5-build-shared.patch
> @@ -0,0 +1,88 @@
> +Allow shared build.
> +
> +We remove all references to the 'testdrivers' directory.  This directory is
> +not included in the distributed source archive, and its absence trips up
> +automake.
> +
> +---
> + Makefile.am             |  6 ------
> + configure.ac            | 12 ------------
> + include/HE5_HdfEosDef.h |  1 +
> + src/Makefile.am         |  3 ++-
> + 4 files changed, 3 insertions(+), 19 deletions(-)
> +
> +diff --git a/Makefile.am b/Makefile.am
> +index 363bcfb..01ed024 100644
> +--- a/Makefile.am
> ++++ b/Makefile.am
> +@@ -3,13 +3,7 @@
> + # Include boilerplate
> + include $(top_srcdir)/config/include.am
> + 
> +-# List of subdirectories.
> +-# Only build the testdrivers directory if configure detected that it's present.
> +-if TESTDRIVERS_CONDITIONAL
> +-TESTDRIVERS=testdrivers
> +-else
> + TESTDRIVERS=
> +-endif

What about patching ‘configure’ such that the shell variable
‘TESTDRIVERS_DIR’ is set to “no”, and such that the
‘TESTDRIVERS_CONDITIONAL’ Automake condition is false?

That should be doable and would avoid this relatively length patch.

> +-# Disable shared libraries (for now)
> +-AC_DISABLE_SHARED

This macro only changes the default behavior.  Passing “--enable-shared”
to #:configure-flags should be enough to build shared libraries.

Could you check if that works?

> --- /dev/null
> +++ b/gnu/packages/patches/hdf-eos5-headers.patch
> @@ -0,0 +1,14 @@
> +Do not install unnecessary headers.
> +
> +diff --git a/include/Makefile.am b/include/Makefile.am
> +index 3de93db..52246af 100755
> +--- a/include/Makefile.am
> ++++ b/include/Makefile.am
> +@@ -4,6 +4,5 @@
> + include $(top_srcdir)/config/include.am
> + 
> + # Headers to install
> +-include_HEADERS = HE5_GctpFunc.h HE5_HdfEosDef.h HE5_config.h cproj.h ease.h \
> +-                  isin.h proj.h tutils.h cfortHdf.h
> ++include_HEADERS = HE5_GctpFunc.h HE5_HdfEosDef.h ease.h isin.h cfortHdf.h

What about removing the extra files in a separate phase?  That would
avoid the need to modify Makefile.am.

Also please add a comment on why you think it’s unnecessary.  :-)

> diff --git a/gnu/packages/patches/hdf-eos5-remove-gctp.patch b/gnu/packages/patches/hdf-eos5-remove-gctp.patch
> new file mode 100644
> index 0000000..359d486
> --- /dev/null
> +++ b/gnu/packages/patches/hdf-eos5-remove-gctp.patch
> @@ -0,0 +1,61 @@
> +Do not build/install the bundled GCTP and link shared -lgctp.
> +
> +---
> + Makefile.am         | 2 +-
> + config/include.am   | 1 -
> + configure.ac        | 3 ---
> + include/Makefile.am | 3 +--
> + src/Makefile.am     | 2 +-
> + 5 files changed, 3 insertions(+), 8 deletions(-)
> +
> +diff --git a/Makefile.am b/Makefile.am
> +index 01ed024..ef325ac 100644
> +--- a/Makefile.am
> ++++ b/Makefile.am
> +@@ -11,5 +11,5 @@ else
> + INCLUDE=
> + endif
> + 
> +-SUBDIRS=gctp src $(INCLUDE) samples $(TESTDRIVERS)
> ++SUBDIRS=src $(INCLUDE) samples $(TESTDRIVERS)

I think this can be achieved by removing it from Makefile.in with
‘substitute*’.  The LDFLAGS below can be passed like this:

  #:configure-flags (list … "LDFLAGS=-Wl,--as-needed -lgctp")

Could you send an updated patch?

Thank you!

Ludo’.
diff mbox

Patch

From f1a3bc9dcfbe9091110aacf353d8224a88788292 Mon Sep 17 00:00:00 2001
From: Thomas Danckaert <thomas.danckaert@gmail.com>
Date: Fri, 17 Jun 2016 10:51:38 +0200
Subject: [PATCH] gnu: Add hdf-eos5.

* gnu/packages/maths.scm (hdf-eos5): New variable.
* gnu/packages/patches/hdf-eos5-build-shared.patch: New file.
* gnu/packages/patches/hdf-eos5-remove-gctp.patch: New file.
* gnu/packages/patches/hdf-eos5-fix-szip.patch: New file.
---
 gnu/packages/maths.scm                           | 45 ++++++++++++
 gnu/packages/patches/hdf-eos5-build-shared.patch | 88 ++++++++++++++++++++++++
 gnu/packages/patches/hdf-eos5-fix-szip.patch     | 32 +++++++++
 gnu/packages/patches/hdf-eos5-headers.patch      | 14 ++++
 gnu/packages/patches/hdf-eos5-remove-gctp.patch  | 61 ++++++++++++++++
 5 files changed, 240 insertions(+)
 create mode 100644 gnu/packages/patches/hdf-eos5-build-shared.patch
 create mode 100644 gnu/packages/patches/hdf-eos5-fix-szip.patch
 create mode 100644 gnu/packages/patches/hdf-eos5-headers.patch
 create mode 100644 gnu/packages/patches/hdf-eos5-remove-gctp.patch

diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm
index 0401cd3..1cc041d 100644
--- a/gnu/packages/maths.scm
+++ b/gnu/packages/maths.scm
@@ -42,6 +42,7 @@ 
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system r)
   #:use-module (gnu packages algebra)
+  #:use-module (gnu packages autotools)
   #:use-module (gnu packages bison)
   #:use-module (gnu packages boost)
   #:use-module (gnu packages check)
@@ -483,6 +484,50 @@  extremely large and complex data collections.")
     (license (license:x11-style
               "http://www.hdfgroup.org/ftp/HDF5/current/src/unpacked/COPYING"))))
 
+(define-public hdf-eos5
+  (package
+    (name "hdf-eos5")
+    (version "1.15")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "ftp://edhs1.gsfc.nasa.gov\
+/edhs/hdfeos5/latest_release/HDF-EOS5." version ".tar.Z"))
+              (sha256
+               (base32
+                "1p83333nzzy8rn5chxlm0hrkjjnhh2w1ji8ac0f9q4xzg838i58i"))
+              (patches (search-patches "hdf-eos5-build-shared.patch"
+                                       "hdf-eos5-remove-gctp.patch"
+                                       "hdf-eos5-fix-szip.patch"
+                                       "hdf-eos5-headers.patch"))))
+    (native-inputs
+     `(("autoconf" ,autoconf)
+       ("automake" ,automake)
+       ("libtool" ,libtool)))
+    (build-system gnu-build-system)
+    (inputs
+     `(("hdf5" ,hdf5)
+       ("zlib" ,zlib)
+       ("gctp" ,gctp)))
+    (arguments
+     `(#:configure-flags '("--enable-install-include")
+       #:phases
+       (modify-phases %standard-phases
+         (add-before
+             'configure 'autogen
+           (lambda _  ; configure.ac was patched
+             (and (chmod "./configure" #o755)
+                  (zero? (system* "autoreconf" "--install"))))))
+       #:parallel-tests? #f))
+    (synopsis
+     "HDF-EOS5: HDF5-based data format for NASA's Earth Observing System")
+    (description
+     "HDF-EOS5 is a software library built on HDF5 to support the construction
+of data structures used in NASA's Earth Observing System (Grid, Point and
+Swath).")
+    (home-page "http://www.hdfeos.org/software/library.php#HDF-EOS5")
+    (license (license:bsd-style
+              "http://www.hdfeos.org/software/library.php#HDF-EOS5"))))
+
 (define-public hdf5-parallel-openmpi
   (package (inherit hdf5)
     (name "hdf5-parallel-openmpi")
diff --git a/gnu/packages/patches/hdf-eos5-build-shared.patch b/gnu/packages/patches/hdf-eos5-build-shared.patch
new file mode 100644
index 0000000..d1b4806
--- /dev/null
+++ b/gnu/packages/patches/hdf-eos5-build-shared.patch
@@ -0,0 +1,88 @@ 
+Allow shared build.
+
+We remove all references to the 'testdrivers' directory.  This directory is
+not included in the distributed source archive, and its absence trips up
+automake.
+
+---
+ Makefile.am             |  6 ------
+ configure.ac            | 12 ------------
+ include/HE5_HdfEosDef.h |  1 +
+ src/Makefile.am         |  3 ++-
+ 4 files changed, 3 insertions(+), 19 deletions(-)
+
+diff --git a/Makefile.am b/Makefile.am
+index 363bcfb..01ed024 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -3,13 +3,7 @@
+ # Include boilerplate
+ include $(top_srcdir)/config/include.am
+ 
+-# List of subdirectories.
+-# Only build the testdrivers directory if configure detected that it's present.
+-if TESTDRIVERS_CONDITIONAL
+-TESTDRIVERS=testdrivers
+-else
+ TESTDRIVERS=
+-endif
+ 
+ if INSTALL_INCLUDE_CONDITIONAL
+ INCLUDE=include
+diff --git a/configure.ac b/configure.ac
+index 5d48b43..cfa9d4e 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -13,9 +13,6 @@ AC_CONFIG_AUX_DIR([config])
+ AM_INIT_AUTOMAKE([foreign])
+ AM_MAINTAINER_MODE
+ 
+-# Disable shared libraries (for now)
+-AC_DISABLE_SHARED
+-
+ AM_PROG_LIBTOOL
+ 
+ dnl ----------------------------------------------------------------------
+@@ -597,13 +594,4 @@ AC_CONFIG_FILES([Makefile
+                  gctp/src/Makefile
+                  samples/Makefile])
+ 
+-if test "X$TESTDRIVERS_DIR" = "Xyes"; then
+-  AC_CONFIG_FILES([ testdrivers/Makefile
+-                 testdrivers/grid/Makefile
+-                 testdrivers/point/Makefile
+-                 testdrivers/swath/Makefile
+-                 testdrivers/threads/Makefile
+-                 testdrivers/za/Makefile])
+-fi
+-
+ AC_OUTPUT
+diff --git a/include/HE5_HdfEosDef.h b/include/HE5_HdfEosDef.h
+index 9ed7881..abf0a90 100755
+--- a/include/HE5_HdfEosDef.h
++++ b/include/HE5_HdfEosDef.h
+@@ -24,6 +24,7 @@
+ #ifndef HE5_HDFEOSDEF_H_
+ #define HE5_HDFEOSDEF_H_
+ 
++#define H5_USE_16_API 1 // required for compilation with recent HDF5
+ #include <hdf5.h>
+ 
+ #ifdef H5_USE_16_API
+diff --git a/src/Makefile.am b/src/Makefile.am
+index 76b1d4c..daf7ad8 100644
+--- a/src/Makefile.am
++++ b/src/Makefile.am
+@@ -10,7 +10,8 @@ INCLUDES=-I$(top_srcdir)/include/
+ 
+ # Set LDFLAGS to allow the HDF-EOS library to use extern variables from
+ # HDF5
+-LDFLAGS=-Wl,-single_module
++LDFLAGS+= -shrext .so
++LIBS+= -lhdf5_hl -lhdf5 -lm
+ 
+ # Build HDF-EOS5
+ lib_LTLIBRARIES=libhe5_hdfeos.la
+-- 
+2.5.0
+
diff --git a/gnu/packages/patches/hdf-eos5-fix-szip.patch b/gnu/packages/patches/hdf-eos5-fix-szip.patch
new file mode 100644
index 0000000..f66ad51
--- /dev/null
+++ b/gnu/packages/patches/hdf-eos5-fix-szip.patch
@@ -0,0 +1,32 @@ 
+Fix compilation without szip.
+
+An ill-placed #endif causes missing symbols when the library is compiled
+without szip.
+
+---
+ src/EHapi.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/EHapi.c b/src/EHapi.c
+index 46a9b5c..504d8f2 100755
+--- a/src/EHapi.c
++++ b/src/EHapi.c
+@@ -11379,6 +11379,7 @@ int HE5_szip_can_encode(void )
+   return(-1);
+ }
+ 
++#endif /* H5_HAVE_FILTER_SZIP */
+ 
+ 
+ /*----------------------------------------------------------------------------|
+@@ -11509,7 +11510,6 @@ HE5_EHHEisHE5(char *filename)
+     }
+ }
+ 
+-#endif /* H5_HAVE_FILTER_SZIP */
+ 
+ 
+ #ifndef __cplusplus
+-- 
+2.5.0
+
diff --git a/gnu/packages/patches/hdf-eos5-headers.patch b/gnu/packages/patches/hdf-eos5-headers.patch
new file mode 100644
index 0000000..84fd90c
--- /dev/null
+++ b/gnu/packages/patches/hdf-eos5-headers.patch
@@ -0,0 +1,14 @@ 
+Do not install unnecessary headers.
+
+diff --git a/include/Makefile.am b/include/Makefile.am
+index 3de93db..52246af 100755
+--- a/include/Makefile.am
++++ b/include/Makefile.am
+@@ -4,6 +4,5 @@
+ include $(top_srcdir)/config/include.am
+ 
+ # Headers to install
+-include_HEADERS = HE5_GctpFunc.h HE5_HdfEosDef.h HE5_config.h cproj.h ease.h \
+-                  isin.h proj.h tutils.h cfortHdf.h
++include_HEADERS = HE5_GctpFunc.h HE5_HdfEosDef.h ease.h isin.h cfortHdf.h
+ 
diff --git a/gnu/packages/patches/hdf-eos5-remove-gctp.patch b/gnu/packages/patches/hdf-eos5-remove-gctp.patch
new file mode 100644
index 0000000..359d486
--- /dev/null
+++ b/gnu/packages/patches/hdf-eos5-remove-gctp.patch
@@ -0,0 +1,61 @@ 
+Do not build/install the bundled GCTP and link shared -lgctp.
+
+---
+ Makefile.am         | 2 +-
+ config/include.am   | 1 -
+ configure.ac        | 3 ---
+ include/Makefile.am | 3 +--
+ src/Makefile.am     | 2 +-
+ 5 files changed, 3 insertions(+), 8 deletions(-)
+
+diff --git a/Makefile.am b/Makefile.am
+index 01ed024..ef325ac 100644
+--- a/Makefile.am
++++ b/Makefile.am
+@@ -11,5 +11,5 @@ else
+ INCLUDE=
+ endif
+ 
+-SUBDIRS=gctp src $(INCLUDE) samples $(TESTDRIVERS)
++SUBDIRS=src $(INCLUDE) samples $(TESTDRIVERS)
+ 
+diff --git a/config/include.am b/config/include.am
+index b82fae9..a1e0332 100644
+--- a/config/include.am
++++ b/config/include.am
+@@ -4,5 +4,4 @@
+ ## shared definitions.
+ 
+ LIBHDFEOS5=$(top_builddir)/src/libhe5_hdfeos.la
+-LIBGCTP=$(top_builddir)/gctp/src/libGctp.la
+ 
+diff --git a/configure.ac b/configure.ac
+index cfa9d4e..860da1c 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -589,9 +589,6 @@ AM_CONDITIONAL([SZIP_ENCODER_CONDITIONAL], [test "X$USE_SZIP_ENCODER" = "Xyes"])
+ AC_CONFIG_FILES([Makefile
+                  include/Makefile
+                  src/Makefile
+-                 gctp/Makefile
+-                 gctp/include/Makefile
+-                 gctp/src/Makefile
+                  samples/Makefile])
+ 
+ AC_OUTPUT
+diff --git a/src/Makefile.am b/src/Makefile.am
+index daf7ad8..f4828cb 100644
+--- a/src/Makefile.am
++++ b/src/Makefile.am
+@@ -11,7 +11,7 @@ INCLUDES=-I$(top_srcdir)/include/
+ # Set LDFLAGS to allow the HDF-EOS library to use extern variables from
+ # HDF5
+ LDFLAGS+= -shrext .so
+-LIBS+= -lhdf5_hl -lhdf5 -lm
++LIBS+= -lgctp -lhdf5_hl -lhdf5 -lm
+ 
+ # Build HDF-EOS5
+ lib_LTLIBRARIES=libhe5_hdfeos.la
+-- 
+2.5.0
+
-- 
2.10.0