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

io/fs: provide backwards-compatible migration path from ioutil.ReadFile to fs.ReadFile #44286

Open
bouk opened this issue Feb 16, 2021 · 4 comments
Open

Comments

@bouk
Copy link

@bouk bouk commented Feb 16, 2021

Hi there, I'm enjoying the go1.16 RC, I'm trying to migrate some code from using ioutil.ReadFile to fs.ReadFile while staying backwards-compatible. I'm running into two things:

  • It's not possible to make code that does something like ioutil.ReadFile("./path/to/file.txt") work with os.DirFS("."), because it rejects the leading ./
  • There is no way to have absolute paths for the same reason, because 'Open should reject attempts to open names that do not satisfy ValidPath(name)' according to the fs.FS documentation

I can get around those things by doing something like this:

// osFS is an fs.FS implementation that just passes on to os.Open
type osFS struct{}

func (c osFS) Open(name string) (fs.File, error) {
	return os.Open(name)
}

Which is technically an invalid implementation, although it's not entirely clear why those restrictions are in place. It would be nice if something like the above osFS was built-in, so there's a clear migration path from ioutil.ReadFile to fs.ReadFile.

What version of Go are you using (go version)?

$ go version go1.16rc1 darwin/arm64
@D1CED
Copy link

@D1CED D1CED commented Feb 16, 2021

You can just create a simple compatibility decorator like this:

package relfs

import (
	"io/fs"
	"os"
	"strings"
)

type allowRelativeFS struct{ fs.FS }

func (rfs allowRelativeFS) Open(name string) (fs.File, error) {
	if strings.HasPrefix(name, "./") {
		name = name[2:]
	}
	return rfs.FS.Open(name)
}

func DirAllowRelative(dir string) fs.FS {
	return allowRelativeFS{os.DirFS(dir)}
}

You can get a lot more fancy like allowing absolute paths or add logging to ease migration.

Loading

@josharian
Copy link
Contributor

@josharian josharian commented Feb 16, 2021

Related: #44279

Loading

@toothrot
Copy link
Contributor

@toothrot toothrot commented Feb 17, 2021

cc @rsc

Loading

@mpx
Copy link
Contributor

@mpx mpx commented Aug 19, 2021

Perhaps fs.ValidPath could accept paths starting with ./? There are (IMO) good reasons for disallowing /, but ./ is still clearly a relative path should be ok? This might be difficult given potential incompatibilty with existing fs.FS implementations tho'.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants