-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Handle symlinks in req_uninstall.py #5442
Conversation
875937a
to
ac20718
Compare
ac20718
to
df48b85
Compare
@@ -359,7 +359,7 @@ def from_dist(cls, dist): | |||
# develop egg | |||
with open(develop_egg_link, 'r') as fh: | |||
link_pointer = os.path.normcase(fh.readline().strip()) | |||
assert (link_pointer == dist.location), ( | |||
assert (os.stat(link_pointer) == os.stat(dist.location)), ( |
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.
Should use os.path.samefile
, as this is defined to do precisely this job. You may need to fall back to something else on Python 2.7, as samefile only added Windows support in Python 3.2, but I'm not sure if a stat comparison is sufficient on before Python 3.4 on Windows (the docs to samefile say "Changed in version 3.4: Windows now uses the same implementation as all other platforms" which implies that prior to that, Windows needed special treatment).
OTOH, this is "only" an assertion, not a normal error - so maybe it could be removed if it's asserting something that isn't always true? What do the other @pypa/pip-committers think?
ed00a03
to
3e948a3
Compare
Handle symlinks in req_uninstall.py
3e948a3
to
57a0231
Compare
@pfmoore Updated. |
Review comments addressed. I'm not going so far as to "approve" this change, though. There's still the question of do we need the assertion anyway. Unfortunately, github doesn't offer a plain "Confirm review comments have been addressed without approving" button...
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes #5439.