[Lldb-commits] [PATCH] D81696: [lldb/Test] Fix ASan/TSan workaround for Xcode Python 3

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 6 inline comments as done.
JDevlieghere added inline comments.


================
Comment at: lldb/test/API/lit.cfg.py:56
+# copy of the "real" python to work with.
+def find_python_interpreter():
+  # Avoid doing any work if we already copied the binary. This serves as
----------------
vsk wrote:
> This isn't relevant for the unit or shell tests because the API tests are the only ones that are executed within a python process, right?
Correct.


================
Comment at: lldb/test/API/lit.cfg.py:58
+  # Avoid doing any work if we already copied the binary. This serves as
+  # synchronization between multiple API tests.
+  copied_python = os.path.join(config.lldb_build_directory, 'copied-python')
----------------
vsk wrote:
> Is the part about synchronization still applicable? Maybe it is, if you run llvm-lit test/API from two different directories?
I guess it is, but not something I'd optimize for. I'll remove the comment. 


================
Comment at: lldb/test/API/lit.cfg.py:102
+if 'DYLD_INSERT_LIBRARIES' in config.environment and platform.system() == 'Darwin':
+  config.python_executable = find_python_interpreter()
+
----------------
vsk wrote:
> What does setting python_executable do? I looked this up in llvm and found:
> 
> ```
> test/lit.cfg.py:    ('%llvm-locstats', "'%s' %s" % (config.python_executable, llvm_locstats_tool)))
> test/lit.site.cfg.py.in:config.python_executable = "@PYTHON_EXECUTABLE@"
> test/tools/UpdateTestChecks/lit.local.cfg:            config.python_executable, script_path, extra_args)))
> ```
> 
> Am I missing something, or does something here translate into a lit directive to run python files under python_interpreter?
`test/lit.site.cfg.py.in` will be configured with the path to the real python executable in the build dir.


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

https://reviews.llvm.org/D81696