-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make building against Valgrind headers optional #80
Comments
I think we can go two ways:
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? |
I almost agree with 2, the problem is you get a non-deterministic build depending on whether Valgrind was installed when you built DTrace... |
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". |
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). |
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.The text was updated successfully, but these errors were encountered: