-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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
proposal: io/fs: add writable interfaces #45757
Comments
I'm currently experimenting with writing a file format encoding/decoding/mutating package that is intended to work with files that aren't guaranteed to easily fit in memory. I would like to implement it in terms of This codec package doesn't want to care about supporting all the different places that the underlying files could be stored. It just wants to take in an Additionally, to make this package more testable, using Unfortunately, since |
I've got a similar issue with my p9 package. It attempts to implement a 9P client and server in a high-level way, similar to Maybe something like this could work? type RWFS interface {
FS
WriteFS
}
type WriteFS interface {
// Create creates a new file with the given name.
Create(string) (WFile, error)
// Modify modifies an existing file with the given name.
Modify(string) (WFile, error)
}
type WFile interface {
Write([]byte) (int, error)
Close() error
// Maybe also some kind of WStat() method?
} Then the returned types would only have to expose either reading or writing methods, and the interface would just handle it transparently. Persionally, I think that it would be a lot better if there was some way to abstract away the specific requirement of an |
To me, this looks like the minimum requirement: package fs
type WFile interface {
Stat() (FileInfo, error)
Write(p []byte) (n int, err error)
Close() error
}
type WriteFS interface {
OpenFile(name string, flag int, perm FileMode) (WFile, error)
} And another (ugh) for making dirs: type MkDirFS interface {
MkDir(name string, perm FileMode) error
} And some helper functions for convenience: func Create(fsys WriteFS, name string) (WFile, error) {
// Use fsys.OpenFile ...
}
func WriteFile(fsys WriteFS, name string, data []byte, perm FileMode) error {
// Use fsys.OpenFile, Write, and Close ...
}
func MkDirAll(fsys MkDirFS, path string, perm FileMode) error {
// Use fsys.MkDir to do the work.
// Also requires either Stat or Open to check for parents.
// I'm not sure how to structure that either/or requirement.
} |
I think that we should lean more heavily on io and os, rather than making top-level Summary
DetailSee sketch of an implementation here: https://gist.github.com/kylelemons/21539a152e9af1dd79c3775ca94efb60#file-io_fs_write_sketch-go This style of implementation appeals to me because:
I think the same patterns can be used to implement |
I think that also having a Just a small contribution as it feels to me in the scope of this proposal, and since no one mentionned file removal 🙂 |
@kylelemons just to be consistent with fs.ReadDirFile, the only File extension at the moment, and iofs draft/The File interface: type ReadDirFile interface {
File
ReadDir(n int) ([]DirEntry, error)
}
type WriteFile interface {
File
Write(p []byte) (n int, err error)
} There is a comment from Russ Cox about this exact change as well. |
There's an interesting package https://pkg.go.dev/github.com/hack-pad/hackpadfs that's got some interfaces defined for a writable version of |
Looks like this proposal has label Proposal but isn't a part Review Meeting #33502 @rsc can you move it into https://golang.org/s/proposal-status#active column please? |
This proposal has been added to the active column of the proposals project |
I haven't thought as in depth as some of the other folks here, but I came up with a similar set of interfaces and shortcuts for an FS backed by both the local filesystem and backed by the dropbox API. Here's the API (obviously not all of this is so general purpose:) type CreateFS interface {
fs.FS
Create(string) (FileWriter, error)
}
type FileWriter interface {
fs.File
Write([]byte) (int, error)
}
type RemoveFS interface {
fs.FS
Remove(string) error
}
type WatchFS interface {
fs.FS
Watch(context.Context, string) (chan []fsnotify.Event, error)
}
type WriteFileFS interface {
fs.FS
WriteFile(string, []byte, fs.FileMode) error
}
func OpenDirFS(d string) fs.FS
func Remove(fss fs.FS, path string) error
func Watch(ctx context.Context, fss fs.FS, dir string) (chan []fsnotify.Event, error)
func WriteFile(fss fs.FS, name string, contents []byte, mode fs.FileMode) (err error) |
If we add a
|
Hello everyone! Recently I encountered a similar use case where I wanted to dependency inject a filesystem, I came up with the following header interface that closely follows the type FileSystem interface {
Stat(name string) (fs.FileInfo, error)
OpenFile(name string, flag int, perm fs.FileMode) (File, error)
Mkdir(name string, perm fs.FileMode) error
Remove(name string) error
}
type File interface {
fs.File
fs.ReadDirFile
io.Writer
io.Seeker
} I made an interface testing suite to cover roughly the ~127 most common filesystem interactions from a behavioural point of view.
The same interface testing suite tests both. You can use them as a drop-in replacement for places where you had to use I plan to use and maintain this until an easy to use replacement comes out in the std library. Cheers! |
I'd write like this: type WritableFS interface {
fs.FS
OpenFile(name string, flag int, perm fs.FileMode) (WritableFile, error)
}
type WritableFile interface {
fs.File
io.Writer
}
func Create(fsys WritableFS, name string) (WritableFile, error)
func WriteFile(fsys WritableFS, name string, data []byte, perm fs.FileMode) error
... |
On top of a +1 for having the need of a writable fs interface, I'd like to enter into the conversation that files are not inherently readable or seekable. for example, files may be opened with That is , when designing the interface, I suggest keeping the writer interface disjoint from the reader interface, and instead having a third interface to union them. Any writable file implementation could return an error for read/seek operations...but it'd be nice to express that they're not necessary |
@chrisguiney there are already |
These interfaces are for writable "things" like files, but the requirement in this issue is for a file-system-level interface, not for a "file"-level interface, so this does not seem to answer the need, does it ? |
Personally, I don't think there is a practical way to support a writable When it comes to reading files, OS-dependent semantic differences can mostly be papered over by handwavingly assuming the Questions which immediately arise are
As far as I can tell, there just is no sound way to build an abstraction over filesystems that allows writing. The best advice I've heard so far on this topic is "if you care about data being written, use a database". Of course, it is possible to just provide some API and tell the programmers that they can't actually rely on the data actually being written, so it shouldn't be used in any production use case. But that just feels irresponsible. |
@Merovius isn’t the os package such an abstraction? Would you consider that irresponsible/not safe for prod? |
Yeah, I’m not convinced by any of that @Merovius. The write interface doesn’t need to support every imaginable operation to be useful, and of the operations it would support, Go is well known for taking a pragmatic approach, even when it leads to correctness issues in some circumstances, and this pattern is all over the file interfaces Go already provides. Windows doesn’t support Unix permission bits, so Go just… does whatever it feels like on Windows. Maybe we shouldn’t use Go in production? I disagree. A lot of your questions would be up to the implementation to decide. If the implementation is transparently delegating to a real OS filesystem, the answers would mirror what we currently see in the |
I created hackpadfs to use file systems in places they were impossible before or did not have the required performance characteristics. I've written about hackpadfs before, which covers why it was needed and some concrete use cases. Strong guarantees on several writable interfaces' behavior is certainly valuable, even though it may be difficult to create precise definitions. The most difficult problem I needed to solve was compliance with While difficult, only a few minor differences appeared across the major OSes with regard to As a small start, I tend to agree with @ianlancetaylor's suggestion for something like an An excerpt from package hackpadfs
// FS provides access to a file system and its files.
// It is the minimum functionality required for a file system, and mirrors Go's io/fs.FS interface.
type FS = gofs.FS
// File provides access to a file. Mirrors io/fs.File.
type File = gofs.File
// CreateFS is an FS that can create files. Should match the behavior of os.Create().
type CreateFS interface {
FS
Create(name string) (File, error)
} (Note: I did my best to read through all of the comments above, though I may have missed some.) |
Returning to this issue. Back on April 21, @bcmills suggested 7 new interfaces in #45757 (comment).
The usual advice is that if you have 7 of something you probably missed one. The hackpadfs post linked in #45757 (comment) notes 27 new interfaces:
The 'er' in the File interface names is non-idiomatic by the way. And I also don't understand why ReadDirFS, StatFS and SubFS are listed as new, when fs.ReadDirFS, fs.StatFS, and fs.SubFS already exist. But those quibbles aside, with 27 new interfaces the chance of having missed one increases exponentially. These illustrate the fundamental problem. We are not going to add 27 new interfaces to io/fs. Even 7 new ones seems like a bit much. If there is a path forward here, it needs to be a more general path. I don't think that general path has been discovered yet. |
When you state it this way - by haphazardly listing a bunch of proposed interfaces - I completely agree. But restated as a simple rule:
you can reduce the chances of "missing one" back down to zero. |
The Moreover, It may be that my list is incomplete or deficient (for example, perhaps |
@rsc All fair criticisms. You're quite right about the "new" interfaces bit, I'll need to fix that. I agree a smaller, focused selection would be more appropriate here for the first blush. A useful and writable hierarchical file system could be reduced to creating both directories and writable files. In other words, these two: type OpenFileFS interface { FS; OpenFile(name string, flag int, perm FileMode) (File, error) }
type MkdirFS interface { FS; Mkdir(name string, perm FileMode) error } |
@bcmills I like your initial selection of interfaces, and agreed on reducing redundancy.
They aren't completely equivalent of course, but it might be good enough to reduce the set of new interfaces. I'm curious what you think. |
This comment was marked as outdated.
This comment was marked as outdated.
The io/fs package was intended to be a useful intersection of things people might want, with the expectation that people would define their own extension interfaces above it, using io/fs as the interoperable core. So far that seems to be happening, including in large examples like HackPadFS. My comment above still seems accurate, that there's no clear convergence happening otherwise. The main general use case I have seen raised since that comment is being able to extract a file system tree, which would require Create and Mkdir. Maybe that is compelling, but maybe not. It might make more sense to let writable file systems implement their own CopyFS functions instead, like
The nice thing about os.CopyFS is that it can decide how much information to take from fs.FS and what to leave out. I remain skeptical about a general-purpose directory copy function working. os.CopyFS is not general-purpose - it knows how to write to os files. Similarly, maybe it makes sense to add a CopyFS method to zip.Writer. That seems entirely workable, whereas zip.Writer cannot support an arbitrary file system create interface, since it cannot have more than one file open for create at a time. So if the goal is to supporting copying of fs.FS, perhaps we should establish that pattern instead of adding something new to io/fs. What are the other general-purpose routines that would be enabled by extending the io/fs interfaces? |
This is basically the route I went down. I wanted to test a project's write operations as well. There were a few hitches though:
So yeah, ultimately this is doable, but this process could be made slightly easier. Sidenote, the interfaces I ended up with were basically type WriteableFile interface { fs.File; io.Writer }
type CreateFS interface { fs.FS; Create(name string) (WriteableFile, error) }
type MkdirFS interface { fs.FS; Mkdir(name string, perm fs.FileMode) (fs.FS, error) } |
I have a routine which reads files from disk, analyzes them, makes some changes, and writes them back. I’d like to make it take an interface fs so that I can test it more easily. |
I'm implementing a networked virtual filesystem and want to have a common write interface for components to use, at present I have to implement adapters to the different abstractions available (which there are many) which reminds me of how reading was before the readable FS was added. |
Even if we do figure out writable FS, it still may not be worth trying to do a general fs.Copy. In the meantime, I wrote this trivial CopyFS that could be added to package os and have already found a few uses for it:
|
Is interface conversion from the Raising my use case from #45757 (comment), if we're not going to add
An alternative might be to add a method to |
Here https://github.com/jarxorg/wfs is a package I made a few years ago and it works fine on the small systems. So far it has 4 filesystems osfs, memfs, s3fs, gcsfs and it passes TestFS in "testing/fstest". I hope a simple writable interface added to "io/fs". |
@bcmills and I looked at this, in particular at whether a step forward would be to adopt the approach in #45757 (comment). Unfortunately, the very first interface poses a problem: what are the open modes and where are they defined? I'm sure existing implementations assume the "os" modes, but io/fs is meant to be a completely portable API, independent of "os". So probably we'd have to define a new set of modes, and code would have to translate those modes into whatever they need. That's doable, but it's a sign that maybe we don't have the right answers yet. I still haven't seen a compelling reason this needs to be in the standard library now. Maybe one will arise later, or maybe not. Either way, the extension interface pattern means that people can define these in their own packages and even interoperate with no problems. The only problems arise when the extensions refer to private types, and none of the proposed extensions do. So it seems like we should probably decline this proposal and maybe circle back in a year or two. |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
Go 1.16 introduced the
embed
andio/fs
packages. The current implementation is a minimal readable filesystem interface.This is a wonderful first step towards standardizing filesystem operations and providing the community with a lot of flexibility in how we implement filesystems. I would like us to take the next step and define a writable file system interface.
Problem
fs.FS
can't be written to after it's defined.Optional interfaces could be defined in user-land:
But it suffers from the same issues that the readable filesystem interface aimed to solve: standardizing the interface across the ecosystem.
Use Cases
I'll list of few use-cases that I've come across since Go 1.16, but I'm sure the community has many more:
We've already seen started to see this pop up in the community around
io/fs
to address the problem in user-land:fs.MkdirAll
andfs.WriteFile
file.Write
,file.Seek
andfile.Close
.A quick search on Github will yield more community libraries: https://github.com/search?q=%22io%2Ffs%22+language%3Ago. For many of these implementations, you can imagine a useful writable implementation.
Of course, there are many other file system libraries that came before
io/fs
that define writable interfaces like afero and billy.Proposal
I don't feel qualified to define an interface, I know people have thought about this much harder than I have. What I would love to see from a community member's perspective is the following:
Nice to Have: Be able to define if a filesystem is readable, writeable or read-writable.
Thanks for your consideration!
The text was updated successfully, but these errors were encountered: