internal/copydir: Factor out our recursive directory copy for reuse

We've previously been copying this function around so it could remain
unexported while being used in various packages. However, it's a
non-trivial function with lots of specific assumptions built into it, so
here we'll put it somewhere that other packages can depend on it _and_
document the assumptions it seems to be making for future reference.

As a bonus, this now uses os.SameFile to detect when two paths point to
the same physical file, instead of the slightly buggy local implementation
we had before which only worked on Unix systems and did not correctly
handle when the paths were on different physical devices.

The copy of the function I extracted here is the one from internal/initwd,
so this commit also includes the removal of that unexported version and
updating the callers in that package to use at at this new location.

internal/copydir/copy_dir.go

@@ -1,4 +1,4 @@
-package initwd
+package copydir
 
 import (
 	"io"
@@ -7,9 +7,32 @@ import (
 	"strings"
 )
 
-// copyDir copies the src directory contents into dst. Both directories
+// CopyDir recursively copies all of the files within the directory given in
-// should already exist.
+// src to the directory given in dst.
-func copyDir(dst, src string) error {
+//
+// Both directories should already exist. If the destination directory is
+// non-empty then the new files will merge in with the old, overwriting any
+// files that have a relative path in common between source and destination.
+//
+// Recursive copying of directories is inevitably a rather opinionated sort of
+// operation, so this function won't be appropriate for all use-cases. Some
+// of the "opinions" it has are described in the following paragraphs:
+//
+// Symlinks in the source directory are recreated with the same target in the
+// destination directory. If the symlink is to a directory itself, that
+// directory is not recursively visited for further copying.
+//
+// File and directory modes are not preserved exactly, but the executable
+// flag is preserved for files on operating systems where it is significant.
+//
+// Any "dot files" it encounters along the way are skipped, even on platforms
+// that do not normally ascribe special meaning to files with names starting
+// with dots.
+//
+// Callers may rely on the above details and other undocumented details of
+// this function, so if you intend to change it be sure to review the callers
+// first and make sure they are compatible with the change you intend to make.
+func CopyDir(dst, src string) error {
 	src, err := filepath.EvalSymlinks(src)
 	if err != nil {
 		return err
@@ -38,7 +61,7 @@ func copyDir(dst, src string) error {
 		dstPath := filepath.Join(dst, path[len(src):])
 
 		// we don't want to try and copy the same file over itself.
-		if eq, err := sameFile(path, dstPath); eq {
+		if eq, err := SameFile(path, dstPath); eq {
 			return nil
 		} else if err != nil {
 			return err
@@ -94,14 +117,16 @@ func copyDir(dst, src string) error {
 	return filepath.Walk(src, walkFn)
 }
 
-// sameFile tried to determine if to paths are the same file.
+// SameFile returns true if the two given paths refer to the same physical
-// If the paths don't match, we lookup the inode on supported systems.
+// file on disk, using the unique file identifiers from the underlying
-func sameFile(a, b string) (bool, error) {
+// operating system. For example, on Unix systems this checks whether the
+// two files are on the same device and have the same inode.
+func SameFile(a, b string) (bool, error) {
 	if a == b {
 		return true, nil
 	}
 
-	aIno, err := inode(a)
+	aInfo, err := os.Lstat(a)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return false, nil
@@ -109,7 +134,7 @@ func sameFile(a, b string) (bool, error) {
 		return false, err
 	}
 
-	bIno, err := inode(b)
+	bInfo, err := os.Lstat(b)
 	if err != nil {
 		if os.IsNotExist(err) {
 			return false, nil
@@ -117,9 +142,5 @@ func sameFile(a, b string) (bool, error) {
 		return false, err
 	}
 
-	if aIno > 0 && aIno == bIno {
+	return os.SameFile(aInfo, bInfo), nil
-		return true, nil
-	}
-
-	return false, nil
 }

internal/copydir/copy_dir_test.go

@@ -1,4 +1,4 @@
-package initwd
+package copydir
 
 import (
 	"io/ioutil"
@@ -52,7 +52,7 @@ func TestCopyDir_symlinks(t *testing.T) {
 	targetDir := filepath.Join(tmpdir, "target")
 	os.Mkdir(targetDir, os.ModePerm)
 
-	err = copyDir(targetDir, moduleDir)
+	err = CopyDir(targetDir, moduleDir)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -92,7 +92,7 @@ func TestCopyDir_symlink_file(t *testing.T) {
 	targetDir := filepath.Join(tmpdir, "target")
 	os.Mkdir(targetDir, os.ModePerm)
 
-	err = copyDir(targetDir, moduleDir)
+	err = CopyDir(targetDir, moduleDir)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/initwd/from_module.go

@@ -2,7 +2,6 @@ package initwd
 
 import (
 	"fmt"
-	"github.com/hashicorp/terraform/internal/earlyconfig"
 	"io/ioutil"
 	"log"
 	"os"
@@ -10,8 +9,11 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/hashicorp/terraform/internal/earlyconfig"
+
 	version "github.com/hashicorp/go-version"
 	"github.com/hashicorp/terraform-config-inspect/tfconfig"
+	"github.com/hashicorp/terraform/internal/copydir"
 	"github.com/hashicorp/terraform/internal/modsdir"
 	"github.com/hashicorp/terraform/registry"
 	"github.com/hashicorp/terraform/tfdiags"
@@ -172,7 +174,7 @@ func DirFromModule(rootDir, modulesDir, sourceAddr string, reg *registry.Client,
 			// We've found the module the user requested, which we must
 			// now copy into rootDir so it can be used directly.
 			log.Printf("[TRACE] copying new root module from %s to %s", record.Dir, rootDir)
-			err := copyDir(rootDir, record.Dir)
+			err := copydir.CopyDir(rootDir, record.Dir)
 			if err != nil {
 				diags = diags.Append(tfdiags.Sourceless(
 					tfdiags.Error,
@@ -292,7 +294,7 @@ func DirFromModule(rootDir, modulesDir, sourceAddr string, reg *registry.Client,
 		// We copy rather than "rename" here because renaming between directories
 		// can be tricky in edge-cases like network filesystems, etc.
 		log.Printf("[TRACE] copying new module %s from %s to %s", newKey, record.Dir, instPath)
-		err := copyDir(instPath, tempPath)
+		err := copydir.CopyDir(instPath, tempPath)
 		if err != nil {
 			diags = diags.Append(tfdiags.Sourceless(
 				tfdiags.Error,

internal/initwd/getter.go

@@ -9,6 +9,7 @@ import (
 
 	cleanhttp "github.com/hashicorp/go-cleanhttp"
 	getter "github.com/hashicorp/go-getter"
+	"github.com/hashicorp/terraform/internal/copydir"
 	"github.com/hashicorp/terraform/registry/regsrc"
 )
 
@@ -112,7 +113,7 @@ func (g reusingGetter) getWithGoGetter(instPath, addr string) (string, error) {
 		if err != nil {
 			return "", fmt.Errorf("failed to create directory %s: %s", instPath, err)
 		}
-		err = copyDir(instPath, prevDir)
+		err = copydir.CopyDir(instPath, prevDir)
 		if err != nil {
 			return "", fmt.Errorf("failed to copy from %s to %s: %s", prevDir, instPath, err)
 		}

internal/initwd/inode.go

@@ -1,21 +0,0 @@
-// +build linux darwin openbsd netbsd solaris dragonfly
-
-package initwd
-
-import (
-	"fmt"
-	"os"
-	"syscall"
-)
-
-// lookup the inode of a file on posix systems
-func inode(path string) (uint64, error) {
-	stat, err := os.Stat(path)
-	if err != nil {
-		return 0, err
-	}
-	if st, ok := stat.Sys().(*syscall.Stat_t); ok {
-		return st.Ino, nil
-	}
-	return 0, fmt.Errorf("could not determine file inode")
-}

internal/initwd/inode_freebsd.go

@@ -1,21 +0,0 @@
-// +build freebsd
-
-package initwd
-
-import (
-	"fmt"
-	"os"
-	"syscall"
-)
-
-// lookup the inode of a file on posix systems
-func inode(path string) (uint64, error) {
-	stat, err := os.Stat(path)
-	if err != nil {
-		return 0, err
-	}
-	if st, ok := stat.Sys().(*syscall.Stat_t); ok {
-		return uint64(st.Ino), nil
-	}
-	return 0, fmt.Errorf("could not determine file inode")
-}

internal/initwd/inode_windows.go

@@ -1,8 +0,0 @@
-// +build windows
-
-package initwd
-
-// no syscall.Stat_t on windows, return 0 for inodes
-func inode(path string) (uint64, error) {
-	return 0, nil
-}

internal/initwd/module_install_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform/configs"
 	"github.com/hashicorp/terraform/configs/configload"
 	"github.com/hashicorp/terraform/helper/logging"
+	"github.com/hashicorp/terraform/internal/copydir"
 	"github.com/hashicorp/terraform/registry"
 	"github.com/hashicorp/terraform/tfdiags"
 )
@@ -543,7 +544,7 @@ func tempChdir(t *testing.T, sourceDir string) (string, func()) {
 		return "", nil
 	}
 
-	if err := copyDir(tmpDir, sourceDir); err != nil {
+	if err := copydir.CopyDir(tmpDir, sourceDir); err != nil {
 		t.Fatalf("failed to copy fixture to temporary directory: %s", err)
 		return "", nil
 	}