-
-
Notifications
You must be signed in to change notification settings - Fork 152
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
refactor the non_host part for not injecting to custom env #346
Conversation
I think the result of this method is the same with the original one. Please take a look if you have time. @kemzeb |
da1a410
to
f6b5164
Compare
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.
Hmm, I noticed that when we're using a venv that has system site packages enabled (i.e. I did python -m venv --system-site-packages venv
), running the following:
pipdeptree --python path/to/python --local-only
...would cause us to hit the else branch (since I am not running in a virtual environment). We don't want this since we will use paths to metadata from the entire environment.
I think what we need to do here is if --python and --local are given, we should grab the result of:
"str(py_path), "-c", "import sys, site; print(site.getsitepackages([sys.prefix]))"
...and pass it to distributions()
. Otherwise, use sys.path
.
f6b5164
to
a73123a
Compare
That's a really good situation which I didn't notice. I will add some tests to cover this too. |
acbf7af
to
8c54c10
Compare
a1b013d
to
76ff513
Compare
FYI once we merge this I'll make a 2.18.0 release. |
d11e5e2
to
6432873
Compare
6432873
to
b5ba32f
Compare
62c12fe
to
49e95f2
Compare
49e95f2
to
60bc27c
Compare
Can't see any other issues, so it looks like this is good to go. Will release 2.18.0 after merging |
615cfc2
to
18f832e
Compare
Looks like we're good to go, releasing 2.18.0 |
fix: #343
We can get the sitepackages directory from the given interpreter directly instead of running our code again with it.