Skip to content

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

Closed
izbyshev wants to merge 2 commits intopython:masterfrom
izbyshev:bpo-32907
Closed

bpo-32907: pathlib: Fix test_resolve_common failure on Windows#5812
izbyshev wants to merge 2 commits intopython:masterfrom
izbyshev:bpo-32907

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

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.

5 participants