search

homalg-project / CddInterface

Gap Interface to Cdd
https://homalg-project.github.io/CddInterface
Other
1 stars 5 forks source link

setoper.h vs. cddlib/setoper.h #45

Closed AndreasEnge closed 1 month ago

AndreasEnge commented 1 month ago

Hello,

trying to add CddInterface to the gap package in the Guix GNU/Linux distribution, where each package is included into its own directory and /usr does not exist, the test program

include

fails, since cdd itself has an additional level; instead, I would suggest to write

include <cddlib/setoper.h>

and to adapt the surrounding code accordingly (drop CDD_CPPFLAGS="-I$with_cddlib/include/cdd -I$with_cddlib/include/cddlib -I$with_cddlib/include", adapt src/CddInterface.c).

The new behaviour is around since version 0.94k of cddlib, which dates from 2020.

Andreas

fingolfin commented 1 month ago

Thanks @AndreasEnge for bringing this up, I think it affects other distros, too.

What @AndreasEnge suggests sounds like the easiest solution. But of course it will cut off some older systems. If you worry about that, it would also be possible to adjust configure.ac to first test if #include "cddlib/setoper.h" works. If yes, then #define HAVE_CDDLIB_SETOPER_H ; if not, then try again with #include "cddlib/setoper.h".

Your call, @kamalsaleh :-) (or perhaps @mohamed-barakat has a strong oppinion)

kamalsaleh commented 1 month ago

Thank you @AndreasEnge and @fingolfin for your inputs. Could you please check whether the above PR resolves the issue?

AndreasEnge commented 1 month ago

It is slightly difficult to test in Guix, where we need to patch the original gap source (which uses release 2022.11.01 of the cddinterface package). With this version, the test now passes, but the HAVE_CDDLIB_SETOPER_H preprocessor constant is not defined anywhere. I think the corresponding configure.ac file misses the statement AC_CONFIG_HEADERS and the C file a corresponding include "config.h".

AndreasEnge commented 1 month ago

And cdd.h also needs to be prefixed by cddlib/

The following patch works in a git checkout of cddinterface: diff --git a/PackageInfo.g b/PackageInfo.g index bad1683..bb97c47 100644 --- a/PackageInfo.g +++ b/PackageInfo.g @@ -10,7 +10,7 @@ SetPackageInfo( rec(

PackageName := "CddInterface", Subtitle := "Gap interface to Cdd package", -Version := "2024.09.01", +Version := "2024.09.02", Date := ~.Version{[ 1 .. 10 ]}, Date := Concatenation( ~.Date{[ 9, 10 ]}, "/", ~.Date{[ 6, 7 ]}, "/", ~.Date{[ 1 .. 4 ]} ), License := "GPL-2.0-or-later", diff --git a/configure.ac b/configure.ac index f2bb10d..5856038 100644 --- a/configure.ac +++ b/configure.ac @@ -12,6 +12,7 @@ AC_PREREQ([2.68]) AC_INIT([CddInterface], [package]) AC_CONFIG_SRCDIR([src/CddInterface.c]) AC_CONFIG_MACRO_DIR([m4]) +AC_CONFIG_HEADERS([config.h]) m4_include([m4/find_gap.m4])

dnl ## @@ -62,7 +63,8 @@ old_CPPFLAGS="$CPPFLAGS" old_LDFLAGS="$LDFLAGS" CPPFLAGS="$CPPFLAGS $CDD_CPPFLAGS" LDFLAGS="$LDFLAGS $CDD_LDFLAGS" -AC_CHECK_HEADER([setoper.h], [], [AC_MSG_ERROR([could not use setoper.h])], []) +AC_CHECK_HEADER([cddlib/setoper.h], [AC_DEFINE([HAVE_CDDLIB_SETOPER_H], [1], [setting HAVE_CDDLIB_SETOPER_H=1])],

+#ifdef HAVE_CDDLIB_SETOPER_H +#include "cddlib/setoper.h" +#include "cddlib/cdd.h" +#else

include "setoper.h"

include "cdd.h"

+#endif +

include "gmp.h"

kamalsaleh commented 1 month ago

Thank you very much. Could you test again whether this resolves the issue.

AndreasEnge commented 1 month ago

I have not tested to include it into the Guix gap package, but I have tried it out separately from a git clone plus the proposed patch. Running ./autogens.sh, ./configure --with-gaproot=... and make successfully creates a library bin/x86_64-unknown-linux-gnu-default64-kv9/CddInterface.so, and ldd shows that this library is linked to my cddlib.

So it looks like this is the good solution, thanks!