Repository: spf13/afero
Pull request #228 (2019-12-11 09:06:11)

Add optional interfaces for Symlink and Readlink
I created two interfaces to facilitate file systems optionally supporting the Symlink and Readlink operations from the os implementation.

```
type Linker interface {
	SymlinkIfPossible(oldname, newname string) error
}
type LinkReader interface {
	ReadlinkIfPossible(name string) (string, error)
}
```

I opted to write them this way to preserve consistency with the Lstater interface created by @bep 

The actual Symlink and Readlink are only implemented in the `OsFs`, with proxy support in:
 - BasePathFs,
 - CopyOnWriteFs, and
 - ReadOnlyFs

for consistency with the other two I implemented the Linker interface on `ReadOnlyFs` with it just returning a not supported error. I would appreciate feedback if this is the correct pattern, or if I should instead remove the method?

The motivation for this change was to allow wrapping an existing use of afero in a project with the [go-billy](https://github.com/src-d/go-billy) interface to allow a git integration to support a client request.
The implementation is in the process of being tested in our logic, but I wanted to create the pull request and get feedback and make any necessary changes early.

claassistantio: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/spf13/afero?pullRequest=228) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/spf13/afero?pullRequest=228) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/spf13/afero?pullRequest=228) it.</sub>
Maldris: ok, got sidetracked with some family things and then assigned to a different project, but I've had a chance to come back to this.

