Add support for virtualenv #1119
Conversation
Add a `gef.virtualenv_path` config var, and activate it at startup.
There was a problem hiding this comment.
TL;DR: I think this needs to be re-worked at least by adding way more test cases and documentation.
As I mentioned, a lot of complications come when dealing with seperate environments, and so it should be tested way more, for instance:
- we need to check the venv is running at least for the minimum Python version we supported
- what happens when a user uses a python version different from that with gdb
- what happens when a user switches between venv
- etc.
In addition, this should be exhaustively documented - not just the gef config option, but how to use gef in a virtual env because it is 100% certain users will come with such questions (and/or bad issue reports about it)
The pyenv was done hastily which came to bite us down the line, let's not repeat that mistake
| if path: | ||
| activate_script_path = pathlib.Path(path)/"bin"/"activate_this.py" | ||
| exec(open(activate_script_path).read(), {'__file__': activate_script_path}) |
There was a problem hiding this comment.
Avoid cascading by returning first on error
| if path: | |
| activate_script_path = pathlib.Path(path)/"bin"/"activate_this.py" | |
| exec(open(activate_script_path).read(), {'__file__': activate_script_path}) | |
| if not path: | |
| return | |
| activate_script_path = pathlib.Path(path)/"bin"/"activate_this.py" | |
| exec(open(activate_script_path).read(), {'__file__': activate_script_path}) |
| if path: | ||
| activate_script_path = pathlib.Path(path)/"bin"/"activate_this.py" | ||
| exec(open(activate_script_path).read(), {'__file__': activate_script_path}) |
There was a problem hiding this comment.
Also simpler
exec(activate_script_path.read_text(), {'__file__': activate_script_path})| def load_virtualenv(self, new_path: Optional[pathlib.Path] = None): | ||
| path = new_path or gef.config["gef.virtualenv_path"] | ||
| if path: | ||
| activate_script_path = pathlib.Path(path)/"bin"/"activate_this.py" |
There was a problem hiding this comment.
Here path is already known to be of type pathlib.Path, no neeed for a 2nd allocation,
activate_script_path = path / "bin/activate_this.py"
assert activate_script_path.is_file()
| @@ -9854,6 +9854,9 @@ def __init__(self) -> None: | |||
| plugins_dir = GefSetting("", str, "Autoload additional GEF commands from external directory", hooks={"on_write": [GefSetting.no_spaces, ]}) | |||
| plugins_dir.add_hook("on_changed", [lambda _, new_val: GefSetting.must_exist(new_val), lambda _, new_val: self.load_extra_plugins(new_val), ]) | |||
| gef.config["gef.extra_plugins_dir"] = plugins_dir | |||
| venv_path = GefSetting("", str, "Path to the virtualenv used by GEF", hooks={"on_write": [GefSetting.no_spaces, ]}) | |||
| venv_path.add_hook("on_changed", [lambda _, new_val: GefSetting.must_exist(new_val), lambda _, new_val: self.load_virtualenv(new_val), ]) | |||
There was a problem hiding this comment.
Not sure why we do it above, but you shuld be able to just add on_changed as an extra key in the hooks arg for the constructor.
| @@ -9943,6 +9946,13 @@ def load_plugins_from_directory(plugin_directory: pathlib.Path): | |||
| dbg(f"Loading extra plugins from directory={directory}") | |||
| return load_plugins_from_directory(directory) | |||
|
|
|||
|
|
|||
| def load_virtualenv(self, new_path: Optional[pathlib.Path] = None): | |||
There was a problem hiding this comment.
Not sure if this makes sense as a method here
| gdb.execute(f"gef config gef.virtualenv_path {self.venv_path}") | ||
|
|
||
| res = gdb.execute("pi __import__('numpy').test()", to_string=True) | ||
| assert 'NumPy version' in res |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. You can reopen it by adding a comment to this issue. |
|
Assigning to shush state-bot |
Add a
gef.virtualenv_pathconfig var, and activate the provided virtualenv at startup.This could be used by gef-extras to install python dependencies cleanly.