jech / babeld

The Babel routing daemon
http://www.irif.fr/~jch/software/babel/
MIT License
385 stars 92 forks source link

Allow using blake2/sha2 system libaries #88

Closed DanielG closed 2 years ago

DanielG commented 2 years ago

Using ?= to assign LDLIBS allows overriding it from the environment to add (for exmaple) -lb2 on Debian.

Instead of hardcoding the path to blake2.h we use -I in CFLAGS instead and introduce a variable NO_BUNDLED_LIBS which can be set via the environment again.

Finally since keeping SRCS and OBJS in sync is tedious maximus I replace OBJS with a simple patsubst call.

jech commented 2 years ago

Does this work with BSD Make? Or have you introduced a hard dependency on GNU Make?

DanielG commented 2 years ago

Hm, I didn't realize this has to support bmake too. Honestly I don't really see the point in punishing yourself like that when gmake is perfectly available on all the BSDs too but whatever ;)

I can replace patsubst with $(SRCS:.c=.o) easily enough but conditionals become a bit narly, my best idea is introducing yet more vars that can be overridden like so:

BLAKE_SRCS ?= ...
BLAKE_CFLGAS ?= ...
SRCS += $(BLAKE_SRCS)
CFLAGS += $(BLAKE_CFLAGS)

Then I should be able to nop those out when I need to. Got any other ideas?

DanielG commented 2 years ago

Ok this should work with bsd make implementations now.

Note that before my patch at least netbsd make (aka Debian's bmake) was already broken since it doesn't use -o by default so sha2/blake objects would land in the toplevel directory instead of the subdirs where $(OBJS) was expecting them.

So probably nobody was using bsd make to build babeld without patching anyway ;)

jech commented 2 years ago

Please submit separate patches for separate features:

Please be aware that my Make-fu is very primitive, so please keep your modifications to makefiles focused and as simple as possible.

DanielG commented 2 years ago

No worries, I've pulled out the bmke fix. I've also simplified things a bit by not relying on make's = assignment late expansion so := could be used without reordering any assignments in the future.

jech commented 2 years ago

Sorry, but that's not something that I feel comfortable maintaining. I'm afraid you'll need to continue patching your local copy (which any distribution's packaging system should be able to support).

DanielG commented 2 years ago

I'm a bit confused. I made the bmake support changes which complicated things since you asked for them. I'm just as happy going back to the gmake only version which is much simpler.

jech commented 2 years ago

I'm not comfortable maintaining code that uses -I. and includes local files by following the include path. I'm also not comfortable with non-POSIX syntax in the Makefile.

Using system libraries is typically a requirement of distribution build systems, which tend to patch the Makefiles in any case. Thus, I feel it's simpler for everyone that you simply patch the Makefiles locally. If there's a good reason why you want babeld to be able to link against system libraries, without the need to modify the Makefile, then please let me know, and I'll devise a solution that I feel comfortable with.