Skip to content
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

Open
thesamesam opened this issue Aug 19, 2024 · 4 comments
Open

Make building against Valgrind headers optional #80

thesamesam opened this issue Aug 19, 2024 · 4 comments

Comments

@thesamesam
Copy link

thesamesam commented Aug 19, 2024

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
Copy link
Member

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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants