diff --git a/internal/fs/fs.go b/internal/fs/fs.go index b33fd0f4c4..84f0346856 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -9,7 +9,9 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "strings" + "syscall" "unicode" "github.com/pkg/errors" @@ -270,10 +272,25 @@ func CopyDir(src, dst string) error { // 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 + return errors.Wrap(err, "symlink check failed") } else if sym { - err := copySymlink(src, dst) - return err + if err := cloneSymlink(src, dst); err != nil { + if runtime.GOOS == "windows" { + // If cloning the symlink fails on Windows because the user + // does not have the required privileges, ignore the error and + // fall back to copying the file contents. + // + // ERROR_PRIVILEGE_NOT_HELD is 1314 (0x522): + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms681385(v=vs.85).aspx + if lerr, ok := err.(*os.LinkError); ok && lerr.Err != syscall.Errno(1314) { + return err + } + } else { + return err + } + } else { + return nil + } } in, err := os.Open(src) @@ -286,19 +303,13 @@ func copyFile(src, dst string) (err error) { if err != nil { return } - defer func() { - if e := out.Close(); e != nil { - err = e - } - }() + defer out.Close() - _, err = io.Copy(out, in) - if err != nil { + if _, err = io.Copy(out, in); err != nil { return } - err = out.Sync() - if err != nil { + if err = out.Sync(); err != nil { return } @@ -306,28 +317,21 @@ func copyFile(src, dst string) (err error) { if err != nil { return } + err = os.Chmod(dst, si.Mode()) - if err != nil { - return - } 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) +// cloneSymlink will create a new symlink that points to the resolved path of sl. +// If sl is a relative symlink, dst will also be a relative symlink. +func cloneSymlink(sl, dst string) error { + resolved, err := os.Readlink(sl) if err != nil { - return errors.Wrap(err, "failed to resolve symlink") - } - - err = os.Symlink(resolved, dst) - if err != nil { - return errors.Wrapf(err, "failed to create symlink %s to %s", src, resolved) + return err } - return nil + return os.Symlink(resolved, dst) } // IsDir determines is the path given is a directory or not. diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go index 33e560ac3e..163b26e647 100644 --- a/internal/fs/fs_test.go +++ b/internal/fs/fs_test.go @@ -499,71 +499,51 @@ 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) - } + h := test.NewHelper(t) + defer h.Cleanup() + h.TempDir(".") - if err = os.Symlink(srcPath, symlinkPath); err != nil { - t.Fatalf("could not create symlink: %v", err) + testcases := map[string]string{ + filepath.Join("./testdata/symlinks/file-symlink"): filepath.Join(h.Path("."), "dst-file"), + filepath.Join("./testdata/symlinks/windows-file-symlink"): filepath.Join(h.Path("."), "windows-dst-file"), + filepath.Join("./testdata/symlinks/dir-symlink"): filepath.Join(h.Path("."), "dst-dir"), + filepath.Join("./testdata/symlinks/invalid-symlink"): filepath.Join(h.Path("."), "invalid-symlink"), } - if err = copyFile(symlinkPath, dstPath); err != nil { - t.Fatalf("failed to copy symlink: %s", err) - } + for symlink, dst := range testcases { + t.Run(symlink, func(t *testing.T) { + var err error + if err = copyFile(symlink, dst); 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) - } + var want, got string + + if runtime.GOOS == "windows" { + // Creating symlinks on Windows require an additional permission + // regular users aren't granted usually. So we copy the file + // content as a fall back instead of creating a real symlink. + srcb, err := ioutil.ReadFile(symlink) + h.Must(err) + dstb, err := ioutil.ReadFile(dst) + h.Must(err) + + want = string(srcb) + got = string(dstb) + } else { + want, err = os.Readlink(symlink) + h.Must(err) + + got, err = os.Readlink(dst) + 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) + if want != got { + t.Fatalf("resolved path is incorrect. expected %s, got %s", want, got) + } + }) } } @@ -814,13 +794,6 @@ func TestIsNonEmptyDir(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) @@ -841,6 +814,7 @@ func TestIsSymlink(t *testing.T) { dirSymlink := filepath.Join(dir, "dirSymlink") fileSymlink := filepath.Join(dir, "fileSymlink") + if err = os.Symlink(dirPath, dirSymlink); err != nil { t.Fatal(err) } @@ -866,10 +840,7 @@ func TestIsSymlink(t *testing.T) { }) defer cleanup() - tests := map[string]struct { - expected bool - err bool - }{ + tests := map[string]struct{ expected, err bool }{ dirPath: {false, false}, filePath: {false, false}, dirSymlink: {true, false}, @@ -878,6 +849,13 @@ func TestIsSymlink(t *testing.T) { inaccessibleSymlink: {false, true}, } + if runtime.GOOS == "windows" { + // XXX: setting permissions works differently in Windows. Skipping + // these cases until a compatible implementation is provided. + delete(tests, inaccessibleFile) + delete(tests, inaccessibleSymlink) + } + for path, want := range tests { got, err := IsSymlink(path) if err != nil { diff --git a/internal/fs/testdata/symlinks/dir-symlink b/internal/fs/testdata/symlinks/dir-symlink new file mode 120000 index 0000000000..777ebd014e --- /dev/null +++ b/internal/fs/testdata/symlinks/dir-symlink @@ -0,0 +1 @@ +../../testdata \ No newline at end of file diff --git a/internal/fs/testdata/symlinks/file-symlink b/internal/fs/testdata/symlinks/file-symlink new file mode 120000 index 0000000000..4c52274de0 --- /dev/null +++ b/internal/fs/testdata/symlinks/file-symlink @@ -0,0 +1 @@ +../test.file \ No newline at end of file diff --git a/internal/fs/testdata/symlinks/invalid-symlink b/internal/fs/testdata/symlinks/invalid-symlink new file mode 120000 index 0000000000..0edf4f301b --- /dev/null +++ b/internal/fs/testdata/symlinks/invalid-symlink @@ -0,0 +1 @@ +/non/existing/file \ No newline at end of file diff --git a/internal/fs/testdata/symlinks/windows-file-symlink b/internal/fs/testdata/symlinks/windows-file-symlink new file mode 120000 index 0000000000..af1d6c8f57 --- /dev/null +++ b/internal/fs/testdata/symlinks/windows-file-symlink @@ -0,0 +1 @@ +C:/Users/ibrahim/go/src/github.com/golang/dep/internal/fs/testdata/test.file \ No newline at end of file