I made use of this branch to make a wrapper for compatibility with [https://github.com/src-d/go-git](https://github.com/src-d/go-git).
This implementation can be found at [https://github.com/Maldris/go-billy-afero](https://github.com/Maldris/go-billy-afero).

Go-git is used in an application we have which otherwise uses afero for other operations. After writing the wrapper we wrote a full test that sequestially tested the core git operations one after another, including its use of symlink and readLink.

other than some issues with the tests themselves, this appears to work without issue on windows, ubuntu and an alpine based docker container (I don't have access to a darwin system to test).

I do have some stylistic questions prior to requesting a final merge howerver:
For consistency with the os package's behaviour of Symlink in which the returned error is always a `os.LinkError`. I've returned a `os.LinkError` wrapping the "symlink not supported" error. Should I instead be returning the base error so it may be tested against, instead of requiring the user to unwrap the error? or is constancy with the os package behaviour better in this case?
kargakis: @Maldris consistency with the os package is nice to have.
CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/signed)](https://cla-assistant.io/spf13/afero?pullRequest=228) <br/>All committers have signed the CLA.
Maldris: In that case, are there any changes (functional or stylistic) you would advise before this can be merged?

If not, I'll rebase the branch against master and it should be good to go.
kargakis: @Maldris your changes lgtm, please rebase and I'll merge.

Patch:
diff --git a/basepath.go b/basepath.go
index 616ff8f..3a14b83 100644
--- a/basepath.go
+++ b/basepath.go
@@ -177,4 +177,30 @@ func (b *BasePathFs) LstatIfPossible(name string) (os.FileInfo, bool, error) {
 	return fi, false, err
 }
 
+func (b *BasePathFs) SymlinkIfPossible(oldname, newname string) error {
+	oldname, err := b.RealPath(oldname)
+	if err != nil {
+		return &os.LinkError{Op: "symlink", Old: oldname, New: newname, Err: err}
+	}
+	newname, err = b.RealPath(newname)
+	if err != nil {
+		return &os.LinkError{Op: "symlink", Old: oldname, New: newname, Err: err}
+	}
+	if linker, ok := b.source.(Linker); ok {
+		return linker.SymlinkIfPossible(oldname, newname)
+	}
+	return &os.LinkError{Op: "symlink", Old: oldname, New: newname, Err: ErrNoSymlink}
+}
+
+func (b *BasePathFs) ReadlinkIfPossible(name string) (string, error) {
+	name, err := b.RealPath(name)
+	if err != nil {
+		return "", &os.PathError{Op: "readlink", Path: name, Err: err}
+	}
+	if reader, ok := b.source.(LinkReader); ok {
+		return reader.ReadlinkIfPossible(name)
+	}
+	return "", &os.PathError{Op: "readlink", Path: name, Err: ErrNoReadlink}
+}
+
 // vim: ts=4 sw=4 noexpandtab nolist syn=go
diff --git a/copyOnWriteFs.go b/copyOnWriteFs.go
index e8108a8..96b7701 100644
--- a/copyOnWriteFs.go
+++ b/copyOnWriteFs.go
@@ -117,6 +117,26 @@ func (u *CopyOnWriteFs) LstatIfPossible(name string) (os.FileInfo, bool, error)
 	return fi, false, err
 }
 
+func (u *CopyOnWriteFs) SymlinkIfPossible(oldname, newname string) error {
+	if slayer, ok := u.layer.(Linker); ok {
+		return slayer.SymlinkIfPossible(oldname, newname)
+	}
+
+	return &os.LinkError{Op: "symlink", Old: oldname, New: newname, Err: ErrNoSymlink}
+}
+
+func (u *CopyOnWriteFs) ReadlinkIfPossible(name string) (string, error) {
+	if rlayer, ok := u.layer.(LinkReader); ok {
+		return rlayer.ReadlinkIfPossible(name)
+	}
+
+	if rbase, ok := u.base.(LinkReader); ok {
+		return rbase.ReadlinkIfPossible(name)
+	}
+
+	return "", &os.PathError{Op: "readlink", Path: name, Err: ErrNoReadlink}
+}
+
 func (u *CopyOnWriteFs) isNotExist(err error) bool {
 	if e, ok := err.(*os.PathError); ok {
 		err = e.Err
diff --git a/os.go b/os.go
index 13cc1b8..4761db5 100644
--- a/os.go
+++ b/os.go
@@ -99,3 +99,11 @@ func (OsFs) LstatIfPossible(name string) (os.FileInfo, bool, error) {
 	fi, err := os.Lstat(name)
 	return fi, true, err
 }
+
+func (OsFs) SymlinkIfPossible(oldname, newname string) error {
+	return os.Symlink(oldname, newname)
+}
+
+func (OsFs) ReadlinkIfPossible(name string) (string, error) {
+	return os.Readlink(name)
+}
diff --git a/readonlyfs.go b/readonlyfs.go
index c6376ec..f94b181 100644
--- a/readonlyfs.go
+++ b/readonlyfs.go
@@ -44,6 +44,18 @@ func (r *ReadOnlyFs) LstatIfPossible(name string) (os.FileInfo, bool, error) {
 	return fi, false, err
 }
 
+func (r *ReadOnlyFs) SymlinkIfPossible(oldname, newname string) error {
+	return &os.LinkError{Op: "symlink", Old: oldname, New: newname, Err: ErrNoSymlink}
+}
+
+func (r *ReadOnlyFs) ReadlinkIfPossible(name string) (string, error) {
+	if srdr, ok := r.source.(LinkReader); ok {
+		return srdr.ReadlinkIfPossible(name)
+	}
+
+	return "", &os.PathError{Op: "readlink", Path: name, Err: ErrNoReadlink}
+}
+
 func (r *ReadOnlyFs) Rename(o, n string) error {
 	return syscall.EPERM
 }
diff --git a/symlink.go b/symlink.go
new file mode 100644
index 0000000..d1c6ea5
--- /dev/null
+++ b/symlink.go
@@ -0,0 +1,55 @@
+// Copyright © 2018 Steve Francia <spf@spf13.com>.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package afero
+
+import (
+	"errors"
+)
+
+// Symlinker is an optional interface in Afero. It is only implemented by the
+// filesystems saying so.
+// It indicates support for 3 symlink related interfaces that implement the
+// behaviors of the os methods:
+//    - Lstat
+//    - Symlink, and
+//    - Readlink
+type Symlinker interface {
+	Lstater
+	Linker
+	LinkReader
+}
+
+// Linker is an optional interface in Afero. It is only implemented by the
+// filesystems saying so.
+// It will call Symlink if the filesystem itself is, or it delegates to, the os filesystem,
+// or the filesystem otherwise supports Symlink's.
+type Linker interface {
+	SymlinkIfPossible(oldname, newname string) error
+}
+
+// ErrNoSymlink is the error that will be wrapped in an os.LinkError if a file system
+// does not support Symlink's either directly or through its delegated filesystem.
+// As expressed by support for the Linker interface.
+var ErrNoSymlink = errors.New("symlink not supported")
+
+// LinkReader is an optional interface in Afero. It is only implemented by the
+// filesystems saying so.
+type LinkReader interface {
+	ReadlinkIfPossible(name string) (string, error)
+}
+
+// ErrNoReadlink is the error that will be wrapped in an os.Path if a file system
+// does not support the readlink operation either directly or through its delegated filesystem.
+// As expressed by support for the LinkReader interface.
+var ErrNoReadlink = errors.New("readlink not supported")


Test patch:
diff --git a/symlink_test.go b/symlink_test.go
new file mode 100644
index 0000000..5a9e5db
--- /dev/null
+++ b/symlink_test.go
@@ -0,0 +1,160 @@
+package afero
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func TestSymlinkIfPossible(t *testing.T) {
+	wd, _ := os.Getwd()
+	defer func() {
+		os.Chdir(wd)
+	}()
+
+	osFs := &OsFs{}
+
+	workDir, err := TempDir(osFs, "", "afero-symlink")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() {
+		osFs.RemoveAll(workDir)
+	}()
+
+	memWorkDir := "/sym"
+
+	memFs := NewMemMapFs()
+	overlayFs1 := &CopyOnWriteFs{base: osFs, layer: memFs}
+	overlayFs2 := &CopyOnWriteFs{base: memFs, layer: osFs}
+	overlayFsMemOnly := &CopyOnWriteFs{base: memFs, layer: NewMemMapFs()}
+	basePathFs := &BasePathFs{source: osFs, path: workDir}
+	basePathFsMem := &BasePathFs{source: memFs, path: memWorkDir}
+	roFs := &ReadOnlyFs{source: osFs}
+	roFsMem := &ReadOnlyFs{source: memFs}
+
+	pathFileMem := filepath.Join(memWorkDir, "aferom.txt")
+	osPath := filepath.Join(workDir, "afero.txt")
+
+	WriteFile(osFs, osPath, []byte("Hi, Afero!"), 0777)
+	WriteFile(memFs, filepath.Join(pathFileMem), []byte("Hi, Afero!"), 0777)
+
+	testLink := func(l Linker, source, destination string, output *string) {
+		if fs, ok := l.(Fs); ok {
+			dir := filepath.Dir(destination)
+			if dir != "" {
+				fs.MkdirAll(dir, 0777)
+			}
+		}
+
+		err := l.SymlinkIfPossible(source, destination)
+		if (err == nil) && (output != nil) {
+			t.Fatalf("Error creating symlink, succeeded when expecting error %v", *output)
+		} else if (err != nil) && (output == nil) {
+			t.Fatalf("Error creating symlink, expected success, got %v", err)
+		} else if err != nil && err.Error() != *output && !strings.HasSuffix(err.Error(), *output) {
+			t.Fatalf("Error creating symlink, expected error '%v', instead got output '%v'", *output, err)
+		} else {
+			// test passed, if expecting a successful link, check the link with lstat if able
+			if output == nil {
+				if lst, ok := l.(Lstater); ok {
+					_, ok, err := lst.LstatIfPossible(destination)
+					if !ok {
+						if err != nil {
+							t.Fatalf("Error calling lstat on file after successful link, got: %v", err)
+						} else {
+							t.Fatalf("Error calling lstat on file after successful link, result didn't use lstat (not link)")
+						}
+						return
+					}
+				}
+			}
+		}
+	}
+
+	notSupported := ErrNoSymlink.Error()
+
+	testLink(osFs, osPath, filepath.Join(workDir, "os/link.txt"), nil)
+	testLink(overlayFs1, osPath, filepath.Join(workDir, "overlay/link1.txt"), &notSupported)
+	testLink(overlayFs2, pathFileMem, filepath.Join(workDir, "overlay2/link2.txt"), nil)
+	testLink(overlayFsMemOnly, pathFileMem, filepath.Join(memWorkDir, "overlay3/link.txt"), &notSupported)
+	testLink(basePathFs, "afero.txt", "basepath/link.txt", nil)
+	testLink(basePathFsMem, pathFileMem, "link/file.txt", &notSupported)
+	testLink(roFs, osPath, filepath.Join(workDir, "ro/link.txt"), &notSupported)
+	testLink(roFsMem, pathFileMem, filepath.Join(memWorkDir, "ro/link.txt"), &notSupported)
+}
+
+func TestReadlinkIfPossible(t *testing.T) {
+	wd, _ := os.Getwd()
+	defer func() {
+		os.Chdir(wd)
+	}()
+
+	osFs := &OsFs{}
+
+	workDir, err := TempDir(osFs, "", "afero-readlink")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer func() {
+		osFs.RemoveAll(workDir)
+	}()
+
+	memWorkDir := "/read"
+
+	memFs := NewMemMapFs()
+	overlayFs1 := &CopyOnWriteFs{base: osFs, layer: memFs}
+	overlayFs2 := &CopyOnWriteFs{base: memFs, layer: osFs}
+	overlayFsMemOnly := &CopyOnWriteFs{base: memFs, layer: NewMemMapFs()}
+	basePathFs := &BasePathFs{source: osFs, path: workDir}
+	basePathFsMem := &BasePathFs{source: memFs, path: memWorkDir}
+	roFs := &ReadOnlyFs{source: osFs}
+	roFsMem := &ReadOnlyFs{source: memFs}
+
+	pathFileMem := filepath.Join(memWorkDir, "aferom.txt")
+	osPath := filepath.Join(workDir, "afero.txt")
+
+	WriteFile(osFs, osPath, []byte("Hi, Afero!"), 0777)
+	WriteFile(memFs, filepath.Join(pathFileMem), []byte("Hi, Afero!"), 0777)
+
+	createLink := func(l Linker, source, destination string) error {
+		if fs, ok := l.(Fs); ok {
+			dir := filepath.Dir(destination)
+			if dir != "" {
+				fs.MkdirAll(dir, 0777)
+			}
+		}
+
+		return l.SymlinkIfPossible(source, destination)
+	}
+
+	testRead := func(r LinkReader, name string, output *string) {
+		_, err := r.ReadlinkIfPossible(name)
+		if (err != nil) && (output == nil) {
+			t.Fatalf("Error reading link, expected success, got error: %v", err)
+		} else if (err == nil) && (output != nil) {
+			t.Fatalf("Error reading link, succeeded when expecting error: %v", *output)
+		} else if err != nil && err.Error() != *output && !strings.HasSuffix(err.Error(), *output) {
+			t.Fatalf("Error reading link, expected error '%v', instead received '%v'", *output, err)
+		}
+	}
+
+	notSupported := ErrNoReadlink.Error()
+
+	err = createLink(osFs, osPath, filepath.Join(workDir, "os/link.txt"))
+	if err != nil {
+		t.Fatal("Error creating test link: ", err)
+	}
+
+	testRead(osFs, filepath.Join(workDir, "os/link.txt"), nil)
+	testRead(overlayFs1, filepath.Join(workDir, "os/link.txt"), nil)
+	testRead(overlayFs2, filepath.Join(workDir, "os/link.txt"), nil)
+	testRead(overlayFsMemOnly, pathFileMem, &notSupported)
+	testRead(basePathFs, "os/link.txt", nil)
+	testRead(basePathFsMem, pathFileMem, &notSupported)
+	testRead(roFs, filepath.Join(workDir, "os/link.txt"), nil)
+	testRead(roFsMem, pathFileMem, &notSupported)
+}