From mboxrd@z Thu Jan 1 00:00:00 1970 From: Leo Famulari Subject: Re: [PATCH] Add gctp Date: Fri, 17 Jun 2016 21:39:26 -0400 Message-ID: <20160618013926.GA1823@jasmine> References: <20160617.144017.336729442489418576.post@thomasdanckaert.be> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:53598) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bE5Ee-0000qm-0g for guix-devel@gnu.org; Fri, 17 Jun 2016 21:39:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bE5EZ-0002GI-RS for guix-devel@gnu.org; Fri, 17 Jun 2016 21:39:43 -0400 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:37627) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bE5EX-0002Fl-HI for guix-devel@gnu.org; Fri, 17 Jun 2016 21:39:39 -0400 Content-Disposition: inline In-Reply-To: <20160617.144017.336729442489418576.post@thomasdanckaert.be> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Thomas Danckaert Cc: guix-devel@gnu.org On Fri, Jun 17, 2016 at 02:40:17PM +0200, Thomas Danckaert wrote: > this is a patch to add the GCTP library. It seems the library is no longer > maintained separately (“official” sources are gone), so this package > downloads the HDF-EOS5 source, which contains GCTP, and patches the build to > only build and install GCTP. (I'm packaging HDF-EOS5 later on) Should we package GCTP separately in that case? Is it used by anything besides HDF-EOS5? Or, should we just package HDF-EOS5? We usually don't accept bundled code, but it sounds like GCTP no longer exists as an independent project. Is that right? Otherwise, it looks like we are forking GCTP and creating a new distribution. > It's not very elegant... An alternative solution would be to get the source > package from, e.g. Debian, and build that (might involve less patching). We do fetch a few things from Debian. Here are some comments: > Subject: [PATCH] gnu: Add gctp We prefer to use complete sentences, with punctuation, even when they are only 2 words long ;) > * gnu/packages/maths.cm (gctp): New variable > > * guix/packages.scm (decompression-type): Modify by adding ".Z" suffix > for gzipped files. I believe this was addressed in commit 5257ab6de. Can you check? > * gnu/packages/patches/gct-build-shared.patch: patch build system for > shared builds. We also capitalize the beginning of our sentences :) > * gnu/packages/patches/gctp-remove-hdf-eos5.patch: don't build the > hdf-eos5 library contained in the same archive. > > * gnu/packages/patches/gctp-fix-soname.patch: use a lower case soname. > --- > gnu/packages/maths.scm | 39 ++ > gnu/packages/patches/gctp-build-shared.patch | 84 +++++ > gnu/packages/patches/gctp-fix-soname.patch | 31 ++ > gnu/packages/patches/gctp-remove-hdf-eos5.patch | 482 ++++++++++++++++++++++++ > 4 files changed, 636 insertions(+) > create mode 100644 gnu/packages/patches/gctp-build-shared.patch > create mode 100644 gnu/packages/patches/gctp-fix-soname.patch > create mode 100644 gnu/packages/patches/gctp-remove-hdf-eos5.patch > > diff --git a/gnu/packages/maths.scm b/gnu/packages/maths.scm > index 3b860a9..9cd0554 100644 > --- a/gnu/packages/maths.scm > +++ b/gnu/packages/maths.scm > @@ -42,6 +42,8 @@ > #: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 base) > #:use-module (gnu packages bison) > #:use-module (gnu packages boost) > #:use-module (gnu packages check) > @@ -408,6 +410,43 @@ plotting engine by third-party applications like Octave.") > (license (license:fsf-free > "http://gnuplot.cvs.sourceforge.net/gnuplot/gnuplot/Copyright")))) > > +(define-public gctp > + (package > + (name "gctp") > + (version "1.0") Is this the upstream version or is it arbitrary? I see that the HDF-EOS5 tarball is at version 1.15. > + (source (origin > + (method url-fetch) > + (uri (string-append "ftp://edhs1.gsfc.nasa.gov/edhs/hdfeos5/latest_release/HDF-EOS5.1.15.tar.Z")) > + (sha256 > + (base32 > + "1p83333nzzy8rn5chxlm0hrkjjnhh2w1ji8ac0f9q4xzg838i58i")) > + (patches (search-patches "gctp-build-shared.patch" "gctp-remove-hdf-eos5.patch" "gctp-fix-soname.patch")) > + )) Some of these lines are too long. We like to keep them shorter than 80 characters whenever possible. > + (native-inputs > + `(("autoconf" ,autoconf) > + ("automake" ,automake) > + ("libtool" ,libtool) > + ("coreutils" ,coreutils))) > + (build-system gnu-build-system) > + (arguments > + `(#:configure-flags '("--enable-install-include") > + #:phases > + (modify-phases %standard-phases > + (add-before > + 'configure 'autogen > + (lambda _ > + (and (zero? (system* "chmod" "-R" "u+w" ".")) ; archive is read-only... We have a Scheme procedure for chmod. There are examples in 'gnu/packages'. Is this what required coreutils as a native-input? > diff --git a/gnu/packages/patches/gctp-build-shared.patch b/gnu/packages/patches/gctp-build-shared.patch In general, I think these patches need some more comments and explanation of the various changes. > new file mode 100644 > index 0000000..1139fbb > --- /dev/null > +++ b/gnu/packages/patches/gctp-build-shared.patch > @@ -0,0 +1,84 @@ > +Allow shared build. > + > +--- > + 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 is the effect of this? > + > + 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 Okay. > +- > + 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 > +- It's related to an earlier diff, but can you explain? > + 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 What is the significance of this definition? > + #include > + > + #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 Okay. > + > + # Build HDF-EOS5 > + lib_LTLIBRARIES=libhe5_hdfeos.la > +-- > +2.5.0 > + > diff --git a/gnu/packages/patches/gctp-fix-soname.patch b/gnu/packages/patches/gctp-fix-soname.patch > new file mode 100644 > index 0000000..5a32970 > --- /dev/null > +++ b/gnu/packages/patches/gctp-fix-soname.patch > @@ -0,0 +1,31 @@ > +Make library name all-lowercase. Is this a stylistic change or does it have some other effect? > + > +--- > + gctp/src/Makefile.am | 4 ++-- > + 1 file changed, 2 insertions(+), 2 deletions(-) > + > +diff --git a/gctp/src/Makefile.am b/gctp/src/Makefile.am > +index 76ecf1c..97c2438 100755 > +--- a/gctp/src/Makefile.am > ++++ b/gctp/src/Makefile.am > +@@ -4,7 +4,7 @@ > + include $(top_srcdir)/config/include.am > + > + # Library to build > +-lib_LTLIBRARIES = libGctp.la > ++lib_LTLIBRARIES = libgctp.la > + > + ## Normally DEFAULT_INCLUDES is supplied by Automake, but one of the > + ## directories included by default is $(top_builddir)/include, which > +@@ -17,7 +17,7 @@ DEFAULT_INCLUDES = -I. -I$(srcdir) > + INCLUDES=-I$(srcdir)/../include/ > + > + # Library source files > +-libGctp_la_SOURCES = gctp.c alberfor.c alberinv.c alconfor.c alconinv.c \ > ++libgctp_la_SOURCES = gctp.c alberfor.c alberinv.c alconfor.c alconinv.c \ > + azimfor.c aziminv.c bceafor.c bceainv.c br_gctp.c ceafor.c \ > + ceainv.c cproj.c eqconfor.c eqconinv.c equifor.c equiinv.c \ > + for_init.c gnomfor.c gnominv.c goodfor.c goodinv.c gvnspfor.c \ > +-- > +2.5.0 > + > diff --git a/gnu/packages/patches/gctp-remove-hdf-eos5.patch b/gnu/packages/patches/gctp-remove-hdf-eos5.patch > new file mode 100644 > index 0000000..4601c5f > --- /dev/null > +++ b/gnu/packages/patches/gctp-remove-hdf-eos5.patch > @@ -0,0 +1,482 @@ > +From c206a99fc4692ef9ea48a11bbb9de8a7dd44a7f8 Mon Sep 17 00:00:00 2001 > +From: Thomas Danckaert > +Date: Fri, 17 Jun 2016 07:55:58 +0200 > +Subject: [PATCH 1/2] Build only GCTP. > + > +--- > + Makefile.am | 2 +- > + configure.ac | 430 +---------------------------------------------------------- > + 2 files changed, 2 insertions(+), 430 deletions(-) 430 deletions: If it works, then okay :)