trusteddomainproject / OpenDKIM

Other
97 stars 52 forks source link

Library searches append junk to the linker path #105

Open orlitzky opened 4 years ago

orlitzky commented 4 years ago

Several library tests in configure.ac loop through a list of paths looking for some header or library. For example,

bdbdirs="/usr/local/BerkeleyDB /usr/local /usr"

for d in $bdbdirs; do
...
  AC_SEARCH_LIBS([db_create], $bdb_lib,
                 [libdbfound="yes"])
  bdb_libdir=$d/lib

This misses the most important (and highest-priority) search path of all -- the empty path! Most people are able to pass things like -ldb and -lssl on the command-line without any additional -L paths. Since that always works, the test above incorrectly concludes that /usr/local/BerkeleyDB needs to be added to my library search path, and I wind up with -L/usr/local/BerkeleyDB appended to all of my linker invocations. For example,

libtool: link: x86_64-pc-linux-gnu-gcc -pthread -O2 -pipe -march=native -pthread -Wl,-O1 -o .libs/opendkim opendkim-opendkim.o opendkim-opendkim-ar.o opendkim-opendkim-arf.o opendkim-opendkim-crypto.o opendkim-opendkim-db.o opendkim-opendkim-dns.o opendkim-opendkim-lua.o opendkim-config.o opendkim-flowrate.o opendkim-reputation.o opendkim-stats.o opendkim-test.o opendkim-util.o  -L/usr/lib -L/usr/local/BerkeleyDB/lib -Wl,--as-needed ../libopendkim/.libs/libopendkim.so -lmilter -lssl -lcrypto -ldb ../libvbr/.libs/libvbr.so -lresolv -lbsd -pthread

The addition of that BerkelyDB path is harmless, but the problem exists elsewhere too. In the linker command above, I also have -L/usr/lib appended. On my machine, /usr/lib64 is correct and /usr/lib is for 32-bit libraries. If I have both 64- and 32-bit copies of a library installed, the command above will attempt to use the 32-bit one first and fail. The erroneous path is appended here, I believe:

for d in lib lib64 lib/libmilter; do
...
  AC_SEARCH_LIBS([smfi_register], [milter],
  [
  LIBMILTER_LIBDIRS="-L$milterpath/$d"
  LIBMILTER_LIBS="-lmilter"

The AC_SEARCH_LIBS would succeed with no additional -L flags, but the test doesn't try that first. As a result, it mistakenly assumes that the first non-default path in the list must be the correct one. This leads to build failures in some situations when the wrong libraries get pulled in, e.g. https://bugs.gentoo.org/751286

orlitzky commented 4 years ago

I wasn't around when all the path-guessing was added, but I think something much simpler should work for libmilter at least. This isn't tested very well yet, but should get the idea across: https://github.com/orlitzky/OpenDKIM/commit/a1371d8c81d5fc22cbc8ea2b1c9eb465e9a8e874

thegushi commented 1 year ago

I feel like there's a better way to do this in modern autoconf, but need to wrap my head around it more. I'll take ownership of this.

orlitzky commented 1 year ago

In general I would say the least bad way is to use pkg-config via the PKG_CHECK_MODULES macro. But only when there's a popular library that is a headache to use without pkg-config; for example parallel installations of different versions of GTK.

Otherwise, a simple AC_SEARCH_LIBS works. In this case, there's only one libmilter, and nobody installs multiple copies of it. On the off chance that someone (say) builds their own copy in $HOME, they can always do the "standard" thing and add the appropriate $LDFLAGS in their environment to allow autoconf to find it.

The downsides to pkg-config are that,

  1. it looks for specific names and versions instead of interfaces;
  2. it requires users to have yet another tool installed; and
  3. it actually makes building your own copy harder. Normally you only have to point your compiler/linker at a custom library for autoconf to find it, but if the build system uses pkg-config, then the user also has to trick pkg-config into looking in the right place. This goes a bit against the autotools philosopy, and strays outside of what is defined by POSIX.

So in short, I would just do vanilla AC_SEARCH_LIBS here, and drop the argument to the --with-milter flag. (While we're at it, I think a better name would be --enable-milter or --with-libmilter per the autoconf docs, but those are more annoying changes.)