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

Fix for 3141: symlink in consolescript path #3142

Closed
wants to merge 1 commit into from
Closed

Conversation

AvdN
Copy link

@AvdN AvdN commented Sep 26, 2015

This fixes issue #3141, where a uninstall error occurs
when upgrading/uninstalling and the path to the console
script contains symlinks. As the path is in installed-files.txt
as well and these paths are joined to the real path coming
from .egg_info, the uninstall will try to remove the
console script using two different paths and fail on the second
(non-real) path.

This fixes issue 3141, where a uninstall error occurs
when upgrading/uninstalling and the path to the console
script contains symlinks. As the path is in installed-files.txt
as well and these paths are joined to the real path coming
from .egg_info, the uninstall will try to remove the
console script using two different paths and fail on the second
(non-real) path.
@xavfernandez
Copy link
Member

Since we dont want to break it for #2552, I think a middle-ground solution could be:

         if config.has_section('console_scripts'):
             if dist_in_usersite(dist):
                 bin_dir = bin_user
             else:
                 bin_dir = bin_py
             bin_dir = normalize_path(bin_dir)
             for name, value in config.items('console_scripts'):         
                 paths_to_remove.add(os.path.join(bin_dir, name))

@xavfernandez
Copy link
Member

cc @qwcode since you reviewed and merged #2552
An alternate solution is mathieulongtin@4dd0ab2 which I like less.

@qwcode
Copy link
Contributor

qwcode commented Sep 29, 2015

I think I'd rather have this fix: https://github.com/qwcode/pip/commit/e2450f0e99d8d94e72ea746f34f0cf1e5c51b7dd

this places the fix closer to the cause of all this (right below it), so I think it's clearer.

also, the point is not to resolve the console scripts I think, but to detect when the set already contains a resolved version of them.

I think my comment in the diff is accurate enough on how this happens, but I'd like to make it clearer tomorrow and add a test, and then I could post a PR.

@xavfernandez
Copy link
Member

@qwcode Read and approved :)

@AvdN
Copy link
Author

AvdN commented Sep 30, 2015

@qwcode I tested your fix ( https://github.com/qwcode/pip/commit/e2450f0e99d8d94e72ea746f34f0cf1e5c51b7dd ) locally here and it solves the issue I am having as well.
Are you going to put that in as a PR? Should I close this PR? I am not partial to any solution, just would like to get rid of the uninstall failure messages ;-)

@qwcode
Copy link
Contributor

qwcode commented Sep 30, 2015

@AvdN yea, I'll be putting in a PR. hopefully today. if/when it's merged, I'd go around and cleanup other issues and PRs

@qwcode
Copy link
Contributor

qwcode commented Sep 30, 2015

btw, my PR will be different than what I posted above, since after adding tests, I realized it was flawed, but I'll explain more when I post

@qwcode
Copy link
Contributor

qwcode commented Oct 3, 2015

closing in lieu of #3154

@qwcode qwcode closed this Oct 3, 2015
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants