NLnetLabs / ldns

LDNS is a DNS library that facilitates DNS tool programming
https://nlnetlabs.nl/ldns
BSD 3-Clause "New" or "Revised" License
285 stars 94 forks source link

Make test suite link to static LDNS rather than dynamic. #200

Open FGasper opened 1 year ago

FGasper commented 1 year ago

Issue #197: Cygwin fails these tests because it’s dicey to link dynamically to a library version that isn’t installed in the $PATH. Rather than adding platform-specific logic, this changeset alters the test suite to link statically rather than dynamically, which alleviates the problem.

Notably, this doesn’t resolve all problems with Cygwin in the test suite; other tests fail for other reasons. (See the GitHub issue for details.)

pemensik commented 1 year ago

I do not think such change would work well for Linux distributions. We want the test as much similar as possible. I think some variable or configure parameter used only by the cygwin builds would be better solution. Actual loading of shared library is one of things we want to test during test suite run.

FGasper commented 1 year ago

I do not think such change would work well for Linux distributions. We want the test as much similar as possible. I think some variable or configure parameter used only by the cygwin builds would be better solution. Actual loading of shared library is one of things we want to test during test suite run.

A fair point. I’ll see about adding that logic to the make scripts.

wtoorop commented 5 days ago

@FGasper have you looked at @pemensik 's comments yet?

pemensik commented 5 days ago

Maybe extra LDNS_LDFLAGS and LDNS_LIBS variables should be defined in those makefiles. Still pointing to dynamic ldns by default. But if a need to override them would arise, just make LDNS_LIBS=$PWD/libldns.a would make it use correct version.

But it seems primary problem to solve is these tests do not use libtool, do not use propagated $(LIB) variable from main Makefile.in and therefore use just vaguely defined relative path ldns library via dynamic linking. If those tests would also use libtool, it could use whatever libtool produced into LIB variable. Instead of relying on LD_LIBRARY_PATH variable to point into correct place. Libtool is not optional, right?

Is there some reason why tpkg tests do not use it? Could it override LDNS_LIBS in .pre if LDNS_LIBS environment were set, for example?

If it pointed to libldns.la, it would use dynamic library configured via libtool. If that works in Cygwin, it would fix it also. Seems a better way, but significantly more work to fix it.

pemensik commented 5 days ago
LDNS_LIBS ?= -lldns
LIBS = @LIBS@ @LIBSSL_SSL_LIBS@ $(LDNS_LIBS)

Should allow simple overriding via environment.

wtoorop commented 5 days ago

Is there some reason why tpkg tests do not use it? Could it override LDNS_LIBS in .pre if LDNS_LIBS environment were set, for example?

No, it just grew this way.

FGasper commented 5 days ago

I no longer actively use LDNS. I also don't know the relevant build tools (eg libtool) well enough to comment.