Skip to content
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

Support golang io/fs interface #30

Open
willscott opened this issue Mar 28, 2021 · 5 comments
Open

Support golang io/fs interface #30

willscott opened this issue Mar 28, 2021 · 5 comments

Comments

@willscott
Copy link
Owner

This library currently makes use of the billy v5 filesystem interface as the expected content that the driving program passes it to serve over NFS.

This interface is imperfect, although that is mostly due to rough edges in golang itself - fs.FileInfo's Sys as an escape hatch to system specific stat structs containing owner/group information on unix permissioned files is awkward to say the least, and the billy interface does not integrate the Change interface particularly well, making the requirements to support a read-write filesystem for this library non-obvious.

Golang 1.16 introduces the io/fs interface pattern for thinking about embedded files and filesystem. This uses a similar interface pattern as billy's Change, for interface-based detection of optional filesystem features. The initial 1.16 interfaces do not include any of the write or mutation portion of filesystems, though there is some indication that we may expect that to come eventually. They also do not provide a mechanism for resolving symlinks (no lstat or Readlink).

We should decide and then support either a io/fs.FS -> billy.Filesystem shim or native io/fs.FS.

@scorpionknifes
Copy link
Collaborator

@willscott What's your thoughts about switching from billy to io/fs?

@willscott
Copy link
Owner Author

I'm happy to see this happen. I think the main thing is that my understanding is that there is not yet a clear standardization on what the extension of the core io/fs interface would be for mutation / changing. E.g. how the methods expressed in the Change link above should be conveyed back to the filesystem.

@scorpionknifes
Copy link
Collaborator

@willscott been playing around with io/fs and somewhat got a working fork https://github.com/scorpionknifes/go-nfs. Currently basing mutation by following os.DirFS and only the osnfs example works. Just want to get some feedback and thoughts about it.

I'm planning to get memfs working and write some tests (the fork is still in very early stages)

@willscott
Copy link
Owner Author

Cool!

I'll add you to this repo so you can work on a branch here and we can get this into a PR and merged as you get happy with it.

  • You have each specific method (StatFS, CreateFS, etc) as a separate interface.

    • Can you re-use the upstream interface definitions where they exist, e.g. io/fs.StatFS?
  • We should define an additional interface layer to be clear about what capabilities are needed for different types of mounts

    • (e.g. you don't need WriteFile if you support WriteAtFile as well, but you do need MkdirAll if you want a writable NFS mount)
    • I think there's probably a 'readable NFS mount' layer, and then 2 ways it can be extended - there's an additional set of interfaces to implement if you want to expose a 'read/write NFS mount', and a different set to expose a 'permissioned NFS mount'
  • There should be some way for the user to 'limit' how much of the filesystem they expose: If i pass in a DirFS, it will currently be a read/write filesystem. It would be good to be able to specify somehow a bit more cleanly (right now i think it would be through a custom handler) that i want to set up the filesystem in read-only mode even if it has the writable methods on it.

@scorpionknifes
Copy link
Collaborator

Thanks for the feedback! (Sry for being slow)

Currently working in https://github.com/willscott/go-nfs/tree/feat/iofs

A bit confused and not too sure what's the best way to 'limit' how much of the file system they expose. I'm thinking of limiting it by using a wrapper. Something like how the change file attribute interface is only available by using certain wrappers in here

Also, I think if you pass in an os.DirFS without a wrapper it would be a read-only fs.

Looking into setting up a memfs and is currently choosing between wrapping a billy.fs or an afero.fs or both. billy.fs wrapper should be a better choice as it will provide backwards compatibility. Let me know your thoughts about this.

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

No branches or pull requests

2 participants