oracle / dtrace-utils

DTrace-utils contains the DTrace port to Linux
Other
132 stars 19 forks source link

Make building against Valgrind headers optional #80

Open thesamesam opened 3 weeks ago

thesamesam commented 3 weeks ago

Valgrind isn't available on all (really, most) arches, but also, even those on supported arches may not love installing Valgrind purely for the headers if it won't ever be used at runtime (e.g. on an AVX512 machine any time soon...).

At https://github.com/oracle/dtrace-utils/blob/devel/libdtrace/dt_work.c#L22, we include <valgrind/valgrind.h> unconditionally.

This came up downstream at https://bugs.gentoo.org/938190 where I wrongly removed the DEPEND on Valgrind because I grepped badly.

kvanhees commented 3 weeks ago

I think we can go two ways:

  1. introduce an option to select whether building with valgrind support or not
  2. automatically build with valgrind support if the needed header file is present

I am inclined to go with 2. simply because it is easier and harmless, and 1. still would require a test that the header is actually present (rather than just letting the build).

Thoughts?

thesamesam commented 3 weeks ago

I almost agree with 2, the problem is you get a non-deterministic build depending on whether Valgrind was installed when you built DTrace...

thesamesam commented 2 weeks ago

Another option is we use a bundled header or inline the relevant macros as a fallback. This came up in GCC and the conclusion was "Valgrind is very unlikely to change its interface".

nickalcock commented 2 weeks ago

Honestly, I'm astonished I'm not already using a header guard. IMHO, we should obviously only be including <valgrind/valgrind.h> if it exists, we should obviously be trying to use it if present by default but not failing by default if absent, and we should obviously have a configure option to force-require its presence, just as with other optional features (and "cooperate when running under valgrind" presumably counts as one of those).