[llvm-commits] [llvm] r122341 - in /llvm/trunk: docs/TestingGuide.html test/lit.cfg utils/lit/lit/TestRunner.py

[Tobias Grosser]

On 12/21/2010 12:37 PM, David A. Greene wrote:
> Tobias Grosser<grosser at fim.uni-passau.de>  writes:
>
>> I just read your replies and have still an open question. You replied
>> the patch provided by me, that prepends the llvm tools directory to the
>> path will not solve the problem as it is too brittle. Can you explain this.
>
> Because you never really know what else lit might prepend to the PATH,
> as the PR demonstrates.
This is something we can control and nothing I expect to happen very 
often. Lit is prepending exactly three items.

 > It's just more solid to say what you really
> mean.
Sure. However replacing strings like " opt " is nothing that I would 
expect while writing test files. So this step is not 100% solid.

However, as it would probably fail in an obvious way, there is no need 
to worry about this. Especially as you documented the change.

Defining the correct path however is also saying what we really mean, 
and we can rely on the parsing of the shell to prepend this path to the 
binaries, but not to arbitrary strings somewhere in the command line. So 
in case of test case writing this is probably more solid. We just need 
to make sure lit does not use a wrong path.


 > As a bonus, when things fail, the run script output by lit will
> include the full paths so you know exactly what ran.
This is a valid point.

I am just worried we gonna forget to add tools to this explicit list.

Do you think we can in addition fix the PATH order as proposed in my 
patch? In case we forget to add some tools, it would still do the right 
thing. We just do not get the convenient absolute path and are doomed if 
someone prepends another path. I believe this will not happen too often 
and I think this would be a bug in lit. We can e.g. add a test case, 
that tests that the first component of the path is the tools directory.

Furthermore it will fix the problem for the moment, without failing any 
builds. So you have some time to figure out what went wrong.

Cheers
Tobi