Opened 6 years ago

Closed 22 months ago

#105 closed defect (fixed)

shared libraries do not properly list dependencies

Reported by: mlubin Owned by: stefan
Priority: major Component: build system
Version: 0.7 Keywords:
Cc:

Description

Previous discussions:

http://list.coin-or.org/pipermail/coin-discuss/2008-October/003480.html http://list.coin-or.org/pipermail/ipopt/2009-May/001531.html http://list.coin-or.org/pipermail/clp/2012-June/001265.html

Also open as ipopt ticket 115, but this is a more appropriate place.

This affects loading coin libraries from dynamic languages and packaging coin projects for Fedora and Debian.

Attachments (5)

dependencies_multipatch_no_LIBADD_for_CL.patch (20.9 KB) - added by kelman 6 years ago.
dependencies_multipatch.patch (20.2 KB) - added by kelman 6 years ago.
updated patch to account for Glpk and Metis now checking for -lm in configure
dependencies_multipatch_config_option.patch (21.7 KB) - added by kelman 6 years ago.
Patch to make use of new option in r3062. Will test this shortly.
dependencies_coinall_trunk.patch (24.6 KB) - added by kelman 5 years ago.
Improved handling of ASL a bit. Couenne is last remaining non-straightfoward change, will contact Couenne mailing list about this.
dependencies_fix_couenne_dip.patch (2.0 KB) - added by kelman 5 years ago.
Improve Couenne so it's actually no change when DEPENDENCY_LINKING is false, fix another missing dependency in lib_dippy, and add a newline at the end of a dippy header

Download all attachments as: .zip

Change History (23)

comment:1 Changed 6 years ago by kelman

Some relevant recent work I did for Bonmin (and its dependencies): http://list.coin-or.org/pipermail/bonmin/2013-October/000386.html

Building DLL's in MinGW requires passing -no-undefined to libtool, and a bit of minor hacking around some of the current assumptions BuildTools? is making. And all of the inter-dependencies have to be specified and built in the correct order. This could be relevant here, and could be improved upon by using some of the existing dependency information in the Makefile.am's. Referring to dependencies in the build location instead of the install location would probably be a better idea, I kept seeing some strange issues with relinking after make install'ing dependencies.

There are a few places where subdirectories get built before the libraries they depend on, which could also require some changes.

comment:2 Changed 6 years ago by kelman

Here is a multi-patch for the automake files of OS/trunk and all of its dependencies (so not quite everything in CoinAll?, but most of them) that allows building on a Linux box with LDFLAGS="-Wl,--no-undefined".

Mostly it's just a matter of slight build-order rearrangement (the linear solvers shouldn't recursively build their Osi* subfolder before the solver library itself is built) and adding fairly obvious things to the lib*_la_LIBADD automake variables.

Bonmin and OS need a dummy (nodist_EXTRA_) C++ source file to make sure they link libraries using CXXLINK instead of LINK.

Couenne is messier... src/libCouenne depends on src/main/libBonCouenne, but the couenne AMPL executable is also built in src/main. The couenne executable depends on src/readnl/libCouenneReadnl, but libCouenneReadnl depends on src/libCouenne. My solution to this was to build the sources of src/main/libBonCouenne directly into libCouenne. Would want to talk to Couenne developers about whether this is ok or makes any difference.

Haven't tested yet on Win or Mac platforms, but will soon. Combined with a handful of BuildTools? tweaks, this should greatly simplify DLL building, among the other useful benefits. Can anyone find a use case that gets broken by these changes?

comment:3 Changed 6 years ago by stefan

I see -lm -ldl added to some libtool calls. Wouldn't that fail on most systems other than Linux?

The main reason why we did not add LIBADD so far was because it did not work with MS/Intel compilers on Windows. I don't know why exactly it doesn't work, but that's why Andreas and co. decided not to use it and go with this addlibs files workarounds back then.

comment:4 Changed 6 years ago by kelman

You're right, -ldl is no good. It was only in ASL, and I missed that there's an ASL_PCLIBS variable that configure populates with -ldl when appropriate. Revised patch attached. -lm was needed in GLPK and Metis, I think that's safer though?

I was able to get DLL's of everything in OS with this and changing coin_disable_shared to no in coin.m4. But make install has trouble relinking, which is a libtool problem I'll look into. Maybe newer versions of libtool will work better.

