Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

internal/fs: handle symlinks in copyFile() #657

Merged
merged 8 commits into from
May 30, 2017
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions internal/fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,13 @@ func CopyDir(src, dst string) error {
// of the source file. The file mode will be copied from the source and
// the copied data is synced/flushed to stable storage.
func copyFile(src, dst string) (err error) {
if sym, err := IsSymlink(src); err != nil {
return err
} else if sym {
err := copySymlink(src, dst)
return err
}

in, err := os.Open(src)
if err != nil {
return
Expand Down Expand Up @@ -280,6 +287,19 @@ func copyFile(src, dst string) (err error) {
return
}

// copySymlink will resolve the src symlink and create a new symlink in dst.
// If src is a relative symlink, dst will also be a relative symlink.
func copySymlink(src, dst string) error {
resolved, err := os.Readlink(src)
if err != nil {
return errors.Wrap(err, "failed to resolve symlink")
}

err = os.Symlink(resolved, dst)

return errors.Wrapf(err, "failed to create symlink %s to %s", src, resolved)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awkwardly halfway between a terse form and the standard form. Either combine this and the preceding line into one (errors.Wrapf(os.Symlink(resolved, dst), ...)), or do the more standard if err != nil check.

}

// IsDir determines is the path given is a directory or not.
func IsDir(name string) (bool, error) {
// TODO: lstat?
Expand Down Expand Up @@ -337,3 +357,13 @@ func IsRegular(name string) (bool, error) {
}
return true, nil
}

// IsSymlink determines if the given path is a symbolic link.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a note to describe the behaviour of this function on Windows.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should behave in the same way. os.Lstat() call GetFileAttributesEx on Windows and FileStat.Mode() will convert the attribute syscall.FILE_ATTRIBUTE_REPARSE_POINT to os.ModeSymlink. (see os/stat_windows.go and os/types_windows.go).

Unless I'm missing something else. 😄

Copy link
Collaborator Author

@ibrasho ibrasho May 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why they the tests for this function are skipped is that creating a symlink (using os.Symlink()) is not supported on Windows. But if the symlink already exists, it will be detected by this function.

func IsSymlink(path string) (bool, error) {
l, err := os.Lstat(path)
if err != nil {
return false, err
}

return l.Mode()&os.ModeSymlink == os.ModeSymlink, nil
}
153 changes: 152 additions & 1 deletion internal/fs/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,75 @@ func TestCopyFile(t *testing.T) {
}
}

func TestCopyFileSymlink(t *testing.T) {
dir, err := ioutil.TempDir("", "dep")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

srcPath := filepath.Join(dir, "src")
symlinkPath := filepath.Join(dir, "symlink")
dstPath := filepath.Join(dir, "dst")

srcf, err := os.Create(srcPath)
if err != nil {
t.Fatal(err)
}
srcf.Close()

if err = os.Symlink(srcPath, symlinkPath); err != nil {
t.Fatalf("could not create symlink: %s", err)
}

if err = copyFile(symlinkPath, dstPath); err != nil {
t.Fatalf("failed to copy symlink: %s", err)
}

resolvedPath, err := os.Readlink(dstPath)
if err != nil {
t.Fatalf("could not resolve symlink: %s", err)
}

if resolvedPath != srcPath {
t.Fatalf("resolved path is incorrect. expected %s, got %s", srcPath, resolvedPath)
}
}

func TestCopyFileSymlinkToDirectory(t *testing.T) {
dir, err := ioutil.TempDir("", "dep")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

srcPath := filepath.Join(dir, "src")
symlinkPath := filepath.Join(dir, "symlink")
dstPath := filepath.Join(dir, "dst")

err = os.MkdirAll(srcPath, 0777)
if err != nil {
t.Fatal(err)
}

if err = os.Symlink(srcPath, symlinkPath); err != nil {
t.Fatalf("could not create symlink: %v", err)
}

if err = copyFile(symlinkPath, dstPath); err != nil {
t.Fatalf("failed to copy symlink: %s", err)
}

resolvedPath, err := os.Readlink(dstPath)
if err != nil {
t.Fatalf("could not resolve symlink: %s", err)
}

if resolvedPath != srcPath {
t.Fatalf("resolved path is incorrect. expected %s, got %s", srcPath, resolvedPath)
}
}

func TestCopyFileFail(t *testing.T) {
if runtime.GOOS == "windows" {
// XXX: setting permissions works differently in
Expand Down Expand Up @@ -659,7 +728,6 @@ func TestIsDir(t *testing.T) {
t.Fatalf("expected %t for %s, got %t", want.exists, f, got)
}
}

}

func TestIsEmpty(t *testing.T) {
Expand Down Expand Up @@ -702,3 +770,86 @@ func TestIsEmpty(t *testing.T) {
}
}
}

func TestIsSymlink(t *testing.T) {
if runtime.GOOS == "windows" {
// XXX: creating symlinks is not supported in Go on
// Microsoft Windows. Skipping this this until a solution
// for creating symlinks is is provided.
t.Skip("skipping on windows")
}

dir, err := ioutil.TempDir("", "dep")
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

dirPath := filepath.Join(dir, "directory")
if err = os.MkdirAll(dirPath, 0777); err != nil {
t.Fatal(err)
}

filePath := filepath.Join(dir, "file")
f, err := os.Create(filePath)
if err != nil {
t.Fatal(err)
}
f.Close()

dirSymlink := filepath.Join(dir, "dirSymlink")
fileSymlink := filepath.Join(dir, "fileSymlink")
if err = os.Symlink(dirPath, dirSymlink); err != nil {
t.Fatal(err)
}
if err = os.Symlink(filePath, fileSymlink); err != nil {
t.Fatal(err)
}

var (
inaccessibleFile string
inaccessibleSymlink string
)

err, cleanup := setupInaccessibleDir(func(dir string) error {
inaccessibleFile = filepath.Join(dir, "file")
if fh, err := os.Create(inaccessibleFile); err != nil {
return err
} else if err = fh.Close(); err != nil {
return err
}

inaccessibleSymlink = filepath.Join(dir, "symlink")
err = os.Symlink(inaccessibleFile, inaccessibleSymlink)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just return this final call directly, no need to assign into var

return err
})
defer cleanup()
if err != nil {
t.Fatal(err)
}

tests := map[string]struct {
expected bool
err bool
}{
dirPath: {false, false},
filePath: {false, false},
dirSymlink: {true, false},
fileSymlink: {true, false},
inaccessibleFile: {false, true},
inaccessibleSymlink: {false, true},
}

for path, want := range tests {
got, err := IsSymlink(path)
if err != nil {
if !want.err {
t.Errorf("expected no error, got %v", err)
}
}

if got != want.expected {
t.Errorf("expected %t for %s, got %t", want.expected, path, got)
}
}
}