Page MenuHomePhabricator

[lldb] [test] Pass LLVM_LIBS_DIR from CMake for linking liblldb
ClosedPublic

Authored by mgorny on Fri, Jan 31, 12:41 AM.

Details

Summary

Pass the correct library directory from CMake to dotest.py when linking
liblldb, instead of trying to reconstruct the path from executable path.
This fixes link failures on platforms having non-null
LLVM_LIBDIR_SUFFIX.

Diff Detail

Event Timeline

mgorny created this revision.Fri, Jan 31, 12:41 AM

If we go down this path, then I think we should just pass the full path to the lib directory, and remove all the code which tries to autodetect it.

But if you're looking for a quick fix, I think adding "netbsd" to the list of operating systems at dotest.py:625 should fix this problem.

I'm actually trying to make 10.0.0 a little less broken for Gentoo.

Aha, I see. Could you then simply pass LLDB_LIBS_DIR instead of just the suffix?

mgorny updated this revision to Diff 241672.Fri, Jan 31, 3:33 AM
mgorny retitled this revision from [lldb] [test] Respect LLVM_LIBDIR_SUFFIX when linking liblldb to [lldb] [test] Pass LLVM_LIBS_DIR from CMake for linking liblldb.
mgorny edited the summary of this revision. (Show Details)

The question is, does this break any of the platform hacks? ;-)

The question is, does this break any of the platform hacks? ;-)

I think it /may/ break one. See inline comment.

BTW, I think the best way to clean a lot of this up would be to take the tests which build executables linking to liblldb, and rewrite them as c++ unit tests (at least those that aren't skipped/xfailed everywhere), as cmake knows best how to link to the thing it has built. Then, a lot of this stuff can be just deleted...

lldb/packages/Python/lldbsuite/test/lldbtest.py
1680

I'm not sure if this is right, because (despite the name) LLDB_LIB_DIR points to the "bin" dir. But then again, this whole function seems pretty wrong, so it's hard to say what should it really do here.

If you don't need this part (the previous patch didn't have it) and want to make this safe-ish for cherry-picking, maybe you could leave this part out for now.

mgorny marked an inline comment as done.Fri, Jan 31, 7:04 AM
mgorny added inline comments.
lldb/packages/Python/lldbsuite/test/lldbtest.py
1680

Ok, I'll do a second run and see if the result changes without it.

mgorny updated this revision to Diff 241728.Fri, Jan 31, 7:20 AM

Removed the aforementioned chunk.

labath accepted this revision.Wed, Feb 5, 8:51 AM
This revision is now accepted and ready to land.Wed, Feb 5, 8:51 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptWed, Feb 5, 9:37 AM