Upgrade and clean up cddlib

[kini]

The cddlib SPKG is huge (when disregarding src/) and contains a lot of weird-looking stuff in the patched directory. The point of this ticket is to simplify the packaging of cddlib and also upgrade it to the latest version, 094g.

Part of metaticket #13025.

Use tarball at:

• http://boxen.math.washington.edu/home/jpflori/upstream/cddlib-094g.tar.bz2

CC: @ohanar @jpflori @SnarkBoojum

Component: packages: standard

Keywords: cddlib sd40.5

Author: R. Andrew Ohana

Branch: 322ec47

Reviewer: Karl-Dieter Crisman, Jeroen Demeyer, Jean-Pierre Flori

Issue created by migration from https://trac.sagemath.org/ticket/13026

[ohanar]

Author: R. Andrew Ohana

[ohanar]

Changed keywords from cddlib to cddlib sd40.5

[kcrisman]

Work Issues: make sure hard links used

[kcrisman]

Reviewer: Karl-Dieter Crisman

[kcrisman]

comment:3

This all makes sense. I've opened #13056 to address that we might eventually want to have the docs available again.

All spkg and geometry/ tests pass on sage.math and my MacBook Pro. As discussed with Andrew, just want to make sure that hard links were used as pointed out in SPKG.txt, which I can't check - otherwise positive review.

[ohanar]

comment:4

Ok, hard links should be setup now.

[ohanar]

Changed work issues from make sure hard links used to none

[jdemeyer]

comment:7

This doesn't even compile for me with gcc-4.2.4 on boxen.math:

****************************************************
Host system:
Linux boxen 2.6.24-24-server #1 SMP Fri Sep 18 16:47:05 UTC 2009 x86_64 GNU/Linux
****************************************************
C compiler: gcc
C compiler version:
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared --with-system-zl
ib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --with-gxx-include-dir=/usr/include/c++/4.2 --prog
ram-suffix=-4.2 --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr --enable-checking=release --build=x86_64-linu
x-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.2.4 (Ubuntu 4.2.4-1ubuntu4)
****************************************************
patching file src/lib-src/cddcore.c
patching file src/lib-src/cddlp.c
patching file src/Makefile.in
patching file src/src-gmp/Makefile.in
patching file src/src/Makefile.in
patching file src/lib-src/Makefile.in
patching file src/lib-src-gmp/Makefile.in
configure: WARNING: unrecognized options: --with-gmp
[...]
libtool: compile:  gcc -DPACKAGE_NAME=\"\" -DPACKAGE_TARNAME=\"\" -DPACKAGE_VERSION=\"\" -DPACKAGE_STRING=\"\" -DPACKAGE_BUGREPORT=\"\" -D
PACKAGE=\"cddlib\" -DVERSION=\"0.94\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE
_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 -DLT_OBJDIR=\".libs/\" -DHAVE_LIBG
MP=1 -DSTDC_HEADERS=1 -I. -DGMPRATIONAL -g -O2 -MT cddio.lo -MD -MP -MF .deps/cddio.Tpo -c cddio.c  -fPIC -DPIC -o .libs/cddio.o
In file included from cdd.h:18,
                 from cddmp.c:21:
cddmp.h:31:18: error: gmp.h: No such file or directory
[...]

Note the line

configure: WARNING: unrecognized options: --with-gmp

[kcrisman]

comment:8

Huh. I did test this on sage.math and OS X, but it's true that I didn't try with random other versions of GCC. I'm not sure what to say about that, and I can't review this one further.

[jdemeyer]

comment:9

I don't think the GCC version has anything to do with it. Probably both machines you tested have a system-wide /usr/include/gmp.h but boxen.math doesn't.

[kiwifb]

comment:10

I can have a look at that. Tomorrow hopefully, but it looks like to me we just need to set CPPFLAGS="-I$SAGE_LOCAL/include" when we run the configuration, LDFLAGS may also need to be set.

[vbraun]

comment:11

Support for the --with-gmp configure option was one of the things that I patched into cddlib. Yes, upstream does not have that. Yet another reason why IMHO you can't have a minimally invasive patch to make it work, just too broken.

[kiwifb]

comment:12

So it was some of your stuff. On the other hand upstream included a slightly updated version of the libtool patch I sent them probably about 3-4 years ago :)

In any case adding the following line to configure should solve the problem

CPPFLAGS="-I$SAGE_LOCAL/include" LDFLAGS="-L$SAGE_LOCAL/lib"

Alternatively the variables can be exported and it should be safe on any system. Your original spkg works on my mac so I could have gmp headers somewhere, strangely enough it seems to find the correct sage libraries so may be there is something more subtle going on

/bin/sh ../libtool --tag=CC   --mode=link gcc  -g -O2   -o cdd_both_reps_gmp cdd_both_reps.o ../lib-src-gmp/libcddgmp.la -lgmp 
libtool: link: gcc -g -O2 -o .libs/cdd_both_reps_gmp cdd_both_reps.o  ../lib-src-gmp/.libs/libcddgmp.dylib /Users/frb15/Desktop/sage-5.1.beta1/local/lib/libgmp.dylib

[ohanar]

comment:13

Ok, I've updated the spkg, hopefully the issues are fixed (it now compiles fine for me with boxen's toolchain).

[jdemeyer]

Diff for the cddlib spkg. For reference / review only.

[vbraun]

comment:54

Not building a shared library would be a serious regression IMHO.

[kiwifb]

comment:55

Replying to @sagetrac-git:

Branch pushed to git repo; I updated commit sha1. New commits:

269c1e1

Previously failing cddlib related doctest now pass.

Thank you. I have had that doctest error in sage-on-gentoo for a year and 19 days - when we moved to 0.94g.

[jpflori]

comment:56

Replying to @vbraun:

Not building a shared library would be a serious regression IMHO.

I didn't disable the build of a shared library.
I disabled the disabling of the build of a static library.

[jpflori]

comment:57

Once we fix #15872 we can feed back the --enable-shared --disable-static stuff to configure.
If that's a sufficient reason reason for you to review that ticket, I'll include it there with pleasure.

[jpflori]

comment:58

(For now there's nothing there, I'm aware of that.)

[jpflori]

comment:59

I was a little too fast.
The mv part of spkg-install are wrong with the git version of sage.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

322ec47

Replace mv by cp in cddlib install script.

[sagetrac-git]

Changed commit from 269c1e1 to 322ec47

[jpflori]

comment:61

Or not actually, as I guess we still copy everything before building....
Anyway, changes are here.
I guess we can keep it as positive review then.

[jpflori]

comment:62

For some mysterious reason the previously failing doctest does not fail either when I revert back to trac/develop, make distclean and rebuild...

[jpflori]

comment:64

Replying to @jpflori:

For some mysterious reason the previously failing doctest does not fail either when I revert back to trac/develop, make distclean and rebuild...

No such problem on another machine.

[vbraun]

Changed branch from u/jpflori/ticket/13026 to 322ec47

[SnarkBoojum]

comment:66

Sorry to come so late to the party, but looking at sage's git, I don't really see why this bug was closed: the cddlib spkg still has patches that mean sage isn't based on cddlib, but on a fork...

[SnarkBoojum]

Changed commit from 322ec47 to none

[vbraun]

comment:67

The bug was closed because the branch was merged. It might not be perfect but it was an improvement. Feel free to work on this.