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

[David A. Greene]

Tobias Grosser <grosser at fim.uni-passau.de> writes:

> 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.

Except testing was broken for over a year for me and we could never
figure out why.  I really, really don't trust magic paths.

>> 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.

Why not?  It's a tool name, right?

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

Yes, that's a risk.

> 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.

I'm a little hesitant to do this fix two different ways.  I'm not the
owner of lit though, so go ahead and propose/commit it if you want.

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

I think I know what went wrong.  Just have to go fix it now.  :)

                            -Dave