-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Python3] Use correct Python version for LLDB-related tests #33181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The linux-fatal-backtrace script needs to run with an LLDB module loaded into Python. This in turn requires that the test be run with the exact same Python version as was used by LLDB (not just the same Python module directory). This can get confused on systems with multiple versions of Python installed. This replaces `lldb-python-path` (the Python module directory path) with `lldb-python` (which is the correct Python version run with the LLDB path). This should ensure that this test is always run with the same Python version and module that LLDB used.
@swift-ci Please test Linux |
Since the only affected test runs only on Linux, let's fully test there first and make sure everything works before running a smoke test on macOS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this definitely will allow the tests to pass. However, we should also simultaneously look at migrating the lldb build to python3. Either way, we should do this to make the system less fragile.
@swift-ci Please test Linux |
Build failed |
Previous code required a decimal point before it would recognize a path component as a Python version identifier, so it would accept `python2.7` but not `python3`.
@swift-ci Please smoke test macOS |
@swift-ci Please smoke test Linux |
While this is fine as a ad-hoc solution, really what we want here is to figure out the Python LLDB was build against from the LLDB build because it's the one-and-only source of truth. I can envision many scenarios where this won't work, the simplest one where a user passes a -DPYTHON_HOME CMake flag to LLDB to a Python that's not in path. |
If the |
I disagree, I don't think that's our responsibility. Nothing prevents you from running Python from inside the embedded script interpreter, maybe we should modify the test to do that instead? FWIW this constraint is not limited to LLDB but applies to anyone that builds a Python module that itself links against Python. Also, the potential problem I described above is inherent to the unfortunate |
My preferred solution would be to move to @compnerd unified build! |
Implemented this in #33235 |
…g#33181) * Use the correct Python executable to match LLDB The linux-fatal-backtrace script needs to run with an LLDB module loaded into Python. This in turn requires that the test be run with the exact same Python version as was used by LLDB (not just the same Python module directory). This can get confused on systems with multiple versions of Python installed. This replaces `lldb-python-path` (the Python module directory path) with `lldb-python` (which is the correct Python version run with the LLDB path). This should ensure that this test is always run with the same Python version and module that LLDB used. * Use consistent braces for the lldb-python substitution * Use os.path utilities to dissect paths * Allow `python3` as a path identifier Previous code required a decimal point before it would recognize a path component as a Python version identifier, so it would accept `python2.7` but not `python3`.
LLDB builds a Python module in the form of a C shared library. This is used in some Swift tests.
Since Python module format changed between Python 2 and Python 3, these tests must be run with the same Python version that was used when LLDB built it.
Of course, in a system with multiple Python versions installed, this may or may not be the same version used by the rest of the Swift build.
This adds some logic to lit.cfg to figure out the version of Python used by LLDB and expose that as
%{lldb-python}
for use in tests that need to access the LLDB module.