Skip to content
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

Question about pyenv #1062

Closed
samas69420 opened this issue Jan 28, 2024 · 6 comments
Closed

Question about pyenv #1062

samas69420 opened this issue Jan 28, 2024 · 6 comments

Comments

@samas69420
Copy link
Contributor

After my last pull request (#1058) I've noticed that the "is_dir()"assertion still fails when using "system" python version in pyenv and when virtual environments managed with pyenv-virtualenv are active, I did fix it but I'm not sure if I should open a new pull request since multiple site-packages directories from different python versions would be allowed to be simultaneously in sys.path and I don't know if this could lead to conflicts, maybe it would be better to just force users to set the global python version to "system" from pyenv and deactivate any virtual environment before running gdb if pyenv is detected

@hugsy
Copy link
Owner

hugsy commented Jan 29, 2024

pyenv is the source of too many unwanted issue and I don't think it's gef's job to check if it's running in a venv or not. In addition, none of the pyenv checks are tested through our CI so it's a bit uncertain to know when bugs hit.

I don't know if this could lead to conflicts, maybe it would be better to just force users to set the global python version to "system" from pyenv and deactivate any virtual environment before running gdb if pyenv is detected

Exactly. As I mentioned in #1048 the correct approach IMO is simply to remove all code related to pyenv detection in gef itself, and instead we can have a standalone script (in scripts/ for instance) that will be here exactly for that purpose; script that must be loaded before gef in the gdbinit. We can easily do that and also provide documentation to assist on setting this up for users.

@samas69420 If you're willing to do this, such PR would be most welcome - simply because I don't think many of the other devs (include myself) use pyenv for gdb.

@samas69420
Copy link
Contributor Author

what should the script do besides detecting pyenv?

@hugsy
Copy link
Owner

hugsy commented Jan 29, 2024

what should the script do besides detecting pyenv?

Exactly that, but in a separate script:

gef/gef.py

Lines 11396 to 11415 in ece5728

try:
pyenv = which("pyenv")
pyenv_root = gef_pystring(subprocess.check_output([pyenv, "root"]).strip())
pyenv_version = gef_pystring(subprocess.check_output([pyenv, "version-name"]).strip())
site_packages_dir = pathlib.Path(pyenv_root) / f"versions/{pyenv_version}/lib/python{pathlib.Path(pyenv_version).stem}/site-packages"
assert site_packages_dir.is_dir()
site.addsitedir(str(site_packages_dir.absolute()))
except FileNotFoundError:
pass
# When using a Python virtual environment, GDB still loads the system-installed Python
# so GEF doesn't load site-packages dir from environment
# In order to fix it, from the shell with venv activated we run the python binary,
# take and parse its path, add the path to the current python process using sys.path.extend
PYTHONBIN = which("python3")
PREFIX = gef_pystring(subprocess.check_output([PYTHONBIN, '-c', 'import os, sys;print((sys.prefix))'])).strip("\\n")
if PREFIX != sys.base_prefix:
SITE_PACKAGES_DIRS = subprocess.check_output(
[PYTHONBIN, "-c", "import os, sys;print(os.linesep.join(sys.path).strip())"]).decode("utf-8").split()
sys.path.extend(SITE_PACKAGES_DIRS)

@hugsy
Copy link
Owner

hugsy commented Jan 29, 2024

And maybe leave an additional global variable or environment variable like RUNNING_IN_PYENV or something in case that's important to detect in the future (I doubt it, but it won't hurt).

@samas69420
Copy link
Contributor Author

i can try, but anyway i already made a little edit on the code (still in gef.py tho), now users are forced to explicitly select "system" python version if they're using pyenv and if they don't do it they get a error message, i also moved the gef declaration above so i could use the err function to print the message, if it's ok i can open a PR
https://github.com/samas69420/gef/blob/a1d1a62aa9826526fc985cf8d534beb563be9158/gef.py#L11383-L11444

@hugsy
Copy link
Owner

hugsy commented May 19, 2024

pyenv support has been support with #1084

Closing this issue

@hugsy hugsy closed this as completed May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants