[PATCH] D47819: [compiler-rt] [test] Disable sunrpc tests when rpc/xdr.h is missing

Peter Wu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 14 09:38:32 PST 2019


Lekensteyn added a comment.

The direction of this patch looks reasonable to me. Is it worth mentioning the issue (https://github.com/google/sanitizers/issues/974) in the commit message?

What environment have you tested this in, could you verify that the tests are indeed skipped on a system without these headers, and passed otherwise?



================
Comment at: lib/sanitizer_common/CMakeLists.txt:195
+set(SANITIZER_COMMON_DEFINITIONS
+  HAVE_RPC_XDR_H=${HAVE_RPC_XDR_H})
 
----------------
The old code defined HAVE_xxx to 0 or 1, but in general wouldn't it be preferable to define it to 1 if available, and not define it at all if unavailable? This would match most other HAVE_xxx checks.

In order to make the lit config work in this case, you have to use something like `pythonize_bool(HAVE_RPC_XDR_H)`.


================
Comment at: test/lit.common.configured.in:39
 set_default("android", @ANDROID_PYBOOL@)
+set_default("have_rpc_xdr_h", @HAVE_RPC_XDR_H@)
 config.available_features.add('target-is-%s' % config.target_arch)
----------------
This becomes 0 or 1 instead of True or False, not sure if that will cause issues later.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D47819/new/

https://reviews.llvm.org/D47819





More information about the llvm-commits mailing list