twpayne / go-vfs Goto Github PK

Thanks for the great repository. Do you have any plan to support Minio/S3 support? Maybe you could use minio's SDK to do that.

Hey! Nice work... Golang really needs a standard Filesystem interface upstream, or at least one that everyone gathers around. In any case, maybe my etcdfs work is helpful here. I'm not contributing to afero anymore because: spf13/afero#184

I saw your compatibility layer in https://github.com/twpayne/go-vfsafero -- it would be interesting to be able to go the other way!

In any case, feel free to close this, thought it might be a good place for discussing additional FS.

While working on twpayne/chezmoi#23 I ran the go-vfs tests on Windows and encountered a couple of failures. I've put together some fixes (https://github.com/zb140/go-vfs/tree/windows) after which all of the tests pass on both Windows and Linux (I don't have a Mac so I haven't tested there).

There were a couple of changes necessary that I think are worth calling special attention to:

• On Windows, symlinks should not be created before the things they point to exist (because links to directories are different from links to files, and the difference must be explicitly specified when calling CreateSymbolicLink). Since the filesystems in vfst are declared as a map and you can't rely on iteration order, tests that were creating symlinks to directories were often creating them before the directories existed, causing them not to work. I solved this by adding support to Builder.build to be able to take a list of any of the types it previously supported, then recursively call them in order. See https://github.com/zb140/go-vfs/blob/cd3b3edf4f462bb963afdab8900eb7e6836fff33/vfst/vfst.go#L83
• When Windows is asked to make a directory containing a path component that exists but is not a directory, the Windows APIs return (the equivalent of) ENOENT rather than ENOTDIR. This causes MkdirAll to attempt to recursively create the path, which will fail because it already exists. This is detected correctly but MkdirAll was swallowing the error, causing the test to detect no error when one was expected. I fixed it by returning the error rather than ignoring it. See https://github.com/zb140/go-vfs/blob/cd3b3edf4f462bb963afdab8900eb7e6836fff33/mkdirall.go#L49
• PathFS.join expects incoming path names to be absolute before the internal prefix is added. But on Windows, a path is never considered absolute if it doesn't start with a volume specifier, which means the check was always failing. To fix this, I coerce the incoming paths to Unix-style using filepath.ToSlash, then check for an absolute path using path.IsAbs instead of filepath.IsAbs. See https://github.com/zb140/go-vfs/blob/cd3b3edf4f462bb963afdab8900eb7e6836fff33/path_fs.go#L224

Current PathFS uses filepath.Join to combine names with the root. This does not prevent writes outside the root, for example by using relative paths. PathFS should prevent writes outside its root.