Guess I could try with MSVC and compile_f2c (don't have ifort) to see if/what this might break there.

comment:5 Changed 6 years ago by kelman

Oh, couldn't we predicate all the LIBADD additions with an if !COIN_CC_IS_CL ?

comment:6 Changed 6 years ago by stefan

MS/Intel compilers on Windows will not understand -lm. They don't understand -l<libname> in general. Maybe patches in libtool will change this to libm.lib, but I'm not sure that this one exists, either. We have logic in configure that adds -lm to linker flags, so I doubt that you will have to do this in the Makefile explicitly again.

comment:7 Changed 6 years ago by kelman

I couldn't find anything related to -lm in configure, config.log, or the generated Makefiles for Metis or Glpk, with the exception of FLIBS. Since neither Metis nor Glpk use any Fortran, that's not useful here. I see a macro AC_COIN_CHECK_LIBM that Ipopt's configure.ac uses, we could add something similar in the configure of Metis and Glpk, but it doesn't look like there's such a check right now.

What do you think of not adding to LIBADD when COIN_CC_IS_CL?

Last edited 6 years ago by kelman (previous) (diff)

comment:8 Changed 6 years ago by stefan

If Metis and Glpk need -lm, then AC_COIN_CHECK_LIBM should be added to their configure.ac files. Maybe it was never recognized that they have these dependencies, since -Wl,--no-undefined was not there.

Adding to LIBADD if !COIN_CC_IS_CL sounds like a viable workaround to me. I would prefer a solution where one can also create dll's if using MS/Intel compilers, but that seems like a big undertaking. Wouldn't do this before having updated the autotools and have found a better way to support cl/icl/ifort.

comment:9 Changed 6 years ago by kelman

I see you added that in r3056 and r3057. Looks like you're missing an argument to the AC_COIN_CHECK_LIBM macro (all it did was add an empty if statement), it should be AC_COIN_CHECK_LIBM(Glpk) and AC_COIN_CHECK_LIBM(Metis). And for Metis, there are no LIBS variables getting sent to the Makefiles yet. Adding either AC_SUBST(METIS_LIBS) or AC_SUBST(METIS_PCLIBS) somewhere after the AC_COIN_CHECK_LIBM(Metis) should fix that.

We should probably open a new ticket for discussion specifically about Windows DLL building, since it's technically a separate issue that just requires these dependency tracking LIBADD additions (which apply to many platforms) to be resolved first.

I ran some tests with cl and f2c, LIBADD doesn't seem to break anything that I can tell when doing a baseline static build. There are libtool problems that make dll building with cl more difficult than dll building with gcc, and libtool doesn't really work at all for cl in Cygwin. I don't think even the latest version of libtool can make native compilers understand Cygwin-style paths, but I could be wrong there. MSYS does do some transparent path translation, works for static builds with or without LIBADD additions. MSYS almost works for dll builds (with LIBADD additions, those will be needed for dll's with either gcc or cl) but has some libtool bugs that I think are more likely to be fixed in recent versions: wrapper executables can't find unistd.h, and relinking-on-install doesn't properly translate Linux style link flags to cl style (--mode=link does the correct replacement of .la names with .lib, --mode=relink doesn't).

When you say "wouldn't do this," do you mean DLL's with MS compilers will have to wait for autotools update, or anything related to dependencies will have to wait for autotools update? I've seen there is an autotools-update branch of BuildTools?, do you have any info on how far that got and how we might be able to help?

comment:10 Changed 6 years ago by stefan

I seem to like introducing a new bug with every commit. Issues from r3056 and r3057 should be fixed by r3059 and r3060.

I was thinking of setting LIBADD for all builds only after updating autotools and having cleaned up coin.m4. Already in the non-MS case, I am worried it would require more that just setting LIBADD. I can imagine we would have to change the way how the .pc files are setup, too. However, there is additional pressure from Ted and others, so the changes for non-MS builds may happen earlier. (For GAMSlinks, I also need to have one shared library that has links to all dependencies stored in the .so file, but I resolved this by doing one LIBADD in the GAMSlinks project, there was no need to do this in all projects.)

Changed 6 years ago by kelman

updated patch to account for Glpk and Metis now checking for -lm in configure

comment:11 Changed 6 years ago by kelman

If there are some unforeseen issues that could cause problems from LIBADD additions, there's always the possibility of basing it on a new disabled-by-default configure option, --enable-dependency-linking or something like that (maybe --disable-undefined and enabled by default?). Since this is a change that applies for multiple platforms, multiple compilers, several different use cases: packaging, dynamic languages, Windows dll's...

Could add a new AC_ARG_ENABLE that sets an AM_CONDITIONAL to conditionally add to LIBADD. Would also want to use the same flag to set LT_LDFLAGS=-no-undefined at https://projects.coin-or.org/BuildTools/browser/trunk/coin.m4#L1820, resolve a long-standing "ToDo?". That setting would be mandatory when on Windows with --enable-shared (aiming towards making that combination allowed but non-default), not entirely necessary elsewhere.

libipoptamplinterface and libBonCouenne don't currently have the line lib*_la_LDADD = $(LT_LDFLAGS) that all the other libraries do, that's easy to fix.

comment:12 Changed 6 years ago by stefan

r3062 adds the configure option --enable-dependency-linking and the automake conditional DEPENDENCY_LINKING. The option is not documented so far and is disabled by default. If enabled, it also sets LT_LDFLAGS to -no-undefined.

I also fixed the LDADD's for libipoptamplinterface and libBonCouenne.

Changed 6 years ago by kelman

Patch to make use of new option in r3062. Will test this shortly.

Changed 5 years ago by kelman

Improved handling of ASL a bit. Couenne is last remaining non-straightfoward change, will contact Couenne mailing list about this.

comment:13 Changed 5 years ago by stefan

I've applied dependencies_coinall_trunk.patch to CoinAll?/trunk.

comment:14 Changed 5 years ago by kelman

Cool. Looks like a local change to OSnl2OS.cpp may have accidentally snuck in there? https://projects.coin-or.org/OS/changeset/4750#file12

comment:15 Changed 5 years ago by stefan

Thanks (and I thought I checked before). Should be fixed now.

comment:16 Changed 5 years ago by kelman

This minor fix improves Couenne handling a little bit so the default behavior (without --enable-dependency-linking) is the same as it was prior to https://projects.coin-or.org/Couenne/changeset/1023

I also missed some dependencies in lib_dippy, since I had been working on systems that didn't have python-dev headers installed (so dippy was getting skipped). There's also a header that should end in a newline (clang flags that as an error), technically a separate issue, but while I'm at it in the same folder...

Changed 5 years ago by kelman

Improve Couenne so it's actually no change when DEPENDENCY_LINKING is false, fix another missing dependency in lib_dippy, and add a newline at the end of a dippy header

comment:17 Changed 5 years ago by stefan

Thanks, I committed this.

comment:18 Changed 22 months ago by stefan

  • Resolution set to fixed
  • Status changed from new to closed

The --enable-dependency-linking has become default with BuildTools 0.8, so I believe this is fixed. If there was still something open, then reopen and remind me what that was.

Note: See TracTickets for help on using tickets.