[Lldb-commits] [PATCH] D66327: [dotest] Make it possible to forward CCC_OVERRIDE_OPTIONS to Make

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Aug 16 01:16:04 PDT 2019


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





More information about the lldb-commits mailing list