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
gh-81790: support "UNC" device paths in ntpath.splitdrive()
#91882
gh-81790: support "UNC" device paths in ntpath.splitdrive()
#91882
Conversation
87267c4
to
5a22909
Compare
5a22909
to
c97bfbf
Compare
@eryksun have a mo to review? thanks |
This also fixes my issue #85871 I think! |
Co-authored-by: Eryk Sun <eryksun@gmail.com>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
Co-authored-by: Eryk Sun <eryksun@gmail.com>
LGTM. Leaving unmerged because some other people have said they're looking at it as well.
As was discussed in previous comments, this PR changes the behavior of For example,
The existing implementation handles these as absolute paths, i.e. paths that have both a drive and a root (per the definition used by both pathlib and the C++
The new restricted behavior will affect the results of |
Thanks Eryk, will fix. |
I've adjusted the patch so that it only affects paths beginning |
@eryksun do you approve of the PR now? |
Any other feedback @eryksun? Thanks |
As far as I can see, this PR is ready to merge. Other problems of interest:
|
@eryksun Could you file those other problems as new issues? Seems like we always have at least one "UNC isn't quite right" issue open, so may as well continue the tradition |
Adds support in
ntpath.splitdrive()
for DOS device paths with UNC links (beginning\\?\UNC\
).Previous patches for this bug substantially rewrote
splitdrive()
to improve how other oddities are handled (e.g. double slashes). This patch is more conservative: existing behaviour is maintained except when the prefixes are present.This brings
splitdrive()
in line with pathlib behaviour, and consequently allows us to factor out a duplicate implementation in pathlib.Fixes #81790 and #85871