Extend filesystem interface

[aol-nnov]

Interface is extended as a part of ext4 improvements effort https://github.com/diskfs/go-diskfs/issues/9#issuecomment-2422384742

Also, Remove method is introduced for all supported filesystems.

@deitch should we keep ext4.Rm() or we can drop it alltogether infavour of newly introduced filesystem.Remove()?

[aol-nnov]

Oh, just stumbled upon https://github.com/diskfs/go-diskfs/pull/233!

What do you think about adding Rename() to the interface as well?

[deitch]

I rather like this. Thank you for doing it.

Oh, just stumbled upon https://github.com/diskfs/go-diskfs/pull/233!

Yup, although that didn't complete the interface, and you did here. So let's complete this one, and then that can just be the specific implementation.

What do you think about adding Rename() to the interface as well?

Sure. There is a parallel in os.Rename() , so it works well.

should we keep ext4.Rm() or we can drop it alltogether infavour of newly introduced filesystem.Remove()?

For now, let's deprecate it. Mark it as deprecated in the comments and refer to Remove(), and have it just call ext4.Remove(). It also parallels os.Remove(). In the future, we can remove it entirely.

The only other comment I had was about the returned error. I will comment inline. Once that is in, I will kick off CI and this can run and get merged in.

Thanks @aol-nnov !

[aol-nnov]

Also, what do you think about adding chown and chmod as well? It seems, the interface will be complete then.

[deitch]

Good call.

[aol-nnov]

Ok, will mock something as soon as I get to my laptop.

[aol-nnov]

Added Rename, Chown, Chmod to the Filesystem interface, used newly introduced ErrReadonlyFilesystem error where applicable.

[deitch]

This is great. Let's let CI go, and if green, we can merge it in.

[aol-nnov]

Bummer!... Linter does not like parameters in unimplemented functions, but I'd rather keep them for documentation reasons...

Is there any annotation for such cases?

[deitch]

Some of the lint errors are duplication, e.g.:

func (fs *FileSystem) Link(oldpath string, newpath string) error {

Should be:

func (fs *FileSystem) Link(oldpath, newpath string) error {

Some of the others are shadow imports, like:

func (fs *FileSystem) Mknod(path string, mode uint32, dev int) error {

Which shadows the path package, so change the name of the parameter to something non-duplicate like name or p, etc.

For the last, unused parameter, just replace it with _, like:

func (fs *FileSystem) Rename(_, newpath string) error {

Or, if you want to keep it, you can put in a nolint for it. E,g,:

// Rename rename a file from oldpath to newpath
//
//nolint:unused-parameter // keep it around to make it easier in the future
func (fs *FileSystem) Rename(oldpath, newpath string) error {

[aol-nnov]

Thank you for the hints! Will fix this up shortly!

[aol-nnov]

For all the ErrNotSupported cases I've replaced parameter names with _ placeholder.

As for ErrNotImplemented cases, unfortunately, I had no luck with //nolint:unused-parameter, but I've worked it around the other way:

func (fs *FileSystem) Remove(_ /*pathname*/ string) error {
    return filesystem.ErrNotImplemented
}

The coder to implement this will then remove the placeholder and uncomment the actual parameter name.

[deitch]

I had no luck with //nolint:unused-parameter

My mistake, it should be //nolint:revive. See this func for example.

CI looks clean. Let's replace that, and then we can merge it in.

[aol-nnov]

Okay, done.