labath added a comment.

How do you envision this to be used? Via something like 
`CCC_OVERRIDE_OPTIONS=foo ninja check-lldb` ?

Overall, I think it would be nice to avoid the tests being affected by the 
inherited environment variables because that opens the door for tests to 
unexpectedly fail or simply behave differently just because someone happens to 
have some weird environment variable set (and he may not even know about it). I 
think this is also consistent with the aproach taken in llvm (see e.g. 
`possibly_dangerous_env_vars` in `llvm/utils/lit/lit/llvm/config.py`). Overall, 
being able to run the (dotest) test suite with different options would be nice, 
but I think it would be better if there was a more explicit way to do that 
(e.g., a command line argument). Incidentally, `dotest` already has a command 
line argument (`--env`), which seems to be exactly intended for its purpose. 
Couldn't you just use that?

(Also, instead of CCC_OVERRIDE_OPTIONS, I think I'd try to use CFLAGS_EXTRAS, 
because that explicitly handled by makefiles, and so it gives an easier way for 
tests to opt-out of this when they want very specific flags for something)



================
Comment at: lldb/packages/Python/lldbsuite/test/plugins/builder_base.py:39
 
+def getEnv():
+    """Returns the build environment."""
----------------
I am confused. How is this function used and what's its relation the the second 
change below?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D66327



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to