os: hard to use DirFS for cross-platform root filesystem #44279
Comments
|
CC @rsc. |
IIUC there is no single root path on Windows: each drive letter is its own root. Perhaps you could implement an |
|
At least for my purposes, I don't need a single root. I only need the root corresponding to a particular path, since I'm only walking up the tree. The broader question is interesting too, though. (Ideally, for my purposes, I'd like to stop at filesystem boundaries, instead of traversing through mount points. But that's out of scope—the point of this issue is that it's hard to do something that feels like it should be easy. It is reasonable for detecting and stopping at mount points to be hard. 😁) |
|
I think the real problem here is, if you want to write an application that can open any file on the filesystem, then on Linux/Mac you can do: files := os.DirFS("/")but on Windows, this will make your application unusable: files := os.DirFS("/")
dir, err := fs.ReadDir(files, "c:/Windows/win.ini") // Error open c:/Windows/win.ini: invalid argument
dir, err := fs.ReadDir(files, "Windows/win.ini") // Error open //Windows/win.ini: The network path was not found"The network path was not found" is especially weird, as it suggests that On Windows, you need to work out which volume the file is on and modify the path before you can even call DirFS. If I have a function I suspect what this means is that many Go app authors will forget about the Windows case and do As a library author, the obivous solution here is to write your function as It would be really nice if this: files := os.DirFS("")
file, err := files.Open('d:/foo/bar.txt')did what most people probably expect it to do. Then you'd still need to call |
|
Maybe we could add a files := os.SystemFS()
// On Windows
file, err := files.Open('d:/foo/bar.txt')
file, err := files.Open('//networkShare/foo/bar')
On Linux
file, err := files.Open('/foo/bar') |
|
Actually - one more possible solution. On Windows, we could make it so files := os.DirFS("/")
// On Windows
file, err := files.Open('d:/foo/bar.txt')
file, err := files.Open('//networkShare/foo/bar')
On Linux
file, err := files.Open('/foo/bar')This changes the behavior of os.DirFS(), but since |
|
I've also been hit by this issue in production code, and after several attempts to fix it I just revert the code to use good old filepath+os. I would be happy with a safe cross-platform example, but a new os API will be also welcomed. |
Note that |
|
Ah, my bad: files := os.DirFS("/")
// On Windows
file, err := files.Open('d:/foo/bar.txt')
file, err := files.Open('networkShare/foo/bar')
On Linux
file, err := files.Open('foo/bar')This should be a reatively easy and safe change to make, since two out of three of these examples already work, and the third today returns an error, and it also seems pretty clear that this was probably what the caller intended. |
|
I don't think I think the less surprising behavior would be a filesystem that resembles the WSL2 files := os.DirFS("/")
file, err := files.Open("d/foo/bar.txt") |
|
(Or, perhaps it should support network shares but not list them in |
|
Unfortunately
Second, imagine you're writing an application that prints the contents of a file (essentially func catFile(path string) error {
files := os.DirFS("/")
buf := make([]byte, 4096)
slashPath := filepath.ToSlash(path)
// Remove the leading slashes.
for len(slashPath > 0) && slashPath[0] == '/' {
slashPath = slashPath[1:]
}
file, err := files.Open(slashPath)
if err != nil {
return err
}
_, err = file.Read(buf)
if err != nil {
return err
}
fmt.Println(string(buf))
return nil
}If the file was |
The Go compatibility policy explicitly allows for fixing “specification errors” and “bugs”. Given the bizarre current behavior and the newness of |
If you're doing that, then you already need a layer to convert absolute paths to rootless ones in order to use func catFile(absPath string) error {
root := "/"
if volume := filepath.VolumeName(path); volume != "" {
root = volume
path = path[len(volume):]
}
fsys, err := os.DirFS("/")
…
}Or, if you (for example) want to use a passed-in func toRootless(absFilePath string) (string, error) {
volume := filepath.VolumeName(absFilePath)
slashPath := filepath.ToSlash(absFilePath[len(volume):])
if !strings.HasPrefix(slashPath, "/") {
// Path is not absolute.
return "", fmt.Errorf(…)
}
relPath := slashPath[1:]
switch {
case volume == "":
return relPath
case len(volume) == 2 && volume[1] == ':':
return path.Join(string.ToLower(volume[:1]), relPath)
default: // UNC path
// Encode the UNC path as a DirFS path.
…
}
}In that case, it would suffice to transform the UNC path to anything that doesn't collide with a drive letter — perhaps f, err := fsys.Open("c/windows/win.ini") f, err := fsys.Open("share/networkShare/foo/bar.txt") |
@bcmills why is Lines 643 to 647 in 1f411e9 Lines 34 to 38 in 1f411e9 This means that this
but for |
|
It is quite confusing or not that obvious as This was example I played around to understand: func TestFsSubWithOpen(t *testing.T) {
osFS := os.DirFS("_fixture")
subPath := "../_fixture" // this depends on user. It could be given with slashes or backslashes
//subFS, err := fs.Sub(osFS, filepath.ToSlash(filepath.Clean(subPath))) // <-- consistent behavior, at least for my use-case
subFS, err := fs.Sub(osFS, filepath.Clean(subPath)) // result depends on OS even if you would think Clean helps you with separators and ../.
if err != nil {
t.Fatal(err) // nonWindows ends here
}
f, err := subFS.Open("index.html") // ./_fixture/index.html is actually existing file
if err != nil {
t.Fatal(err) // Windows ends here. Because eventually dirFs.Open tries to open `..\_fixture/index.html` and rejects `\`
}
f.Close()
}is it confusing then you are doing thanks @ianlancetaylor |
|
I suppose I would say that it doesn't really make sense to pass the result of |
|
Here is my take (workaround maybe?), which is stable now, and I think will be stable in the future. With this approach, we get a standard type OsFilesystem struct{}
func (f *OsFilesystem) Open(name string) (fs.File, error) {
return os.Open(name)
}
// *OsFilesystem implements fs.FS
var _ fs.FS = new(OsFilesystem)
func TestOsFilesystem(t *testing.T) {
// given
openFile := func(fsys fs.FS, path string) error {
_, err := fsys.Open(path)
return err
}
fsys := new(OsFilesystem)
t.Run("exists", func(t *testing.T) {
// given
path := "/tmp/existent.file"
// when
err := openFile(fsys, path)
// then
assert.NoError(t, err)
})
t.Run("does not exist", func(t *testing.T) {
// given
path := "/tmp/nonexistent.file"
// when
err := openFile(fsys, path)
// then
assert.Error(t, err)
assert.True(t, errors.Is(err, fs.ErrNotExist))
})
} |
|
@sewera, the use of |
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 <[email protected]>
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 <[email protected]>
|
Starting with Go 1.19.4 and Go 1.18.9, the |
|
This is a problem for my application as well; If you have control over the APIs that accept If you don't have control over the APIs, then it's a lot more complicated to wedge in the |
|
I thought I'd share how I decided to work around this issue. It might technically break the rules specified by First, I defined a way for a FS to set it's own path cleaning rules and a helper function that defaults to // CleanPathFS is an interface that extends fs.FS by adding path cleaning capabilities.
//
// Implementations of CleanPathFS can normalize or sanitize file system paths
// according to their specific requirements while maintaining compatibility
// with the standard fs.FS interface.
type CleanPathFS interface {
fs.FS
// CleanPath takes a path string and returns a cleaned version of that path
// according to the file system's specific requirements.
//
// The cleaning process may include:
// - Removing redundant path separators
// - Resolving relative path components ("." and "..")
// - Converting path separators to the file system's preferred format
// - Applying any file system-specific path normalization rules
//
// Parameters:
// - p: The path string to be cleaned
//
// Returns:
// - A cleaned version of the input path string
CleanPath(p string) string
}
// CleanPath cleans the provided path name using the file system's specific path cleaning
// mechanism if available, otherwise falls back to standard path cleaning.
//
// If the provided fs.FS implements CleanPathFS interface, it uses the file system's
// custom CleanPath method. Otherwise, it performs standard path cleaning.
//
// Parameters:
// - fsys: The filesystem to use for path cleaning
// - name: The path name to clean
//
// Returns:
// - A cleaned version of the path name
func CleanPath(fsys fs.FS, name string) string {
if cpfs, ok := fsys.(CleanPathFS); ok {
return cpfs.CleanPath(name)
}
return path.Clean(name)
}I have a similar helper function for opening files that cleans the paths first. // Open opens a file using the provided fs.FS implementation, automatically
// cleaning the path before opening.
//
// This is a convenience wrapper that:
// 1. Cleans the path using CleanPath
// 2. Opens the file using the filesystem's Open method
//
// If the filesystem implements CleanPathFS, its custom path cleaning will be used.
// Otherwise, standard path cleaning is applied.
//
// Parameters:
// - fsys: The filesystem implementation to use
// - name: The name of the file to open
//
// Returns:
// - An fs.File interface if successful
// - An error if the file cannot be opened or if the path is invalid
func Open(fsys fs.FS, name string) (fs.File, error) {
name = CleanPath(fsys, name)
return fsys.Open(name)
}I define an OSFS that conforms to the various // OSFS is a global instance of osFS that provides a standardized io/fs-compatible interface
// to the operating system's filesystem. This allows code written against the io/fs interfaces
// to seamlessly work with the local filesystem, enabling better interoperability between
// filesystem abstractions and the actual OS filesystem.
var OSFS osFS = osFS{}
// osFS implements the standard io/fs interfaces as a thin wrapper around the os package.
// It serves as an adapter between the abstract io/fs interfaces and the concrete OS filesystem,
// allowing OS filesystem operations to be used anywhere that expects io/fs interfaces.
//
// This enables code to be written against the abstract fs interfaces while still maintaining
// the ability to work with the local filesystem, promoting better code reuse and testing.
type osFS struct{}
// Open opens the named file for reading. If successful, methods on the returned
// file can be used for reading; the associated file descriptor has mode O_RDONLY.
func (o osFS) Open(name string) (fs.File, error) {
name = o.CleanPath(name)
return os.Open(name)
}
// ReadFile reads the named file and returns its contents.
// A successful call returns a nil error, not io.EOF.
func (o osFS) ReadFile(name string) ([]byte, error) {
name = o.CleanPath(name)
return os.ReadFile(name)
}
// Stat returns a FileInfo describing the named file.
// If there is an error, it will be of type *PathError.
func (o osFS) Stat(name string) (fs.FileInfo, error) {
name = o.CleanPath(name)
return os.Stat(name)
}
// ReadDir reads the named directory and returns a list of directory entries.
// The entries are sorted by filename.
func (o osFS) ReadDir(name string) ([]fs.DirEntry, error) {
name = o.CleanPath(name)
return os.ReadDir(name)
}
// Glob returns the names of all files matching pattern according to the rules
// used by filepath.Match. The pattern may describe hierarchical names such as
// usr/*/bin/ed.
func (o osFS) Glob(pattern string) ([]string, error) {
return filepath.Glob(pattern)
}Then for my osFS implementation, on Windows, I normalize all paths to Windows Extended Length Paths, which // CleanPath normalizes file paths on Windows by converting them to Windows Extended Length Path format (\\?\).
//
// It handles several path formats:
// - Already extended paths (\\?\) are returned as-is
// - Device paths (\\.\) are converted to \\?\
// - UNC paths (\\server\share) are converted to \\?\UNC\server\share
// - Relative or local absolute paths are converted to their absolute extended form
//
// If path conversion fails, it returns the cleaned original path using filepath.Clean.
//
// Extended-length paths allow Windows API to handle paths longer than 260 characters (MAX_PATH).
// For more information about extended-length paths, see:
// https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#maximum-path-length-limitation
//
// Parameters:
// - name: The file path to normalize
//
// Returns:
// - string: The normalized path in Windows Extended Length Path format
func (osFS) CleanPath(name string) string {
// Normalize everything to a Windows Extended Length Path (\\?\)
name = filepath.FromSlash(name)
// If the path is already an extended length path, return it
if strings.HasPrefix(name, `\\?\`) {
return name
}
// Handle Device Paths
if strings.HasPrefix(name, `\\.\`) {
return `\\?\` + name[4:]
}
// Handle UNC Paths
if strings.HasPrefix(name, `\\`) {
return `\\?\UNC\` + name[2:]
}
// Handle relative or local absolute paths
if abs, err := filepath.Abs(name); err == nil {
return `\\?\` + abs
}
// If all else fails, just clean and return the original path
return filepath.Clean(name)
} |
I wrote a little package called cdup that walks a directory and each of its ancestors looking for a file/dir. This is useful e.g. to find the root of a git dir (look for
.git) or a Go module (look forgo.mod).I aimed to write it using
fs.FSand useos.DirFSas a bridge to the OS filesystem as needed, perhaps inside a wrapper function.I may just be holding it wrong, but I encountered a few surprising bumps.
A typical invocation is
cdup.Find(pwd, ".git"). The obvious but wrong translation of this iscdup.FindIn(os.DirFS("/"), pwd, ".git").What is the correct root path to pass to
os.DirFSon Windows?filepath.VolumeNameseems like the right API to use. (I think?)Then we must convert
pwdto a relative path usingfilepath.Rel. On Windows, we also need to callfilepath.ToSlash.So I believe that the correct code is something like:
This is a surprising amount of non-obvious work for what seems like it should be a trivial bridge between fs.FS and the OS filesystem.
I'm not sure exactly what the fix should be here. Docs/examples? New package filepath API to get the OS fs root for a path? New package os API to construct a root filesystem (and expose the root for use with filepath.Rel)?
The text was updated successfully, but these errors were encountered: