[dotest] Simplify logic to find the Python path

[JDevlieghere]

Summary

Simplify the logic of parsing the lldb -P output to find the python path. This removes the special handling for the LLDB.framework and instead of pattern matching known errors focus on finding a directory path that contains an __init__.py.

[rupprecht]

lldb/packages/Python/lldbsuite/test/dotest.py

550–551

Is DEVNULL necessary? I think stderr=None might work just as well.

FWIW, subprocess.DEVNULL exists as of python 3.3, but I guess we still have python2 users.

[JDevlieghere]

lldb/packages/Python/lldbsuite/test/dotest.py

550–551

Neat, None is the default argument so I'll just omit it.

[kastiglione]

About -P, the man page for lldb and the driver's Options.td say it:

Prints out the path to the lldb.py file for this version of lldb.

Should it do just that? If so this can be simplified further.

[rupprecht]

In D88840#2313331, @kastiglione wrote:

About -P, the man page for lldb and the driver's Options.td say it:

Prints out the path to the lldb.py file for this version of lldb.

Should it do just that? If so this can be simplified further.

It can print the path, or it can print <COULD NOT FIND PATH> if e.g. lldb wasn't built with python support or has some non-standard python setup. So I think this is basically as simple as the parsing can get.

[labath]

In D88840#2314587, @rupprecht wrote:

In D88840#2313331, @kastiglione wrote:

About -P, the man page for lldb and the driver's Options.td say it:

Prints out the path to the lldb.py file for this version of lldb.

Should it do just that? If so this can be simplified further.

It can print the path, or it can print <COULD NOT FIND PATH> if e.g. lldb wasn't built with python support or has some non-standard python setup. So I think this is basically as simple as the parsing can get.

One way this could be simplified further is to ditch -P and pass down the appropriate value from cmake/lit.

[JDevlieghere]

In D88840#2314594, @labath wrote:

In D88840#2314587, @rupprecht wrote:

In D88840#2313331, @kastiglione wrote:

About -P, the man page for lldb and the driver's Options.td say it:

Prints out the path to the lldb.py file for this version of lldb.

Should it do just that? If so this can be simplified further.

It can print the path, or it can print <COULD NOT FIND PATH> if e.g. lldb wasn't built with python support or has some non-standard python setup. So I think this is basically as simple as the parsing can get.

One way this could be simplified further is to ditch -P and pass down the appropriate value from cmake/lit.

LLDB in Xcode supports both Python 2 and 3 and we use a default com.apple.dt.lldb DefaultPythonVersion to override the version. By using -P here we can run the test suite with both versions by just changing the value of the default without having to reconfigure CMake. I'm not saying we should not do this because our donwstream fork, just that it would complicate things for us.

[labath]

In D88840#2314860, @JDevlieghere wrote:

In D88840#2314594, @labath wrote:

One way this could be simplified further is to ditch -P and pass down the appropriate value from cmake/lit.

LLDB in Xcode supports both Python 2 and 3 and we use a default com.apple.dt.lldb DefaultPythonVersion to override the version. By using -P here we can run the test suite with both versions by just changing the value of the default without having to reconfigure CMake. I'm not saying we should not do this because our donwstream fork, just that it would complicate things for us.

Yea, I'm not volunteering to do that (though I might one day), but... doesn't the python version that lldb uses and the python version running dotest needs to match? I'd expect that you'd still need to change /something/ so that it runs the different version of the python interpreter...

[JDevlieghere]

In D88840#2314883, @labath wrote:

In D88840#2314860, @JDevlieghere wrote:

In D88840#2314594, @labath wrote:

One way this could be simplified further is to ditch -P and pass down the appropriate value from cmake/lit.

LLDB in Xcode supports both Python 2 and 3 and we use a default com.apple.dt.lldb DefaultPythonVersion to override the version. By using -P here we can run the test suite with both versions by just changing the value of the default without having to reconfigure CMake. I'm not saying we should not do this because our donwstream fork, just that it would complicate things for us.

Yea, I'm not volunteering to do that (though I might one day), but... doesn't the python version that lldb uses and the python version running dotest needs to match? I'd expect that you'd still need to change /something/ so that it runs the different version of the python interpreter...

Yep, we have a little snippet that checks the default in lit.cfg.py that swaps out the configured Python interpreter with the system one matching the default when set, similar to our little hack for running Python under ASan: https://github.com/llvm/llvm-project/blob/a3adcba645eec31b42ad0a1f727975c5c9c236f0/lldb/test/API/lit.cfg.py#L117

lldb/packages/Python/lldbsuite/test/dotest.py

if is_exe(lldbVSCodeExec):
         os.environ["LLDBVSCODE_EXEC"] = lldbVSCodeExec
     else:
         if not configuration.shouldSkipBecauseOfCategories(["lldb-vscode"]):
             print(
                 "The 'lldb-vscode' executable cannot be located.  The lldb-vscode tests can not be run as a result.")
             configuration.skip_categories.append("lldb-vscode")
 
     lldbPythonDir = None  # The directory that contains 'lldb/__init__.py'
-    if configuration.lldb_framework_path:
-        lldbtest_config.lldb_framework_path = configuration.lldb_framework_path
-        candidatePath = os.path.join(
-            configuration.lldb_framework_path, 'Resources', 'Python')
-        if os.path.isfile(os.path.join(candidatePath, 'lldb/__init__.py')):
-            lldbPythonDir = candidatePath
-        if not lldbPythonDir:
-            print(
-                'Resources/Python/lldb/__init__.py was not found in ' +
-                configuration.lldb_framework_path)
-            sys.exit(-1)
-    else:
-        # If our lldb supports the -P option, use it to find the python path:
-        init_in_python_dir = os.path.join('lldb', '__init__.py')
-
-        lldb_dash_p_result = subprocess.check_output(
-            [lldbtest_config.lldbExec, "-P"], stderr=subprocess.STDOUT, universal_newlines=True)
 
-        if lldb_dash_p_result and not lldb_dash_p_result.startswith(
-                ("<", "lldb: invalid option:")) and not lldb_dash_p_result.startswith("Traceback"):
+    # If our lldb supports the -P option, use it to find the python path:
+    with open(os.devnull, 'w') as DEVNULL:
+        lldb_dash_p_result = subprocess.check_output([lldbtest_config.lldbExec, "-P"], stderr=DEVNULL, universal_newlines=True)
+        if lldb_dash_p_result and not lldb_dash_p_result.startswith(("<", "lldb: invalid option:")):
             lines = lldb_dash_p_result.splitlines()
-
-            # Workaround for readline vs libedit issue on FreeBSD.  If stdout
-            # is not a terminal Python executes
-            #     rl_variable_bind ("enable-meta-key", "off");
-            # This produces a warning with FreeBSD's libedit because the
-            # enable-meta-key variable is unknown.  Not an issue on Apple
-            # because cpython commit f0ab6f9f0603 added a #ifndef __APPLE__
-            # around the call.  See http://bugs.python.org/issue19884 for more
-            # information.  For now we just discard the warning output.
-            if len(lines) >= 1 and lines[0].startswith(
-                    "bind: Invalid command"):
-                lines.pop(0)
-
-            # Taking the last line because lldb outputs
-            # 'Cannot read termcap database;\nusing dumb terminal settings.\n'
-            # before the path
-            if len(lines) >= 1 and os.path.isfile(
-                    os.path.join(lines[-1], init_in_python_dir)):
-                lldbPythonDir = lines[-1]
-                if "freebsd" in sys.platform or "linux" in sys.platform:
-                    os.environ['LLDB_LIB_DIR'] = os.path.join(
-                        lldbPythonDir, '..', '..')
+            for line in lines:
+                if os.path.isdir(line) and os.path.exists(os.path.join(line, 'lldb', '__init__.py')):
+                    lldbPythonDir = line
+                    break
 
-        if not lldbPythonDir:
-            print(
-                "Unable to load lldb extension module.  Possible reasons for this include:")
-            print("  1) LLDB was built with LLDB_ENABLE_PYTHON=0")
-            print(
-                "  2) PYTHONPATH and PYTHONHOME are not set correctly.  PYTHONHOME should refer to")
-            print(
-                "     the version of Python that LLDB built and linked against, and PYTHONPATH")
-            print(
-                "     should contain the Lib directory for the same python distro, as well as the")
-            print("     location of LLDB\'s site-packages folder.")
-            print(
-                "  3) A different version of Python than that which was built against is exported in")
-            print("     the system\'s PATH environment variable, causing conflicts.")
-            print(
-                "  4) The executable '%s' could not be found.  Please check " %
-                lldbtest_config.lldbExec)
-            print("     that it exists and is executable.")
 
     if lldbPythonDir:
         lldbPythonDir = os.path.normpath(lldbPythonDir)
         # Some of the code that uses this path assumes it hasn't resolved the Versions... link.
         # If the path we've constructed looks like that, then we'll strip out
         # the Versions/A part.
         (before, frameWithVersion, after) = lldbPythonDir.rpartition(
             "LLDB.framework/Versions/A")
         if frameWithVersion != "":
             lldbPythonDir = before + "LLDB.framework" + after
 
         lldbPythonDir = os.path.abspath(lldbPythonDir)
 
+        if "freebsd" in sys.platform or "linux" in sys.platform:
+            os.environ['LLDB_LIB_DIR'] = os.path.join(lldbPythonDir, '..', '..')
+
         # If tests need to find LLDB_FRAMEWORK, now they can do it
         os.environ["LLDB_FRAMEWORK"] = os.path.dirname(
             os.path.dirname(lldbPythonDir))
 
         # This is to locate the lldb.py module.  Insert it right after
         # sys.path[0].
         sys.path[1:1] = [lldbPythonDir]