-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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: os: add FS() (filesystem simply using os functions) #47803
Comments
These functions already exist in a different form: import "io/fs"
...
root := os.DirFS("/")
...
f, err := root.Open(path)
dirs, err := fs.ReadDir(root, path)
fi, err := fs.Stat(root, path) Note, all |
The different form is exactly what is the limiting factor today, which is that Given the current unrooted aspect, this proposal is likely asking to relax that restriction. There's likely a good reason for the restriction that I'm unaware of, though. |
From the draft design:
OS paths and It's probably better/less buggy to translate paths to |
Somewhat related: #44279. |
#44286 also highlights some difficulty using relative paths. |
This runs into the problem of dealing with the current working directory, which would be at a different path prefix. Rereading the proposal and ValidPath documentation, maybe |
I wanted to write an FS for CLIs that would be able to open files or URLs, e.g. |
fs.FS very clearly disallows paths starting with / or with ../ or even with ./ - they must be rejected by any valid Open implementation. |
This proposal has been added to the active column of the proposals project |
Is there any way to make fs.FS broadly useful for rooted filepaths, or is that a nonstarter? If it's a nonstarter, this proposal should be closed. |
Existing This issue, and a number of issues linked here (#44279 in particular) highlight the difficulty of translating between OS and It may make sense to provide functionality to sanitise OS paths so they are suitable for |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
There's no correct way to provide the behavior of native `os` functions with an `fs.FS` since `fs.FS`s should reject rooted paths and only use unix path separators ("/"). Initially I created a `rawFS` that directly forwarded calls to the `os` package but it felt more wrong the more I looked at it. Relevant issues: * golang/go#47803 * golang/go#44279 closes open-policy-agent#5066 Signed-off-by: julio <julio.grillo98@gmail.com>
There's no correct way to provide the behavior of native `os` functions with an `fs.FS` since `fs.FS`s should reject rooted paths and only use unix path separators ("/"). Initially I created a `rawFS` that directly forwarded calls to the `os` package but it felt more wrong the more I looked at it. Relevant issues: * golang/go#47803 * golang/go#44279 closes open-policy-agent#5066 Signed-off-by: julio <julio.grillo98@gmail.com>
There's no correct way to provide the behavior of native `os` functions with an `fs.FS` since `fs.FS`s should reject rooted paths and only use unix path separators ("/"). Initially I created a `rawFS` that directly forwarded calls to the `os` package but it felt more wrong the more I looked at it. Relevant issues: * golang/go#47803 * golang/go#44279 closes #5066 Signed-off-by: julio <julio.grillo98@gmail.com>
With the advent of
fs.FS
, I prefer to use that interface where possible. One paper cut that I've run into a few times is that there no way to use the os package outright as anfs.FS
. Today, there is onlyos.DirFS
, which returns a filesystem anchored at the given directory.This is great for something like a file server, where we only want to serve files under a given directory and not provide access to anything outside that directory, but is less great for when we want to provide arbitrary access to the file system. In particular, if I have a program that accepts flags for where to load files, I want to accept both a local path (
cmd -path localfile
), or a fully specified path (cmd -path /absolute/directory/file
). Where I usefs.FS
within this program, I have to specify a thin wrapper around os, becauseDirFS
does not provide the necessary flexibility.The proposal is to add the following function and implementation:
The text was updated successfully, but these errors were encountered: