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

Resolves junction links when deleting packages installed as editable #10697

Merged
merged 5 commits into from
Feb 19, 2022

Conversation

jiunbae
Copy link
Contributor

@jiunbae jiunbae commented Dec 1, 2021

Resolve #10696

Or would it be better to use pathlib.Path?

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or would it be better to use pathlib.Path?

You can if you prefer, but this is also fine. I think we want to check both the original recorded path and the real path though (and pass if either of them match).

news/10696.bugfix.rst Outdated Show resolved Hide resolved
Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>
@jiunbae
Copy link
Contributor Author

jiunbae commented Dec 1, 2021

I don't know perfectly how pypi pakcages are stored and managed, so I thought it would be sufficient to make sure that the absolute path of the package path and the absolute path of the saved path are the same when deleting.

@uranusjr
Copy link
Member

uranusjr commented Dec 1, 2021

Let’s call realpath on both sides them? (Or use os.path.samefile?)

@jiunbae
Copy link
Contributor Author

jiunbae commented Dec 1, 2021

Does this look good to you?

assert os.path.samefile(link_pointer, os.path.realpath(dist_location)), (
    f"Egg-link {link_pointer} does not match installed location of "
    f"{dist.raw_name} (at {dist_location})"
)

or re-define dist_location as realpath?

@uranusjr
Copy link
Member

uranusjr commented Dec 1, 2021

I think samfile actually calls realpath underneath so we don’t even need that.

Edit: Oh, not actually, it’s using stat instead. But I think the Windows API should resolve junctions for stat. Can you try removing realpath and relying on only samefile and see if that works?

@jiunbae
Copy link
Contributor Author

jiunbae commented Dec 1, 2021

I checked that it works well in the environment presented in the #10696 . The os.path.samefile seems to work as expected in terms of operation.

@jiunbae jiunbae changed the title Resolves junction links when deleting packages installed as editable. Resolves junction links when deleting packages installed as editable Dec 1, 2021
@sio
Copy link

sio commented Feb 19, 2022

Using os.path.samefile should fix a similar failure I've encountered due to shortened Windo~1 paths:

AssertionError: Egg-link c:\users\runner~1\appdata\local\temp\makefile.venv_test_7o0hpfn3 
does not match installed location of hello 
(at c:\users\runneradmin\appdata\local\temp\makefile.venv_test_7o0hpfn3)

Is there anything that blocks this PR from being merged? @uranusjr?

@pradyunsg pradyunsg merged commit d35bedc into pypa:main Feb 19, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to delete packages installed as editable from Windows junction link
5 participants