[PATCH] D56712: compiler-rt/test: Add a couple of convenience features for Android.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc marked an inline comment as done.
pcc added inline comments.


================
Comment at: compiler-rt/test/lit.common.cfg:282
+    env['ANDROID_SERIAL'] = config.android_serial
+    config.environment['ANDROID_SERIAL'] = config.android_serial
+
----------------
eugenis wrote:
> Is this actually better than setting ANDROID_SERIAL in the environment?
> 
> In any case, it looks like this line makes tests ignore ANDROID_SERIAL, even if the cmake variable is not set. This would get in the way if one wants to switch devices (ex. same arch but different api levels).
> Is this actually better than setting ANDROID_SERIAL in the environment?

I guess it's a matter of opinion, but I personally find it more convenient if I can just do "ninja check-hwasan" without needing to set an environment variable.

> In any case, it looks like this line makes tests ignore ANDROID_SERIAL, even if the cmake variable is not set. This would get in the way if one wants to switch devices (ex. same arch but different api levels).

cmake expands unset variables to the empty string, and Python treats the empty string as false-ish, so this code won't get executed if the variable is not set.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56712