This is an archive of the discontinued LLVM Phabricator instance.

Fix lookup path for lldb-mi
ClosedPublic

Authored by beanz on Oct 11 2016, 2:07 PM.

Details

Summary

The test suite calls realpath on the lldb executable then append "-mi" to it to find the path of the lldb-mi executable. This does not work when using CMake builds on *nix platforms. On *nix platforms when a version number is set on executables CMake generates the binary as ${name}-${version} with a symlink named ${name} pointing to it.

This results in the lldb executable being named lldb-4.0.0, and since lldb-4.0.0-mi doesn't ever match the lldb-mi executable these tests are always disabled.

This patch looks for lldb-mi in the same directory as lldb.

Diff Detail

Repository
rL LLVM

Event Timeline

beanz updated this revision to Diff 74292.Oct 11 2016, 2:07 PM
beanz retitled this revision from to Fix lookup path for lldb-mi.
beanz updated this object.
beanz added reviewers: tfiala, zturner.
beanz added a subscriber: lldb-commits.
zturner accepted this revision.Oct 11 2016, 3:53 PM
zturner edited edge metadata.

Heh. It just occurred to me that this is probably the reason why all of the lldb-mi tests fail on Windows. Because .exe is not appended. I'm not ready to turn that on and deal with all the failures just yet, but for now can you please add a #TODO right above this that says "Append .exe on Windows"

This revision is now accepted and ready to land.Oct 11 2016, 3:53 PM
tfiala accepted this revision.Oct 11 2016, 5:59 PM
tfiala edited edge metadata.

Looks good!

Heh. It just occurred to me that this is probably the reason why all of the lldb-mi tests fail on Windows. Because .exe is not appended. I'm not ready to turn that on and deal with all the failures just yet, but for now can you please add a #TODO right above this that says "Append .exe on Windows"

Yep, I had to fix this locally just the other day, but all the tests still fail :)

ki.stfu added inline comments.
packages/Python/lldbsuite/test/dotest.py
676–677 ↗(On Diff #74292)

Maybe it would be better to replace "lldb" with "lldb-mi" in lldbtest_config.lldbExec? So we will take into account their versions instead of ignoring them.

Closed by commit rL284041: Fix lookup path for lldb-mi (authored by cbieneman). · Explain WhyOct 12 2016, 1:25 PM
This revision was automatically updated to reflect the committed changes.
beanz added inline comments.Oct 12 2016, 1:53 PM
packages/Python/lldbsuite/test/dotest.py
676–677 ↗(On Diff #74292)

That is a good idea, assuming the patch as-is doesn't break the world, I'll revise it to that to fix the Windows issue as well.