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

Symlink relative path deficiencies #1

Closed
jaraco opened this issue May 2, 2011 · 3 comments
Closed

Symlink relative path deficiencies #1

jaraco opened this issue May 2, 2011 · 3 comments
Labels

Comments

@jaraco
Copy link
Owner

@jaraco jaraco commented May 2, 2011

Originally reported by: Waldemar Kornewald (Bitbucket: wkornewald, GitHub: wkornewald)


The symlink() function doesn't correctly detect whether the target is a directory if the link is relative and placed outside of the current working directory. Also, readlink() always converts the link target to an absolute path which isn't correct when dealing with relative links. The attached patch fixes both problems.


@jaraco

This comment has been minimized.

Copy link
Owner Author

@jaraco jaraco commented May 2, 2011

Original comment by Jason R. Coombs (Bitbucket: jaraco, GitHub: jaraco):


Fixed in <<changeset 8adaddc5c801>>. Thanks for the patch.

In <<changeset 304ef3016729>>, I removed the use of isabs, because I don't think it's necessary, and also the normalization of link. Please review and let me know if those calls serve a purpose I didn't recognize.

@jaraco

This comment has been minimized.

Copy link
Owner Author

@jaraco jaraco commented May 2, 2011

Original comment by Waldemar Kornewald (Bitbucket: wkornewald, GitHub: wkornewald):


The isabs check is necessary because only relative paths are relative to the link's folder. Absolute paths are already absolute and can be passed to isdir() directly.

The normalization is more consistent because the other os.path commands work correctly on Windows even when there's a "/" in the path, so symlink should convert "/" to "" on Windows, too.

@jaraco

This comment has been minimized.

Copy link
Owner Author

@jaraco jaraco commented May 2, 2011

Original comment by Waldemar Kornewald (Bitbucket: wkornewald, GitHub: wkornewald):


Oh I just noticed that you left the target path normalization in the code. In that case, I guess it's fine to remove the link name normalization if that's already handled by the rest of the code.

So, only the isabs check is necessary.

@jaraco jaraco added major bug labels Jan 29, 2016
@jaraco jaraco closed this Jan 29, 2016
jaraco added a commit that referenced this issue Jan 29, 2016
Now `symlink()` recognizes as directories relative paths that are not given as relative to the current working directory (but instead relative to the target).
`readlink()` no longer converts the link target to an absolute path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.