Switch to using lstat() instead of stat() to not match symlink targets #13161
Conversation
|
See #13156 (comment) - I think the existing code should work without this change, and we need to understand why it doesn't before trying to "fix" the issue. |
roubert
force-pushed
the
running_under_virtualenv
branch
from
ed0b04b to
f6e5730
Compare
|
I'll leave this for someone familiar with Unix to comment on whether it's a good fix. It seems reasonable to me. |
roubert
changed the title
roubert
force-pushed
the
running_under_virtualenv
branch
from
f6e5730 to
211596f
Compare
roubert
force-pushed
the
running_under_virtualenv
branch
2 times, most recently
from
5e3f8ee to
d989254
Compare
There was a problem hiding this comment.
Looks reasonable and does the right thing locally. My best guess is that this worked in some limited situations because the virtual environment was created using copies instead of symlinks. This seems rare in practice as both venv and virtualenv have defaulted to using symlinks on Linux/macOS for ages, but I dunno what else could make this work.
The get_best_invocation_for_this_pip() logic has the same flaw, although I doubt anyone uses symlinks for the pip binaries in practice (at least on Unix, the scripts' shebang includes a reference to the virtual environment's Python, so they can't be symlinks. Not sure how Windows works...).
roubert
force-pushed
the
running_under_virtualenv
branch
4 times, most recently
from
625403a to
0e47a0f
Compare
The convenience function samefile() calls stat() and samestat(), but this leads to treating a symlink and its target as the same which is unlikely to be intentional here as that doesn't work well with venv.
roubert
force-pushed
the
running_under_virtualenv
branch
from
9348ec9 to
127134a
Compare
Closes #13156.