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

bpo-32907: pathlib: Fix test_resolve_common failure on Windows #5812

Closed
wants to merge 2 commits into from

Conversation

izbyshev
Copy link
Contributor

@izbyshev izbyshev commented Feb 22, 2018

Canonicalize the temporary directory path since it is used in comparisons with other canonicalized paths.

Use Path.resolve() instead of os.path.realpath() to ensure that symlinks and "short" (8.3) filenames are resolved on Windows.

https://bugs.python.org/issue32907

# Use Path.resolve() to ensure that it doesn't contain the username in
# "short" (8.3) format on Windows (os.path.realpath() can't be used
# for that).
d = str(P(d).resolve())
Copy link
Contributor

Choose a reason for hiding this comment

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

Resolving makes sense in general. For example, "C:\Users" could be a junction or the user's %LocalAppData% may be a junction or relocated to a path that traverses a junction, or the user's %TEMP% could be non-standard.

That said, it might be useful in other cases to add nt._getlongpathname and nt._getshortpathname functions that call WinAPI GetLongPathName and GetShortPathName, and add methods to get the short and long paths to pathlib's Windows flavour. In particular I can see someone wanting the long path, but not the fully resolved path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. I've changed the wording a bit to make it look less like a Windows-specific fix. Thank you!

Canonicalize the temporary directory path since it is used in
comparisons with other canonicalized paths.

Use Path.resolve() instead of os.path.realpath() to ensure
that symlinks and "short" (8.3) filenames are resolved on Windows.
@csabella
Copy link
Contributor

csabella commented Oct 18, 2019

@izbyshev, this line was changed as part of bpo-34977. Does the issue you reported still occur? If it does, please resolve the merge conflict. Thanks!

@csabella csabella closed this Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants