[PATCH] D46020: [lit, lldbsuite] Update the lldbsuite to correctly run tests on windows and windows server
Zachary Turner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 24 09:46:19 PDT 2018
zturner accepted this revision.
zturner added inline comments.
================
Comment at: lit/Suite/lldbtest.py:47
+ # of the command.
+ cmd = [sys.executable] + self.dotest_cmd + [testPath, '-p', testFile]
----------------
stella.stamenova wrote:
> zturner wrote:
> > labath wrote:
> > > Do we need to worry about picking the correct `python`/`python_d` for release vs. debug builds?
> > If it's a debug build, `sys.executable` should already be `python_d.exe`. So this should be correct.
> >
> > My question though, is where does `self.dotest_cmd` come from? Would it ever be the case that this already has the python executable? Then you'd have an error because it would be specified twice.
> Currently, dotest_cmd is hardcoded in the lldb suite lit.cfg to point to dotest_path (+some arguments) and dotest_path itself is hardcoded in lldb-dotest.in to point to the dotest.py script.
>
> I considered testing whether the command contained python before adding it, but since the command is currently hardcoded to point to dotest.py, it seemed wasteful.
This seems fine to me then. I have a mild preference for prepending `sys.executable` from inside of `lit.cfg` and then putting an assertion here that `sys.executable` is present as the first argument, but it's up to you, if you like that too you can do it, otherwise this is fine as is IMO.
Repository:
rL LLVM
https://reviews.llvm.org/D46020
More information about the llvm-commits
mailing list