coin-or / DyLP

Dynamic Simplex solver
Other
2 stars 5 forks source link

Fails to build on Gentoo with -Werror=implicit-function-declaration #27

Open eli-schwartz opened 7 months ago

eli-schwartz commented 7 months ago
 x86_64-pc-linux-gnu-gcc -DHAVE_CONFIG_H -I. -I. -I../../src/DylpStdLib -I./../DylpStdLib -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wformat -Werror=format-security -Werror=implicit-function-declaration -Werror=implicit-int -Werror=int-conversion -Werror=incompatible-pointer-types -DDYLP_BUILD -fno-strict-aliasing -c dy_consys_utils.c  -fPIC -DPIC -o .libs/dy_consys_utils.o
dy_consys_utils.c: In function 'consys_getcoeff':
dy_consys_utils.c:2980:13: error: implicit declaration of function 'quiet_nan' [-Werror=implicit-function-declaration]
 2980 |     return (quiet_nan(0)) ; }
      |             ^~~~~~~~~
cc1: some warnings being treated as errors
make[1]: *** [Makefile:532: dy_consys_utils.lo] Error 1

I am not even sure where quiet_nan should come from. config.log reports that 2006-era autoconf is reporting such a function exists. I am not sure why.

This has been reported at https://bugs.gentoo.org/878143

eli-schwartz commented 4 months ago

https://github.com/coin-or/DyLP/blob/1413211ed51aa6cad062d3a5b65a77407518feaa/configure.ac#L107-L113

https://github.com/coin-or/DyLP/blob/1413211ed51aa6cad062d3a5b65a77407518feaa/m4/ac_c_get_sunpro_libs.m4#L9-L11

Indeed, it appears quiet_nan is Sun-specific code. It is also described as "legacy math library" so I wonder if it really makes sense to use it at all...

In theory regenerating autoconf could likely fix this, in practice the latest stable release from 2019 cannot actually regenerate:

/usr/share/autoconf-2.71/autoconf/trailer.m4:4: warning: AC_OUTPUT was never used
configure.ac:45: error: possibly undefined macro: AC_COIN_PROJECTDIR_INIT
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure.ac:48: error: possibly undefined macro: AC_COIN_DEBUG_COMPILE
configure.ac:51: error: possibly undefined macro: AC_COIN_PROG_CC
configure.ac:54: error: possibly undefined macro: AC_COIN_PROG_CXX
configure.ac:57: error: possibly undefined macro: AC_COIN_INIT_AUTO_TOOLS
configure.ac:67: error: possibly undefined macro: AC_COIN_CHECK_PACKAGE
configure.ac:82: error: possibly undefined macro: AC_DYLP_FIX_CPPFLAGS
configure.ac:86: error: possibly undefined macro: AC_DYLP_EQUIV_FOR_CPP_BOOL
configure.ac:99: error: possibly undefined macro: AC_COIN_CHECK_LIBM
configure.ac:104: error: possibly undefined macro: AC_C_ADD_TO_INCLUDES
configure.ac:121: error: possibly undefined macro: AC_DYLP_GET_SUNSTUDIO_LIBDIRS
configure.ac:136: error: possibly undefined macro: AC_DYLP_FIND_ISFINITE
configure.ac:137: error: possibly undefined macro: AC_DYLP_FIND_ISNAN
configure.ac:196: error: possibly undefined macro: AC_DYLP_PARANOIA
configure.ac:197: error: possibly undefined macro: AC_DYLP_STATISTICS
configure.ac:198: error: possibly undefined macro: AC_DYLP_INFO
configure.ac:204: error: possibly undefined macro: AC_ODSI_INFO
configure.ac:205: error: possibly undefined macro: AC_ODSI_STATISTICS
configure.ac:206: error: possibly undefined macro: AC_ODSI_PARANOIA
configure.ac:215: error: possibly undefined macro: AC_COIN_DOXYGEN
configure.ac:224: error: possibly undefined macro: AC_COIN_VPATH_LINK
configure.ac:271: error: possibly undefined macro: AC_COIN_FINALIZE
LouHafer commented 4 months ago

Eli, two things to say. First, you need the COIN-OR BuildTools to regenerate a COIN project starting from the autotools level. See the run_autotools script in that repo. But this would be low on my list of suspects.

There's a macro definition in src/Dylp/dy_vector.h that should handle the (usual) case where no quiet_nan function is present. Not sure why that's failing. I'll poke around and see if I can duplicate the problem.

eli-schwartz commented 4 months ago

Thanks for the link. That BuildTools repository confuses me, though. I'm fairly familiar with using autotools; assuming that coin-or has project-specific (or even org-specific) macros that need to be used, one could install them e.g. the same way autoconf-archive does. It's even possible to fork and maintain your own modified versions of builtin macros.

I get a little lost by the need to compile and install a custom copy of autotools? I have autotools already. Several copies of it, even -- Gentoo allows me to specify which version a package needs, see WANT_AUTOCONF etc. at https://devmanual.gentoo.org/eclass-reference/autotools.eclass/index.html as well as to install many of them simultaneously.

Are you making specific modifications to autoconf/automake/libtool?

Usually the *.m4 files that get used to create a configure script can/do get copied into the source tree by aclocal, allowing people to patch them and/or regenerate the configure script using a newer autoconf.

EDIT: Cloning BuildTools and using autoreconf -fi -I path/to/BuildTools resolves several of the missing macros but by no means all of them.

But this would be low on my list of suspects.

The reason I wanted to try it is because newer versions of autoconf have a much better expanded definition of AC_CHECK_FUNC and the one in the dist tarball was produced by autoconf 2.59.

