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

Use finalized io/fs.FS interface definitions #63

Closed
infogulch opened this issue Jun 22, 2021 · 1 comment · Fixed by #86
Closed

Use finalized io/fs.FS interface definitions #63

infogulch opened this issue Jun 22, 2021 · 1 comment · Fixed by #86

Comments

@infogulch
Copy link

infogulch commented Jun 22, 2021

The fs.FS interfaces have been published in Go 1.16. I suggest updating the interface definitions defined in filesystem.go to align with the finalized design of FS.

This project defines custom FS/File interfaces (reproduced below) that match an early draft of the FS interface design. I'm happy to see the attempt to match the coming spec, but unfortunately the spec had a few changes between the referenced draft and final design. Luckily it seems that the draft was still pretty close and I expect would require minimal changes to conform.

// FS represents a minimal filesystem implementation
// See io/fs.FS in http://golang.org/s/draft-iofs-design
type FS interface {
	Open(name string) (File, error)
	ReadFile(name string) ([]byte, error)
	ReadDir(dirname string) ([]os.FileInfo, error)
}

Thoughts?

@apparentlymart
Copy link
Contributor

I agree that it would be ideal to use the same interfaces and types as io/fs.

Unfortunately, as you've noted #49 adopted interfaces from a draft version of that design which doesn't match the final design. It isn't clear to me that we can change this in-place now without breaking existing callers that may have implemented tfconfig.FS.

A type cannot implement both tfconfig.FS and fs.ReadDirFS at the same time because they both use method name ReadDir but with different return types. Therefore I think we may need to define some sort of bridge implementation so that existing callers can keep using tfconfig.FS while new callers can use fs.FS.

It isn't clear yet what the best design would be for that, but the key requirement is that tfconfig.LoadModuleFromFilesystem must keep expecting tfconfig.FS. Hopefully it's possible to write a tfconfig.FS implementation that wraps a fs.FS and translates to the corresponding functions in io/fs.

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

Successfully merging a pull request may close this issue.

2 participants