Skip to content

[lit] Detect lldb by checking both for lldb_build_root and that that root contains an lldb exec. #32643

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

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jul 1, 2020

Otherwise, if one builds lldb with a specific build-script config and does not
fully build lldb, one will not be blocked from testing. With this change, one
instead gets a warning that we found a configured lldb dir, but not lldb exec.

…root contains an lldb exec.

Otherwise, if one builds lldb with a specific build-script config and does not
fully build lldb, one will not be blocked from testing. With this change, one
instead gets a warning that we found a configured lldb dir, but not lldb exec.
@gottesmm gottesmm requested a review from JDevlieghere July 1, 2020 06:31
@gottesmm
Copy link
Contributor Author

gottesmm commented Jul 1, 2020

People were talking about hitting this when they were messing around with infer (which creates build dir). Seems like a good overall change than erroring.

@gottesmm
Copy link
Contributor Author

gottesmm commented Jul 1, 2020

@swift-ci test

@@ -100,6 +100,8 @@ def get_simulator_command(run_os, run_cpu, sdk_path):

def get_lldb_python_path(lldb_build_root):
lldb_path = os.path.join(lldb_build_root, 'bin', 'lldb')
if not os.access(lldb_path, os.F_OK):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this is preferable compared to something like os.path.exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I just did it this way out of habit.

Copy link
Contributor

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@gottesmm gottesmm merged commit 2e85347 into swiftlang:master Jul 1, 2020
@gottesmm gottesmm deleted the pr-aa6d8ea8a6a7c33cd050eaa2e542cef37f6470fa branch July 1, 2020 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants