This is an archive of the discontinued LLVM Phabricator instance.

[lldb] [test] Avoid double negations in llgs/debugserver logic
ClosedPublic

Authored by mgorny on Nov 5 2020, 12:57 PM.

Details

Summary

Use positive logic (i.e. llgs_platform/debugserver_platform) for
indicating which platforms use the particular server variant.
Deduplicate the lists — it is rather expected that none of the platforms
using LLGS would use debugserver.

Diff Detail

Event Timeline

mgorny requested review of this revision.Nov 5 2020, 12:57 PM
mgorny created this revision.
mgorny added inline comments.
lldb/packages/Python/lldbsuite/test/dotest.py
953

@labath, can we assume that all future platforms will use LLGS?

labath accepted this revision.Nov 5 2020, 11:31 PM
labath added inline comments.
lldb/packages/Python/lldbsuite/test/dotest.py
950

This is not entirely equivalent to the previous version -- it won't match "platforms" like "freebsd4.7". Fortunately, we already have some canonicalization code in lldbplatformutil.getPlatform(), so if you switch to that, it should be fine.

953

I certainly hope so. I doubt anyone would port debugserver to another os, and I definitely would not want to carry another debugserver around.

This revision is now accepted and ready to land.Nov 5 2020, 11:31 PM
mgorny marked 2 inline comments as done.Nov 6 2020, 12:18 AM
mgorny added inline comments.
lldb/packages/Python/lldbsuite/test/dotest.py
950

Good catch! However, this doesn't seem to work:

Command Output (stderr):
--
Traceback (most recent call last):
  File "/home/mgorny/llvm-project/llvm/tools/lldb/test/API/dotest.py", line 7, in <module>
    lldbsuite.test.run_suite()
  File "/usr/home/mgorny/llvm-project/lldb/packages/Python/lldbsuite/test/dotest.py", line 855, in run_suite
    from lldbsuite.test import lldbplatformutil
  File "/usr/home/mgorny/llvm-project/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py", line 19, in <module>
    import lldb
ModuleNotFoundError: No module named 'lldb'

--

There is apparently some obscure problem behind the scenes and it seems above my pay grade. I'll just revert to the old code.

953

Ok, I will remove the comment about adding more platforms then.

mgorny updated this revision to Diff 303349.Nov 6 2020, 12:23 AM
mgorny marked 2 inline comments as done.
mgorny retitled this revision from [lldb] [test] Avoid double negatives in llgs/debugserver logic to [lldb] [test] Avoid double negations in llgs/debugserver logic.

Removed the part about adding new platforms without LLGS, and restored old conditions for matching platforms.

labath added inline comments.Nov 6 2020, 12:29 AM
lldb/packages/Python/lldbsuite/test/dotest.py
950

This is fiddly, because all of this happens very early, before everything is set up.
You've tried to import this package much too early. The paths necessary for "import lldb" to work are only set up on line 869 (setupSysPath()). And lldb.selected_platform is only set on line ~931. You need to put the import statement after those -- say line 941, to replace the manual triple chopping...

mgorny updated this revision to Diff 303374.Nov 6 2020, 1:54 AM

Used lldbplatformutil.getPlatform() as requested.

Herald added a project: Restricted Project. · View Herald TranscriptNov 6 2020, 5:34 AM