[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add gctp
From: |
Thomas Danckaert |
Subject: |
Re: [PATCH] Add gctp |
Date: |
Sat, 18 Jun 2016 11:05:40 +0200 (CEST) |
From: Leo Famulari <address@hidden>
Subject: Re: [PATCH] Add gctp
Date: Fri, 17 Jun 2016 21:39:26 -0400
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?
The only other use I'm aware of is in HDF-EOS2, which is a separate
library from HDF-EOS5, built on HDF4 instead of HDF5, and which also
bundles gctp. I intend to package HDF-EOS2 as well, once HDF4 is
included.
We usually don't accept bundled code, but it sounds like GCTP no
longer
exists as an independent project. Is that right?
That is my impression, too (broken urls and undeliverable e-mails).
The package is quite small anyway, so perhaps bundling with the 2
HDF-EOS libraries is acceptable?
Here are some comments:
[ Style issues noted, I will take care of that. ]
+(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.
The archive does not contain an explicit version number or changelog
(it just says it's the “new C version of the GCTP” -- before that, it
seems there were some Fortran routines). I've also found a gctpc2.0
archive, which *does* have a changelog, and on closer inspection
(comparing the source of this package with comments from the
changelog from 2.0), it seems that this code corresponds to version
1.3... (though e.g. Debian also calls it 1.0). It's quite messy
actually. I'll see if HDF-EOS5 builds against gctp-2.0 (for which a
I've found a cleaner archive), and maybe package that instead...
We have a Scheme procedure for chmod. There are examples in
'gnu/packages'. Is this what required coreutils as a native-input?
That's the reason indeed. I wasn't aware of the chmod procedure, I'll
adapt my package definition .
In general, I think these patches need some more comments and
explanation of the various changes.
I'll resubmit an improved patch.
Before I do that, though, I'll await your opinion on whether an
independent gctp package is actually needed or not.
Some other explanations in-line:
+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?
About this and the other “TESTDRIVERS”-related stuff:
When we run automake after patching the build, it needs the
“testdrivers” directory. This is a set of additional
tests/demonstration programs (HDF-EOS5 already contains tests in the
src/samples directory), which are distributed in a separate tarball
(the configure script will test if the testdrivers are there or not,
so they are optional in a “standard” build scenario). In order to
allow automake to complete, I removed all references to the
testdrivers in my patch. The alternative solution would be to
download 2 tarballs (source + testdrivers) and extract them in the
build directory. Removing the testdrivers was the easiest solution.
+ 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?
Actually this only affects the HDF-EOS5 package, and I now realize it
can be removed from this patchset. I was using the same patch for
shared library compilation in my HDF5 and GCTP packages... lazyness.
(The HDF5 API changed in version 1.8. Code written against previous
version (such as HDF-EOS5) needs this #define to link with newer
versions of HDF5.)
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?
Otherwise the library is installed as “libGctp.so.0”, which seems
like an unconventional capitalization (?). Like this, users switching
from Debian & Co. will have a seamless experience ;-)
Thomas
[PATCH] Add gctp, Thomas Danckaert, 2016/06/17