internal/fs: update TestCopyFileSymlink and rename copySymlink

This change updates TestCopyFileSymlink tests and renames copySymlink to
cloneSymlink to clarify the intention.

Fixes golang#773

Signed-off-by: Ibrahim AshShohail <[email protected]>

internal/fs/fs.go

@@ -272,8 +272,7 @@ 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
+		return cloneSymlink(src, dst)
 	}
 
 	in, err := os.Open(src)
@@ -314,17 +313,17 @@ 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)
+// 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 errors.Wrapf(err, "failed to create symlink %s to %s", dst, resolved)
 	}
 
 	return nil

internal/fs/fs_test.go

@@ -499,71 +499,48 @@ 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)
-	}
-}
+	h := test.NewHelper(t)
+	defer h.Cleanup()
+	h.TempDir(".")
 
-func TestCopyFileSymlinkToDirectory(t *testing.T) {
-	dir, err := ioutil.TempDir("", "dep")
-	if err != nil {
-		t.Fatal(err)
+	testcases := map[string]string{
+		filepath.Join("./testdata/symlinks/file-symlink"):    filepath.Join(h.Path("."), "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"),
 	}
-	defer os.RemoveAll(dir)
 
-	srcPath := filepath.Join(dir, "src")
-	symlinkPath := filepath.Join(dir, "symlink")
-	dstPath := filepath.Join(dir, "dst")
+	for symlink, dst := range testcases {
+		var err error
+		if err = copyFile(symlink, dst); err != nil {
+			t.Fatalf("failed to copy symlink: %s", err)
+		}
 
-	err = os.MkdirAll(srcPath, 0777)
-	if err != nil {
-		t.Fatal(err)
-	}
+		var want, got string
 
-	if err = os.Symlink(srcPath, symlinkPath); err != nil {
-		t.Fatalf("could not create symlink: %v", err)
-	}
+		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)
 
-	if err = copyFile(symlinkPath, dstPath); err != nil {
-		t.Fatalf("failed to copy symlink: %s", err)
-	}
+			want = string(srcb)
+			got = string(dstb)
+		} else {
+			want, err = os.Readlink(symlink)
+			h.Must(err)
 
-	resolvedPath, err := os.Readlink(dstPath)
-	if err != nil {
-		t.Fatalf("could not resolve symlink: %s", 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)
+		}
 	}
 }
 
@@ -866,10 +843,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 +852,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 {

internal/fs/testdata/symlinks/dir-symlink

@@ -0,0 +1 @@
+../../testdata

internal/fs/testdata/symlinks/file-symlink

@@ -0,0 +1 @@
+../test.file

internal/fs/testdata/symlinks/invalid-symlink

@@ -0,0 +1 @@
+/non/existing/file