Don't fail build if lsan isn't available #397
Conversation
Since lsan isn't a required dependency for these packages, they shouldn't fail to build when it isn't present. Note that part of using a sanitizer involves additional libraries, so using check_cxx_compiler_flag might not fail because it doesn't perform linking like check_cxx_source_compiles does. Signed-off-by: Scott K Logan <logans@cottsay.net>
There was a problem hiding this comment.
This generally looks good to me.
As mentioned inline, I wonder why we have the sanitizers in the compression package. I couldn't find a place where this is used. I think that could be removed all together. I not, I think I would like to have this cmake logic somewhere central, to lower the maintenance effort of it.
wrt to your alternatives, do you have an idea who to add optional test dependencies?
On Fedora, the missing package is Maybe this is just a Redhat thing? |
Signed-off-by: Scott K Logan <logans@cottsay.net>
|
I am okay with this solution. I am waiting for @zmichaels11 or maybe @piraka9011 to comment on the usage within the compression. But it generally looks good to me overall. |
|
@cottsay do you mind triggering a CI for this? Just making sure that our main platforms don't experience any hiccups with this. |
Signed-off-by: Scott K Logan <logans@cottsay.net> Co-authored-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Signed-off-by: Scott K Logan <logans@cottsay.net>
|
|
Interesting - the check failed on macOS. I'll look into it. |
Signed-off-by: Scott K Logan <logans@cottsay.net>
I see, the flags only get added later for GCC. I made the test only happen for GCC then, which preserves the behavior of leaving Side note - the test silently succeeds on MSVC even though the flag isn't supported. It looks like Reruns:
|
Since lsan isn't a required dependency for these packages, they shouldn't fail to build when it isn't present.
Note that part of using a sanitizer involves additional libraries, so using
check_cxx_compiler_flagmight not fail because it doesn't perform linking likecheck_cxx_source_compilesdoes.Alternatively, the sanitizer(s) could be added either to the ROS 2 installation instructions or as an explicit dependency for these packages.