Make storage file writes atomic

controllers/helmchart_controller.go

@@ -17,8 +17,10 @@ limitations under the License.
 package controllers
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/url"
 	"os"
@@ -255,12 +257,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
 	}
 
-	chartBytes, err := ioutil.ReadAll(res)
-	if err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-	}
-
-	sum := r.Storage.Checksum(chartBytes)
+	var buf bytes.Buffer
+	sum := r.Storage.Checksum(io.TeeReader(res, &buf))
 	artifact := r.Storage.ArtifactFor(chart.Kind, chart.GetObjectMeta(),
 		fmt.Sprintf("%s-%s-%s.tgz", cv.Name, cv.Version, sum), cv.Version, sum)
 
@@ -280,7 +278,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	defer unlock()
 
 	// save artifact to storage
-	err = r.Storage.WriteFile(artifact, chartBytes)
+	err = r.Storage.AtomicWriteFile(artifact, &buf, 0644)
 	if err != nil {
 		err = fmt.Errorf("unable to write chart file: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err

controllers/helmrepository_controller.go

@@ -17,6 +17,7 @@ limitations under the License.
 package controllers
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"io/ioutil"
@@ -212,23 +213,23 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
 	}
 
-	data, err := ioutil.ReadAll(res)
+	b, err := ioutil.ReadAll(res)
 	if err != nil {
 		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
 	}
 
-	i := &repo.IndexFile{}
-	if err := yaml.Unmarshal(data, i); err != nil {
+	i := repo.IndexFile{}
+	if err := yaml.Unmarshal(b, &i); err != nil {
 		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
 	}
 	i.SortEntries()
 
-	index, err := yaml.Marshal(i)
+	b, err = yaml.Marshal(&i)
 	if err != nil {
 		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
 	}
 
-	sum := r.Storage.Checksum(index)
+	sum := r.Storage.Checksum(bytes.NewReader(b))
 	artifact := r.Storage.ArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(),
 		fmt.Sprintf("index-%s.yaml", sum), i.Generated.Format(time.RFC3339Nano), sum)
 
@@ -248,7 +249,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 	defer unlock()
 
 	// save artifact to storage
-	err = r.Storage.WriteFile(artifact, index)
+	err = r.Storage.AtomicWriteFile(artifact, bytes.NewReader(b), 0644)
 	if err != nil {
 		err = fmt.Errorf("unable to write repository index file: %w", err)
 		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err

controllers/helmrepository_controller_test.go

@@ -140,7 +140,7 @@ var _ = Describe("HelmRepositoryReconciler", func() {
 			Eventually(func() error {
 				r := &sourcev1.HelmRepository{}
 				return k8sClient.Get(context.Background(), key, r)
-			}).ShouldNot(Succeed())
+			}, timeout, interval).ShouldNot(Succeed())
 
 			exists := func(path string) bool {
 				// wait for tmp sync on macOS

controllers/storage.go

@@ -36,6 +36,7 @@ import (
 	"github.com/fluxcd/pkg/lockedfile"
 
 	sourcev1 "github.com/fluxcd/source-controller/api/v1alpha1"
+	"github.com/fluxcd/source-controller/internal/fs"
 )
 
 const (
@@ -95,7 +96,8 @@ func (s *Storage) RemoveAll(artifact sourcev1.Artifact) error {
 	return os.RemoveAll(dir)
 }
 
-// RemoveAllButCurrent removes all files for the given artifact base dir excluding the current one
+// RemoveAllButCurrent removes all files for the given v1alpha1.Artifact base dir,
+// excluding the current one.
 func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error {
 	localPath := s.LocalPath(artifact)
 	dir := filepath.Dir(localPath)
@@ -120,8 +122,8 @@ func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error {
 	return nil
 }
 
-// ArtifactExist returns a boolean indicating whether the artifact exists in storage and is a
-// regular file.
+// ArtifactExist returns a boolean indicating whether the v1alpha1.Artifact exists in storage
+// and is a regular file.
 func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool {
 	fi, err := os.Lstat(s.LocalPath(artifact))
 	if err != nil {
@@ -130,9 +132,10 @@ func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool {
 	return fi.Mode().IsRegular()
 }
 
-// Archive creates a tar.gz to the artifact path from the given dir excluding any VCS specific
-// files and directories, or any of the excludes defined in the excludeFiles.
-func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1.GitRepositorySpec) error {
+// Archive atomically creates a tar.gz to the v1alpha1.Artifact path from the given dir,
+// excluding any VCS specific files and directories, or any of the excludes defined in
+// the excludeFiles.
+func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1.GitRepositorySpec) (err error) {
 	if _, err := os.Stat(dir); err != nil {
 		return err
 	}
@@ -141,22 +144,53 @@ func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1.
 	if err != nil {
 		return err
 	}
-
 	matcher := gitignore.NewMatcher(ps)
 
-	gzFile, err := os.Create(s.LocalPath(artifact))
+	localPath := s.LocalPath(artifact)
+	tmpGzFile, err := ioutil.TempFile(filepath.Split(localPath))
 	if err != nil {
 		return err
 	}
-	defer gzFile.Close()
-
-	gw := gzip.NewWriter(gzFile)
-	defer gw.Close()
+	tmpName := tmpGzFile.Name()
+	defer func() {
+		if err != nil {
+			os.Remove(tmpName)
+		}
+	}()
 
+	gw := gzip.NewWriter(tmpGzFile)
 	tw := tar.NewWriter(gw)
-	defer tw.Close()
+	if err := writeToArchiveExcludeMatches(dir, matcher, tw); err != nil {
+		tw.Close()
+		gw.Close()
+		tmpGzFile.Close()
+		return err
+	}
 
-	return filepath.Walk(dir, func(p string, fi os.FileInfo, err error) error {
+	if err := tw.Close(); err != nil {
+		gw.Close()
+		tmpGzFile.Close()
+		return err
+	}
+	if err := gw.Close(); err != nil {
+		tmpGzFile.Close()
+		return err
+	}
+	if err := tmpGzFile.Close(); err != nil {
+		return err
+	}
+
+	if err := os.Chmod(tmpName, 0644); err != nil {
+		return err
+	}
+
+	return fs.RenameWithFallback(tmpName, localPath)
+}
+
+// writeToArchiveExcludeMatches walks over the given dir and writes any regular file that does
+// not match the given gitignore.Matcher.
+func writeToArchiveExcludeMatches(dir string, matcher gitignore.Matcher, writer *tar.Writer) error {
+	fn := func(p string, fi os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
@@ -187,33 +221,48 @@ func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1.
 		}
 		header.Name = relFilePath
 
-		if err := tw.WriteHeader(header); err != nil {
+		if err := writer.WriteHeader(header); err != nil {
 			return err
 		}
 
 		f, err := os.Open(p)
 		if err != nil {
+			f.Close()
 			return err
 		}
-		if _, err := io.Copy(tw, f); err != nil {
+		if _, err := io.Copy(writer, f); err != nil {
 			f.Close()
 			return err
 		}
 		return f.Close()
-	})
+	}
+	return filepath.Walk(dir, fn)
 }
 
-// WriteFile writes the given bytes to the artifact path if the checksum differs
-func (s *Storage) WriteFile(artifact sourcev1.Artifact, data []byte) error {
+// AtomicWriteFile atomically writes a file to the v1alpha1.Artifact Path.
+func (s *Storage) AtomicWriteFile(artifact sourcev1.Artifact, reader io.Reader, mode os.FileMode) (err error) {
 	localPath := s.LocalPath(artifact)
-	sum := s.Checksum(data)
-	if file, err := os.Stat(localPath); !os.IsNotExist(err) && !file.IsDir() {
-		if fb, err := ioutil.ReadFile(localPath); err == nil && sum == s.Checksum(fb) {
-			return nil
+	tmpFile, err := ioutil.TempFile(filepath.Split(localPath))
+	if err != nil {
+		return err
 	}
+	tmpName := tmpFile.Name()
+	defer func() {
+		if err != nil {
+			os.Remove(tmpName)
 		}
-
-	return ioutil.WriteFile(localPath, data, 0644)
+	}()
+	if _, err := io.Copy(tmpFile, reader); err != nil {
+		tmpFile.Close()
+		return err
+	}
+	if err := tmpFile.Close(); err != nil {
+		return err
+	}
+	if err := os.Chmod(tmpName, mode); err != nil {
+		return err
+	}
+	return fs.RenameWithFallback(tmpName, localPath)
 }
 
 // Symlink creates or updates a symbolic link for the given artifact
@@ -241,12 +290,14 @@ func (s *Storage) Symlink(artifact sourcev1.Artifact, linkName string) (string,
 	return url, nil
 }
 
-// Checksum returns the SHA1 checksum for the given bytes as a string
-func (s *Storage) Checksum(b []byte) string {
-	return fmt.Sprintf("%x", sha1.Sum(b))
+// Checksum returns the SHA1 checksum for the data of the given io.Reader as a string.
+func (s *Storage) Checksum(reader io.Reader) string {
+	h := sha1.New()
+	_, _ = io.Copy(h, reader)
+	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
-// Lock creates a file lock for the given artifact
+// Lock creates a file lock for the given v1alpha1.Artifact.
 func (s *Storage) Lock(artifact sourcev1.Artifact) (unlock func(), err error) {
 	lockFile := s.LocalPath(artifact) + ".lock"
 	mutex := lockedfile.MutexAt(lockFile)
@@ -262,6 +313,8 @@ func (s *Storage) LocalPath(artifact sourcev1.Artifact) string {
 	return filepath.Join(s.BasePath, artifact.Path)
 }
 
+// getPatterns collects ignore patterns from the given reader and returns them
+// as a gitignore.Pattern slice.
 func getPatterns(reader io.Reader, path []string) []gitignore.Pattern {
 	var ps []gitignore.Pattern
 	scanner := bufio.NewScanner(reader)

internal/fs/LICENSE

@@ -0,0 +1,27 @@
+Copyright (c) 2014 The Go Authors. All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+   * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+   * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+   * Neither the name of Google Inc. nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

internal/fs/fs.go

@@ -0,0 +1,346 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package fs
+
+import (
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"runtime"
+	"syscall"
+)
+
+// RenameWithFallback attempts to rename a file or directory, but falls back to
+// copying in the event of a cross-device link error. If the fallback copy
+// succeeds, src is still removed, emulating normal rename behavior.
+func RenameWithFallback(src, dst string) error {
+	_, err := os.Stat(src)
+	if err != nil {
+		return fmt.Errorf("cannot stat %s: %w", src, err)
+	}
+
+	err = os.Rename(src, dst)
+	if err == nil {
+		return nil
+	}
+
+	return renameFallback(err, src, dst)
+}
+
+// renameByCopy attempts to rename a file or directory by copying it to the
+// destination and then removing the src thus emulating the rename behavior.
+func renameByCopy(src, dst string) error {
+	var cerr error
+	if dir, _ := IsDir(src); dir {
+		cerr = CopyDir(src, dst)
+		if cerr != nil {
+			cerr = fmt.Errorf("copying directory failed: %w", cerr)
+		}
+	} else {
+		cerr = copyFile(src, dst)
+		if cerr != nil {
+			cerr = fmt.Errorf("copying file failed: %w", cerr)
+		}
+	}
+
+	if cerr != nil {
+		return fmt.Errorf("rename fallback failed: cannot rename %s to %s: %w", src, dst, cerr)
+	}
+
+	if err := os.RemoveAll(src); err != nil {
+		return fmt.Errorf("cannot delete %s: %w", src, err)
+	}
+
+	return nil
+}
+
+var (
+	errSrcNotDir = errors.New("source is not a directory")
+	errDstExist  = errors.New("destination already exists")
+)
+
+// CopyDir recursively copies a directory tree, attempting to preserve permissions.
+// Source directory must exist, destination directory must *not* exist.
+func CopyDir(src, dst string) error {
+	src = filepath.Clean(src)
+	dst = filepath.Clean(dst)
+
+	// We use os.Lstat() here to ensure we don't fall in a loop where a symlink
+	// actually links to a one of its parent directories.
+	fi, err := os.Lstat(src)
+	if err != nil {
+		return err
+	}
+	if !fi.IsDir() {
+		return errSrcNotDir
+	}
+
+	_, err = os.Stat(dst)
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	if err == nil {
+		return errDstExist
+	}
+
+	if err = os.MkdirAll(dst, fi.Mode()); err != nil {
+		return fmt.Errorf("cannot mkdir %s: %w", dst, err)
+	}
+
+	entries, err := ioutil.ReadDir(src)
+	if err != nil {
+		return fmt.Errorf("cannot read directory %s: %w", dst, err)
+	}
+
+	for _, entry := range entries {
+		srcPath := filepath.Join(src, entry.Name())
+		dstPath := filepath.Join(dst, entry.Name())
+
+		if entry.IsDir() {
+			if err = CopyDir(srcPath, dstPath); err != nil {
+				return fmt.Errorf("copying directory failed: %w", err)
+			}
+		} else {
+			// This will include symlinks, which is what we want when
+			// copying things.
+			if err = copyFile(srcPath, dstPath); err != nil {
+				return fmt.Errorf("copying file failed: %w", err)
+			}
+		}
+	}
+
+	return nil
+}
+
+// copyFile copies the contents of the file named src to the file named
+// by dst. The file will be created if it does not already exist. If the
+// destination file exists, all its contents will be replaced by the contents
+// of the source file. The file mode will be copied from the source.
+func copyFile(src, dst string) (err error) {
+	if sym, err := IsSymlink(src); err != nil {
+		return fmt.Errorf("symlink check failed: %w", err)
+	} else if sym {
+		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)
+	if err != nil {
+		return
+	}
+	defer in.Close()
+
+	out, err := os.Create(dst)
+	if err != nil {
+		return
+	}
+
+	if _, err = io.Copy(out, in); err != nil {
+		out.Close()
+		return
+	}
+
+	// Check for write errors on Close
+	if err = out.Close(); err != nil {
+		return
+	}
+
+	si, err := os.Stat(src)
+	if err != nil {
+		return
+	}
+
+	// Temporary fix for Go < 1.9
+	//
+	// See: https://github.com/golang/dep/issues/774
+	// and https://github.com/golang/go/issues/20829
+	if runtime.GOOS == "windows" {
+		dst = fixLongPath(dst)
+	}
+	err = os.Chmod(dst, si.Mode())
+
+	return
+}
+
+// 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 err
+	}
+
+	return os.Symlink(resolved, dst)
+}
+
+// IsDir determines is the path given is a directory or not.
+func IsDir(name string) (bool, error) {
+	fi, err := os.Stat(name)
+	if err != nil {
+		return false, err
+	}
+	if !fi.IsDir() {
+		return false, fmt.Errorf("%q is not a directory", name)
+	}
+	return true, nil
+}
+
+// IsSymlink determines if the given path is a symbolic link.
+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
+}
+
+// fixLongPath returns the extended-length (\\?\-prefixed) form of
+// path when needed, in order to avoid the default 260 character file
+// path limit imposed by Windows. If path is not easily converted to
+// the extended-length form (for example, if path is a relative path
+// or contains .. elements), or is short enough, fixLongPath returns
+// path unmodified.
+//
+// See https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath
+func fixLongPath(path string) string {
+	// Do nothing (and don't allocate) if the path is "short".
+	// Empirically (at least on the Windows Server 2013 builder),
+	// the kernel is arbitrarily okay with < 248 bytes. That
+	// matches what the docs above say:
+	// "When using an API to create a directory, the specified
+	// path cannot be so long that you cannot append an 8.3 file
+	// name (that is, the directory name cannot exceed MAX_PATH
+	// minus 12)." Since MAX_PATH is 260, 260 - 12 = 248.
+	//
+	// The MSDN docs appear to say that a normal path that is 248 bytes long
+	// will work; empirically the path must be less then 248 bytes long.
+	if len(path) < 248 {
+		// Don't fix. (This is how Go 1.7 and earlier worked,
+		// not automatically generating the \\?\ form)
+		return path
+	}
+
+	// The extended form begins with \\?\, as in
+	// \\?\c:\windows\foo.txt or \\?\UNC\server\share\foo.txt.
+	// The extended form disables evaluation of . and .. path
+	// elements and disables the interpretation of / as equivalent
+	// to \. The conversion here rewrites / to \ and elides
+	// . elements as well as trailing or duplicate separators. For
+	// simplicity it avoids the conversion entirely for relative
+	// paths or paths containing .. elements. For now,
+	// \\server\share paths are not converted to
+	// \\?\UNC\server\share paths because the rules for doing so
+	// are less well-specified.
+	if len(path) >= 2 && path[:2] == `\\` {
+		// Don't canonicalize UNC paths.
+		return path
+	}
+	if !isAbs(path) {
+		// Relative path
+		return path
+	}
+
+	const prefix = `\\?`
+
+	pathbuf := make([]byte, len(prefix)+len(path)+len(`\`))
+	copy(pathbuf, prefix)
+	n := len(path)
+	r, w := 0, len(prefix)
+	for r < n {
+		switch {
+		case os.IsPathSeparator(path[r]):
+			// empty block
+			r++
+		case path[r] == '.' && (r+1 == n || os.IsPathSeparator(path[r+1])):
+			// /./
+			r++
+		case r+1 < n && path[r] == '.' && path[r+1] == '.' && (r+2 == n || os.IsPathSeparator(path[r+2])):
+			// /../ is currently unhandled
+			return path
+		default:
+			pathbuf[w] = '\\'
+			w++
+			for ; r < n && !os.IsPathSeparator(path[r]); r++ {
+				pathbuf[w] = path[r]
+				w++
+			}
+		}
+	}
+	// A drive's root directory needs a trailing \
+	if w == len(`\\?\c:`) {
+		pathbuf[w] = '\\'
+		w++
+	}
+	return string(pathbuf[:w])
+}
+
+func isAbs(path string) (b bool) {
+	v := volumeName(path)
+	if v == "" {
+		return false
+	}
+	path = path[len(v):]
+	if path == "" {
+		return false
+	}
+	return os.IsPathSeparator(path[0])
+}
+
+func volumeName(path string) (v string) {
+	if len(path) < 2 {
+		return ""
+	}
+	// with drive letter
+	c := path[0]
+	if path[1] == ':' &&
+		('0' <= c && c <= '9' || 'a' <= c && c <= 'z' ||
+			'A' <= c && c <= 'Z') {
+		return path[:2]
+	}
+	// is it UNC
+	if l := len(path); l >= 5 && os.IsPathSeparator(path[0]) && os.IsPathSeparator(path[1]) &&
+		!os.IsPathSeparator(path[2]) && path[2] != '.' {
+		// first, leading `\\` and next shouldn't be `\`. its server name.
+		for n := 3; n < l-1; n++ {
+			// second, next '\' shouldn't be repeated.
+			if os.IsPathSeparator(path[n]) {
+				n++
+				// third, following something characters. its share name.
+				if !os.IsPathSeparator(path[n]) {
+					if path[n] == '.' {
+						break
+					}
+					for ; n < l; n++ {
+						if os.IsPathSeparator(path[n]) {
+							break
+						}
+					}
+					return path[:n]
+				}
+				break
+			}
+		}
+	}
+	return ""
+}

internal/fs/fs_test.go

@@ -0,0 +1,657 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package fs
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"sync"
+	"testing"
+)
+
+var (
+	mu sync.Mutex
+)
+
+func TestRenameWithFallback(t *testing.T) {
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	if err = RenameWithFallback(filepath.Join(dir, "does_not_exists"), filepath.Join(dir, "dst")); err == nil {
+		t.Fatal("expected an error for non existing file, but got nil")
+	}
+
+	srcpath := filepath.Join(dir, "src")
+
+	if srcf, err := os.Create(srcpath); err != nil {
+		t.Fatal(err)
+	} else {
+		srcf.Close()
+	}
+
+	if err = RenameWithFallback(srcpath, filepath.Join(dir, "dst")); err != nil {
+		t.Fatal(err)
+	}
+
+	srcpath = filepath.Join(dir, "a")
+	if err = os.MkdirAll(srcpath, 0777); err != nil {
+		t.Fatal(err)
+	}
+
+	dstpath := filepath.Join(dir, "b")
+	if err = os.MkdirAll(dstpath, 0777); err != nil {
+		t.Fatal(err)
+	}
+
+	if err = RenameWithFallback(srcpath, dstpath); err == nil {
+		t.Fatal("expected an error if dst is an existing directory, but got nil")
+	}
+}
+
+func TestCopyDir(t *testing.T) {
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	srcdir := filepath.Join(dir, "src")
+	if err := os.MkdirAll(srcdir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	files := []struct {
+		path     string
+		contents string
+		fi       os.FileInfo
+	}{
+		{path: "myfile", contents: "hello world"},
+		{path: filepath.Join("subdir", "file"), contents: "subdir file"},
+	}
+
+	// Create structure indicated in 'files'
+	for i, file := range files {
+		fn := filepath.Join(srcdir, file.path)
+		dn := filepath.Dir(fn)
+		if err = os.MkdirAll(dn, 0755); err != nil {
+			t.Fatal(err)
+		}
+
+		fh, err := os.Create(fn)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if _, err = fh.Write([]byte(file.contents)); err != nil {
+			t.Fatal(err)
+		}
+		fh.Close()
+
+		files[i].fi, err = os.Stat(fn)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	destdir := filepath.Join(dir, "dest")
+	if err := CopyDir(srcdir, destdir); err != nil {
+		t.Fatal(err)
+	}
+
+	// Compare copy against structure indicated in 'files'
+	for _, file := range files {
+		fn := filepath.Join(srcdir, file.path)
+		dn := filepath.Dir(fn)
+		dirOK, err := IsDir(dn)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !dirOK {
+			t.Fatalf("expected %s to be a directory", dn)
+		}
+
+		got, err := ioutil.ReadFile(fn)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if file.contents != string(got) {
+			t.Fatalf("expected: %s, got: %s", file.contents, string(got))
+		}
+
+		gotinfo, err := os.Stat(fn)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if file.fi.Mode() != gotinfo.Mode() {
+			t.Fatalf("expected %s: %#v\n to be the same mode as %s: %#v",
+				file.path, file.fi.Mode(), fn, gotinfo.Mode())
+		}
+	}
+}
+
+func TestCopyDirFail_SrcInaccessible(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		// XXX: setting permissions works differently in
+		// Microsoft Windows. Skipping this this until a
+		// compatible implementation is provided.
+		t.Skip("skipping on windows")
+	}
+
+	var srcdir, dstdir string
+
+	cleanup := setupInaccessibleDir(t, func(dir string) error {
+		srcdir = filepath.Join(dir, "src")
+		return os.MkdirAll(srcdir, 0755)
+	})
+	defer cleanup()
+
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	dstdir = filepath.Join(dir, "dst")
+	if err = CopyDir(srcdir, dstdir); err == nil {
+		t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir)
+	}
+}
+
+func TestCopyDirFail_DstInaccessible(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		// XXX: setting permissions works differently in
+		// Microsoft Windows. Skipping this this until a
+		// compatible implementation is provided.
+		t.Skip("skipping on windows")
+	}
+
+	var srcdir, dstdir string
+
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	srcdir = filepath.Join(dir, "src")
+	if err = os.MkdirAll(srcdir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	cleanup := setupInaccessibleDir(t, func(dir string) error {
+		dstdir = filepath.Join(dir, "dst")
+		return nil
+	})
+	defer cleanup()
+
+	if err := CopyDir(srcdir, dstdir); err == nil {
+		t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir)
+	}
+}
+
+func TestCopyDirFail_SrcIsNotDir(t *testing.T) {
+	var srcdir, dstdir string
+
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	srcdir = filepath.Join(dir, "src")
+	if _, err = os.Create(srcdir); err != nil {
+		t.Fatal(err)
+	}
+
+	dstdir = filepath.Join(dir, "dst")
+
+	if err = CopyDir(srcdir, dstdir); err == nil {
+		t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir)
+	}
+
+	if err != errSrcNotDir {
+		t.Fatalf("expected %v error for CopyDir(%s, %s), got %s", errSrcNotDir, srcdir, dstdir, err)
+	}
+
+}
+
+func TestCopyDirFail_DstExists(t *testing.T) {
+	var srcdir, dstdir string
+
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	srcdir = filepath.Join(dir, "src")
+	if err = os.MkdirAll(srcdir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	dstdir = filepath.Join(dir, "dst")
+	if err = os.MkdirAll(dstdir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	if err = CopyDir(srcdir, dstdir); err == nil {
+		t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir)
+	}
+
+	if err != errDstExist {
+		t.Fatalf("expected %v error for CopyDir(%s, %s), got %s", errDstExist, srcdir, dstdir, err)
+	}
+}
+
+func TestCopyDirFailOpen(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		// XXX: setting permissions works differently in
+		// Microsoft Windows. os.Chmod(..., 0222) below is not
+		// enough for the file to be readonly, and os.Chmod(...,
+		// 0000) returns an invalid argument error. Skipping
+		// this this until a compatible implementation is
+		// provided.
+		t.Skip("skipping on windows")
+	}
+
+	var srcdir, dstdir string
+
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	srcdir = filepath.Join(dir, "src")
+	if err = os.MkdirAll(srcdir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	srcfn := filepath.Join(srcdir, "file")
+	srcf, err := os.Create(srcfn)
+	if err != nil {
+		t.Fatal(err)
+	}
+	srcf.Close()
+
+	// setup source file so that it cannot be read
+	if err = os.Chmod(srcfn, 0222); err != nil {
+		t.Fatal(err)
+	}
+
+	dstdir = filepath.Join(dir, "dst")
+
+	if err = CopyDir(srcdir, dstdir); err == nil {
+		t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir)
+	}
+}
+
+func TestCopyFile(t *testing.T) {
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	srcf, err := os.Create(filepath.Join(dir, "srcfile"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	want := "hello world"
+	if _, err := srcf.Write([]byte(want)); err != nil {
+		t.Fatal(err)
+	}
+	srcf.Close()
+
+	destf := filepath.Join(dir, "destf")
+	if err := copyFile(srcf.Name(), destf); err != nil {
+		t.Fatal(err)
+	}
+
+	got, err := ioutil.ReadFile(destf)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if want != string(got) {
+		t.Fatalf("expected: %s, got: %s", want, string(got))
+	}
+
+	wantinfo, err := os.Stat(srcf.Name())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	gotinfo, err := os.Stat(destf)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if wantinfo.Mode() != gotinfo.Mode() {
+		t.Fatalf("expected %s: %#v\n to be the same mode as %s: %#v", srcf.Name(), wantinfo.Mode(), destf, gotinfo.Mode())
+	}
+}
+
+func TestCopyFileSymlink(t *testing.T) {
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanUpDir(dir)
+
+	testcases := map[string]string{
+		filepath.Join("./testdata/symlinks/file-symlink"):         filepath.Join(dir, "dst-file"),
+		filepath.Join("./testdata/symlinks/windows-file-symlink"): filepath.Join(dir, "windows-dst-file"),
+		filepath.Join("./testdata/symlinks/invalid-symlink"):      filepath.Join(dir, "invalid-symlink"),
+	}
+
+	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)
+			}
+
+			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)
+				if err != nil {
+					t.Fatalf("%+v", err)
+				}
+				dstb, err := ioutil.ReadFile(dst)
+				if err != nil {
+					t.Fatalf("%+v", err)
+				}
+
+				want = string(srcb)
+				got = string(dstb)
+			} else {
+				want, err = os.Readlink(symlink)
+				if err != nil {
+					t.Fatalf("%+v", err)
+				}
+
+				got, err = os.Readlink(dst)
+				if err != nil {
+					t.Fatalf("could not resolve symlink: %s", err)
+				}
+			}
+
+			if want != got {
+				t.Fatalf("resolved path is incorrect. expected %s, got %s", want, got)
+			}
+		})
+	}
+}
+
+func TestCopyFileLongFilePath(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		// We want to ensure the temporary fix actually fixes the issue with
+		// os.Chmod and long file paths. This is only applicable on Windows.
+		t.Skip("skipping on non-windows")
+	}
+
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cleanUpDir(dir)
+
+	// Create a directory with a long-enough path name to cause the bug in #774.
+	dirName := ""
+	for len(dir+string(os.PathSeparator)+dirName) <= 300 {
+		dirName += "directory"
+	}
+
+	fullPath := filepath.Join(dir, dirName, string(os.PathSeparator))
+	if err := os.MkdirAll(fullPath, 0755); err != nil && !os.IsExist(err) {
+		t.Fatalf("%+v", fmt.Errorf("unable to create temp directory: %s", fullPath))
+	}
+
+	err = ioutil.WriteFile(fullPath+"src", []byte(nil), 0644)
+	if err != nil {
+		t.Fatalf("%+v", err)
+	}
+
+	err = copyFile(fullPath+"src", fullPath+"dst")
+	if err != nil {
+		t.Fatalf("unexpected error while copying file: %v", err)
+	}
+}
+
+// C:\Users\appveyor\AppData\Local\Temp\1\gotest639065787\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890
+
+func TestCopyFileFail(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		// XXX: setting permissions works differently in
+		// Microsoft Windows. Skipping this this until a
+		// compatible implementation is provided.
+		t.Skip("skipping on windows")
+	}
+
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	srcf, err := os.Create(filepath.Join(dir, "srcfile"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	srcf.Close()
+
+	var dstdir string
+
+	cleanup := setupInaccessibleDir(t, func(dir string) error {
+		dstdir = filepath.Join(dir, "dir")
+		return os.Mkdir(dstdir, 0777)
+	})
+	defer cleanup()
+
+	fn := filepath.Join(dstdir, "file")
+	if err := copyFile(srcf.Name(), fn); err == nil {
+		t.Fatalf("expected error for %s, got none", fn)
+	}
+}
+
+// setupInaccessibleDir creates a temporary location with a single
+// directory in it, in such a way that that directory is not accessible
+// after this function returns.
+//
+// op is called with the directory as argument, so that it can create
+// files or other test artifacts.
+//
+// If setupInaccessibleDir fails in its preparation, or op fails, t.Fatal
+// will be invoked.
+//
+// This function returns a cleanup function that removes all the temporary
+// files this function creates. It is the caller's responsibility to call
+// this function before the test is done running, whether there's an error or not.
+func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() {
+	dir, err := ioutil.TempDir("", "dep")
+	if err != nil {
+		t.Fatal(err)
+		return nil // keep compiler happy
+	}
+
+	subdir := filepath.Join(dir, "dir")
+
+	cleanup := func() {
+		if err := os.Chmod(subdir, 0777); err != nil {
+			t.Error(err)
+		}
+		if err := os.RemoveAll(dir); err != nil {
+			t.Error(err)
+		}
+	}
+
+	if err := os.Mkdir(subdir, 0777); err != nil {
+		cleanup()
+		t.Fatal(err)
+		return nil
+	}
+
+	if err := op(subdir); err != nil {
+		cleanup()
+		t.Fatal(err)
+		return nil
+	}
+
+	if err := os.Chmod(subdir, 0666); err != nil {
+		cleanup()
+		t.Fatal(err)
+		return nil
+	}
+
+	return cleanup
+}
+
+func TestIsDir(t *testing.T) {
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var dn string
+
+	cleanup := setupInaccessibleDir(t, func(dir string) error {
+		dn = filepath.Join(dir, "dir")
+		return os.Mkdir(dn, 0777)
+	})
+	defer cleanup()
+
+	tests := map[string]struct {
+		exists bool
+		err    bool
+	}{
+		wd:                            {true, false},
+		filepath.Join(wd, "testdata"): {true, false},
+		filepath.Join(wd, "main.go"):  {false, true},
+		filepath.Join(wd, "this_file_does_not_exist.thing"): {false, true},
+		dn: {false, true},
+	}
+
+	if runtime.GOOS == "windows" {
+		// This test doesn't work on Microsoft Windows because
+		// of the differences in how file permissions are
+		// implemented. For this to work, the directory where
+		// the directory exists should be inaccessible.
+		delete(tests, dn)
+	}
+
+	for f, want := range tests {
+		got, err := IsDir(f)
+		if err != nil && !want.err {
+			t.Fatalf("expected no error, got %v", err)
+		}
+
+		if got != want.exists {
+			t.Fatalf("expected %t for %s, got %t", want.exists, f, got)
+		}
+	}
+}
+
+func TestIsSymlink(t *testing.T) {
+	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
+	)
+
+	cleanup := setupInaccessibleDir(t, 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")
+		return os.Symlink(inaccessibleFile, inaccessibleSymlink)
+	})
+	defer cleanup()
+
+	tests := map[string]struct{ expected, err bool }{
+		dirPath:             {false, false},
+		filePath:            {false, false},
+		dirSymlink:          {true, false},
+		fileSymlink:         {true, false},
+		inaccessibleFile:    {false, true},
+		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 {
+			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)
+		}
+	}
+}
+
+func cleanUpDir(dir string) {
+	if runtime.GOOS == "windows" {
+		mu.Lock()
+		exec.Command(`taskkill`, `/F`, `/IM`, `git.exe`).Run()
+		mu.Unlock()
+	}
+	if dir != "" {
+		os.RemoveAll(dir)
+	}
+}

internal/fs/rename.go

@@ -0,0 +1,30 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build !windows
+
+package fs
+
+import (
+	"fmt"
+	"os"
+	"syscall"
+)
+
+// renameFallback attempts to determine the appropriate fallback to failed rename
+// operation depending on the resulting error.
+func renameFallback(err error, src, dst string) error {
+	// Rename may fail if src and dst are on different devices; fall back to
+	// copy if we detect that case. syscall.EXDEV is the common name for the
+	// cross device link error which has varying output text across different
+	// operating systems.
+	terr, ok := err.(*os.LinkError)
+	if !ok {
+		return err
+	} else if terr.Err != syscall.EXDEV {
+		return fmt.Errorf("link error: cannot rename %s to %s: %w", src, dst, terr)
+	}
+
+	return renameByCopy(src, dst)
+}

internal/fs/rename_windows.go

@@ -0,0 +1,41 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build windows
+
+package fs
+
+import (
+	"fmt"
+	"os"
+	"syscall"
+)
+
+// renameFallback attempts to determine the appropriate fallback to failed rename
+// operation depending on the resulting error.
+func renameFallback(err error, src, dst string) error {
+	// Rename may fail if src and dst are on different devices; fall back to
+	// copy if we detect that case. syscall.EXDEV is the common name for the
+	// cross device link error which has varying output text across different
+	// operating systems.
+	terr, ok := err.(*os.LinkError)
+	if !ok {
+		return err
+	}
+
+	if terr.Err != syscall.EXDEV {
+		// In windows it can drop down to an operating system call that
+		// returns an operating system error with a different number and
+		// message. Checking for that as a fall back.
+		noerr, ok := terr.Err.(syscall.Errno)
+
+		// 0x11 (ERROR_NOT_SAME_DEVICE) is the windows error.
+		// See https://msdn.microsoft.com/en-us/library/cc231199.aspx
+		if ok && noerr != 0x11 {
+			return fmt.Errorf("link error: cannot rename %s to %s: %w", src, dst, terr)
+		}
+	}
+
+	return renameByCopy(src, dst)
+}

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

internal/fs/testdata/symlinks/windows-file-symlink

@@ -0,0 +1 @@
+C:/Users/ibrahim/go/src/github.com/golang/dep/internal/fs/testdata/test.file