Skip to content

Handle filesystem roots in DirFileSystem path translation - #2290

Open
zhechen-cs wants to merge 1 commit into
fsspec:masterfrom
zhechen-cs:fix/dirfs-filesystem-root
Open

zhechen-cs wants to merge 1 commit into
fsspec:masterfrom
zhechen-cs:fix/dirfs-filesystem-root

Conversation

@zhechen-cs

Copy link
Copy Markdown
Contributor

DirFileSystem rooted at a local filesystem root can raise AssertionError when listing that root. Root path joining and relative-name stripping both add a separator to a prefix that already ends with one; POSIX double-leading-slash normalization can also make leading-slash and ordinary relative queries disagree.

Join root prefixes with one separator and use the same prefix when returning relative paths. Preserve existing subtree behavior and trailing-slash copy semantics. Add POSIX/drive-root path cases for synchronous/asynchronous wrappers and real local ls(), info(), find() and glob() checks with both forms of the query path.

Validation on macOS arm64 / CPython 3.12.13, independently based on upstream 8cf902e:

  • Unchanged-source path/regression selection: 10 failed, 22 passed.
  • DirFS/local/core/spec/mapping/chained suites: 801 passed, 92 skipped, 1 xfailed. The clean baseline passes 785 tests with the same skips and expected failure.
  • Direct local root listings for both empty and slash paths now succeed and return relative names.
  • Applicable automatic pre-commit hooks and git diff --check pass.

The suite emits the same multiprocessing fork deprecation warning on the baseline and patch. Native Windows, cloud, FUSE and downstream matrices were not run locally. A changelog entry is included.

Comment on lines +84 to +85
if self.path.endswith(self.fs.sep):
path = path.lstrip(self.fs.sep)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this wrong? Since we join on sep, shouldn't path always be lstrip-ped?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants