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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis accepted this revision.
eugenis added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: compiler-rt/test/lit.common.cfg:282
+    env['ANDROID_SERIAL'] = config.android_serial
+    config.environment['ANDROID_SERIAL'] = config.android_serial
+
----------------
pcc wrote:
> 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.
Ah, right. That's OK then.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56712