Repository navigation
fix(fsspec): handle Windows drive paths with the file protocol - #801
Open
dajiaohuang wants to merge 2 commits into
Open
dajiaohuang wants to merge 2 commits into
dajiaohuang wants to merge 2 commits into
Conversation
On Windows, paths like `D:/data/file.txt` are parsed by `urlparse` with `D:` treated as a URL scheme, so the drive is dropped from the bucket and later re-resolved against the current drive, producing duplicated-drive paths like `D:/D:/data/file.txt` (os error 123). This broke every `file`-protocol operation on Windows, including relative paths, which are absolutized to drive paths (developmentseed#763, developmentseed#555). - `_split_path`: detect single-letter drive schemes on Windows and return the drive (with colon, e.g. `D:`) as the bucket, preserving the original drive instead of re-resolving against the cwd drive. - `_construct_store`: for the `file` protocol with a drive bucket, construct a `LocalStore` rooted at that drive directly instead of going through `from_url`, which cannot express drive roots. - `_info`: on Windows, `head` on a directory raises `GenericError` rather than `FileNotFoundError`, so also catch `GenericError` and fall through to the directory-listing fallback for local directories. - `_get_file`: a Windows drive-letter local path was treated as a URL scheme and silently skipped; only treat genuine schemes as remote sources.
kylebarron
reviewed
Oct 16, 2026
Comment on lines
+310
to
+332
| if self.protocol == "file" and _is_windows_drive_path(path): | ||
| # A Windows drive path such as `D:\dir\file` or `D:/dir/file` is | ||
| # not a URL: `urlparse` would parse the drive letter as the URL | ||
| # scheme. Resolve the whole path and return the addressed drive | ||
| # as the bucket so that `_construct_store` can root a local | ||
| # store on that drive. | ||
| drive, _, rest = str(make_path_posix(path)).partition(":") | ||
| return (f"{drive}:", rest.lstrip("/")) | ||
| parsed_path = ( | ||
| f"{parsed.netloc}/{parsed.path.lstrip('/')}" if parsed.scheme else path | ||
| ) | ||
| if self.protocol == "file": | ||
| parsed_path = str(make_path_posix(parsed_path)) | ||
| if os.name == "nt": | ||
| # On Windows, `make_path_posix` resolves any path to a | ||
| # drive-rooted path such as `D:/dir/file`. As the store for | ||
| # an empty bucket is rooted at the current drive, joining a | ||
| # drive-rooted path onto it would duplicate the drive; | ||
| # instead return the resolved drive as the bucket, as for | ||
| # drive paths above. | ||
| drive, _, rest = parsed_path.partition(":") | ||
| if len(drive) == 1 and drive.isalpha(): | ||
| return (f"{drive}:", rest.lstrip("/")) |
Member
There was a problem hiding this comment.
This is extremely verbose and I don't think you should need successive checks for windows.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Fixes #763. Also resolves the Windows half of #555 (the
exists()-on-existing-directory failure; see analysis below).On Windows, every
file-protocol operation ofFsspecStoreis broken. Reproduced on Windows 10 with obstore 0.12.0 / fsspec 2026.9.0:Relative paths are broken too, because fsspec absolutizes them to drive paths before calling into the store.
Root cause
_split_pathrunsurlparseon the incoming path. On Windows,urlparse("D:/x")parsesD:as a URL scheme, so the drive is dropped from the bucket. Later,make_path_posixre-resolves the path against the current drive, producing duplicated-drive paths likeD:/D:/x(os error 123). This also silently changes the semantics of explicit cross-drive paths (e.g.E:/xused from aD:cwd)._construct_storebuilds theLocalStoreviafrom_url("file://")(root of the current drive) and joins the (already drive-less) path, which is where the duplication surfaces._info: upstream object_store'sheadon a directory raisesFileNotFoundErroron POSIX, but on WindowsFile::open(dir)fails with os error 5, which surfaces asGenericError. The existingexcept FileNotFoundErrornever triggers, so the directory-listing fallback for_info/existsnever runs on Windows (LocalStore.head()fails for existing local folders in Windows - makingFsspecStore.exists(),FsspecStore.rm()fail too #555)._get_file:urlparse(lpath)treats a Windows drive-letter local path as having a scheme, so local downloads with drive paths silently no-op.Changes
Minimal, Windows-only changes in
obstore/python/obstore/fsspec.py:_split_path: detect single-letter drive schemes on Windows and return the drive (with colon, e.g.D:) as the bucket — covers absolute posix/backslash forms,file://D:/...URLs, and (post-absolutization) relative paths; preserves the originally requested drive._construct_store: for thefileprotocol with a drive bucket, constructLocalStore("D:/")directly instead of going throughfrom_url, which cannot express a drive root. Thefrom_urlpath (empty bucket) is unchanged, so POSIX behavior is byte-for-byte identical._info: also catchGenericErroron Windows and fall through to the directory fallback only when the local path really is a directory; otherwise re-raise._get_file: only treat a genuine URL scheme as a remote source, so drive-letter local paths are handled as local.tests/test_fsspec.py:test_split_pathis now platform-conditional (POSIX assertions unchanged); newtest_file_protocol_windows_drive_paths(skipped on non-Windows) exercisesexists/info/isdir/cat_file/open/ls/pipe_file/open(wb)/get/rm_fileon real Windows drive paths without Docker.Note: the project's Python CI runs on ubuntu-latest only, so none of this is currently caught by CI; that's why these bugs went unnoticed.
Validation
exists(posix/backslash/URL forms), missing→False, directory handling,info,cat_file,openread/write,ls,pipe_file, copy,rm_file,get/putwith Windows local paths,modified,_split_pathunit forms (absolute/URL/relative/other-drive preserved), and unchangedmemory-protocol behavior — all pass.tests/test_fsspec.py: 13 passed, including the new Windows test andtest_buffered_file_forwards_size_to_open_reader, which failed before this fix.ruff check+ruff format --checkon both modified files: clean (repo config).Bytesstub-resolution diagnostic is identical onmain).Context: @amachanic offered to fix #763 back in August but no PR has landed; happy to hand this over if you'd rather take it from here.