Repository navigation
Fix detection of bundled python in PythonScriptingProvider - #8542
Conversation
fuzyll
left a comment
There was a problem hiding this comment.
Approved, thanks! I think this could potentially be further improved (and I have an idea in a code comment), but I see no reason this should be held up any longer than it already has been.
| if not python_lib and not use_bundled_python: | ||
| if not sys.executable: | ||
| return (None, "Failed to find the python binary for the running interpreter") | ||
| return (sys.executable, "Success") |
There was a problem hiding this comment.
I debated requesting changes here because it feels like we should be doing more checks here. Does sys.executable exist? Is it executable? Is it an accepted version? Does it respond to -c or -m pip or other switches that we use?
Some of these are probably redundant. (I'm not sure how you would get here with a sys.executable that doesn't exist or isn't executable.) Others are probably only relevant if you're in an embedded Python from another application, which is not typically how people use us.
When I started trying to sketch out what this would look like, I realized ensuring some of these things would be pretty complicated. And, I would hope that setting this to something that doesn't work would fail fairly quickly for the user such that they would understand what the likely source of the failures is.
If you want to do any of that here, I still think it would be a good idea. But, I don't think holding this up over it is a good move.
293d093 to
1914232
Compare
|
For extra context, since I had forgotten by the time I came back from PTO: This was to address an issue from an Enterprise customer that was trying to configure Binary Ninja in a Docker container. When running headless, they did not have |
No description provided.