There's a macro definition in src/Dylp/dy_vector.h that should handle the (usual) case where no quiet_nan function is present. Not sure why that's failing. I'll poke around and see if I can duplicate the problem.

Thanks for taking a look.

As I said, when I run ./configure, it reports on linux:

checking for quiet_nan... yes

So the macro definition in dy_vector.h is told it isn't needed. AC_CHECK_FUNC([quiet_nan], ... is erroneously reporting falsehoods.

It is high on my list of suspects, that this falsehood is reported because autoconf 2.59 is missing out on many years of robustness fixes.

LouHafer commented 4 months ago

Sorry, missed where you said configure was giving the wrong result --- that's clearly the problem. Not sure why the tarball is so old. Will do some checking. Current BuildTools uses reasonably current autotools.

eli-schwartz commented 4 months ago

Update: I cloned BuildTools, tried using it to regenerate configure for DyLP. It failed due to still missing macros. After a bit of rummaging around in the BuildTools history, I hit upon this commit, which removes some macros: https://github.com/coin-or-tools/BuildTools/commit/f0363fe4ef8b11940138a39fb90ec53717e0b5de

By checking out the commit immediately before that, I was able to successfully rerun autoreconf-2.71. (BuildTools must be in the DyLP directory, as Makefile.am uses include BuildTools/Makemain.inc).

Here is the difference in configure scripts: configure.diff.txt

Unfortunately the result seems to be fairly broken. It's performing compiler detection (AC_PROG_CC?) twice, the first time without ac_objext set, and the second time reusing the cached values. The resulting Makefile eventually tries to pass various linker options directly to the compiler without -Wl, and fails. And that's enough debugging for now...

Notice though that with the new configure script, the quiet_nan check looks like this:

char quiet_nan (void);

int
main (void) {
return quiet_nan ();
  ;
  return 0;
}

The old tarball uses this:

char quiet_nan ();
char (*f) () = quiet_nan;

int
main ()
{
return f != quiet_nan;
  ;
  return 0;
}

The former is guaranteed to generate a symbol at runtime... autoconf changed that IIRC due to compiler optimizations.

svigerske commented 4 months ago

I'm not sure which version of DyLP is actually used here. The latest releases still rely on a pretty old version of the autotools. To regenerate configure, etc, for this, you will need to use branch stable/0.8 of BuildTools. There is a corresponding script to install the required autotools versions. But as these are indeed very old, that won't help to get around the problem. I would suggest to instead put some fallback into the code in this old branch of DyLP.

Some years ago, there was a larger update to the BuildTools, which also meant updating to a much more recent autotools version. DyLP master branch has been updated to this, but there hasn't been a release so far. (One reason may be that dependencies CoinUtils and Osi didn't have a release either.) The current BuildTools/master also expects a particular version of autotools, but we should be only about a year behind the latest at the moment. Chances are good that the issue with quiet_nan() is resolved on DyLP/master.

Our setup may not be most standard. It somehow worked for us, though. We indeed patch up libtool and the compile script, as also the latest versions did not include everything that is necessary for us, in particular for building on Windows (https://github.com/coin-or-tools/BuildTools/blob/master/coin.m4#L339-L356, https://github.com/coin-or-tools/BuildTools/blob/master/compile.patch, https://github.com/coin-or-tools/BuildTools/blob/master/libtool-icl.patch). These patches can require extra work when updating to newer autotools versions.

LouHafer commented 4 months ago

Thanks, Stefan! Would have taken me a while to sort that out.

Eli, my apologies for being so slow to clue in to what you were saying. The releases are indeed old, and I've been working with the new BuildTools for years (was one of the folks who contributed to the updates). The master branches for BuildTools and Dylp are compatible and are what you want to use. (At least, they worked for me yesterday when I started checking into this :-) If you go this route, you really do want to use the run_autotools script, it deals with all the details of BuildTools setup, teardown, patches, etc. Just drop BuildTools and Dylp into sibling directories. Then from the parent, the command BuildTools/run_autotools Dylp should do it. If you need different autotools versions, look around line 70 in run_autotools. It should be safe to change autoconf and automake. Libtool, probably not. As Stefan mentions, it's patched.

LouHafer commented 4 months ago

Eli, I should add that unless you have some specific use case for dylp, you're better off using one of the other LP codes out there: HiGHS or clp from COIN, or glpk. Dylp still works, but there's been no active algorithm development for 15 years.

eli-schwartz commented 4 months ago

libtool patches... fun. :D

Gentoo has one of those meant for quick fixups without regenerating projects: https://packages.gentoo.org/packages/app-portage/elt-patches Very handy for backporting targeted fixes into existing release tarballs, and autodetects which patches are applicable to any given version of ltmain.sh. Kind of a lot of work to maintain it though. :D

eli-schwartz commented 4 months ago

Eli, I should add that unless you have some specific use case for dylp, you're better off using one of the other LP codes out there: HiGHS or clp from COIN, or glpk. Dylp still works, but there's been no active algorithm development for 15 years.

As a disclaimer, my interest in this is solely because Gentoo has a distro package for it, and it fails to build (in particular, -Werror=implicit-function-declaration becomes the default in GCC 14), and I've been triaging tons of issues like this.

I would like to either have it building well, or else perhaps dropped from the Gentoo repos if no one is actually using it, with a side bonus of "if we have issues, maybe other people are having issues too, so it would be good to let upstream be aware of the problem".

There is no particular rush, to be honest.

LouHafer commented 4 months ago

You might want to err towards 'perhaps dropped.' But this exchange will give us COIN folks a nudge towards updating our releases, so some good may come of it.