-
Notifications
You must be signed in to change notification settings - Fork 72
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
Fix creating local filer at the root of a windows file system #487
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a tiny unit test for this.
Can be:
- Skip unless Windows
- Get a TempDir
- Create filer on /
- Assert stat on TempDir succeeds
integration tests pass |
Not a complete solution yet |
return &LocalClient{ | ||
root: NewRootPath(root), | ||
root: NewUnixRootPath(root), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's still a windows path if the specified path is not equal to /
, the name is confusing.
Since you have a UnixRootPath
, please add a WindowsRootPath
that uses filepath.Join
instead of path.Join
and includes the root check in Join
itself. Then there is no need for a NopRootPath
and we always end up picking the Windows one when running on Windows, regardless of the specified root.
solved by #506 |
Changes
Allows filer to be created at root of a windows file system
Tests
Manually