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

resolve_path differs on Windows depending on Python version #1813

Closed
daddycocoaman opened this issue Mar 13, 2021 · 4 comments · Fixed by #1825
Closed

resolve_path differs on Windows depending on Python version #1813

daddycocoaman opened this issue Mar 13, 2021 · 4 comments · Fixed by #1825
Assignees
Milestone

Comments

@daddycocoaman
Copy link

daddycocoaman commented Mar 13, 2021

Hi. There was an issue filed under Typer that gives a full explanation, but basically the use of os.path.realpath in the resolve_path logic for click.Path differs between Python 3.7 and 3.8 on Windows. Prior to 3.8, os.path.realpath did not resolve symlinks. Therefore Click users on Windows using Python 3.7 or lower are getting the wrong results for resolve_path.

More info: https://docs.python.org/3/library/os.path.html#os.path.realpath

@davidism
Copy link
Member

I'm not sure what you're proposing click should do here.

@daddycocoaman
Copy link
Author

Sorry...I probably should have specified the solution.

This resolve path logic needs to account for symbolic links, particularly for the Windows issue.

The way to do this is to check if the path is a link with os.path.islink, and if it is, then get the path with os.readlink. That seems like the best cross-platform solution, and I don't think it should break anything.

I would normally submit a PR but currently super occupied this month. 🤔

@BALaka-18
Copy link
Contributor

BALaka-18 commented Mar 25, 2021

@davidism I'm an MLH Fellow, I am working on this issue.

@davidism davidism added this to the 8.0.0 milestone Mar 30, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2021
@davidism
Copy link
Member

davidism commented Jun 14, 2021

This is causing issues: #1921, #1958. I'm going to revert this change. Update your Python version if you want the behavior offered in a later Python version. Discussion is on #1921

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 a pull request may close this issue.

3 participants