When deploying via link, deploy symlinks directly instead of resolving them first.

Author: mitchell-as

internal/smartlink/smartlink.go

@@ -10,7 +10,7 @@ import (
 )
 
 // LinkContents will link the contents of src to desc
-func LinkContents(src, dest string, visited map[string]bool) error {
+func LinkContents(src, dest string) error {
 	if !fileutils.DirExists(src) {
 		return errs.New("src dir does not exist: %s", src)
 	}
@@ -29,7 +29,7 @@ func LinkContents(src, dest string, visited map[string]bool) error {
 		return errs.Wrap(err, "Reading dir %s failed", src)
 	}
 	for _, entry := range entries {
-		if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name()), visited); err != nil {
+		if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name())); err != nil {
 			return errs.Wrap(err, "Link failed")
 		}
 	}
@@ -39,27 +39,21 @@ func LinkContents(src, dest string, visited map[string]bool) error {
 
 // Link creates a link from src to target. MS decided to support Symlinks but only if you opt into developer mode (go figure),
 // which we cannot reasonably force on our users. So on Windows we will instead create dirs and hardlinks.
-func Link(src, dest string, visited map[string]bool) error {
-	srcWasSymlink := isSymlink(src)
-
+func Link(src, dest string) error {
 	var err error
 	src, dest, err = resolvePaths(src, dest)
 	if err != nil {
 		return errs.Wrap(err, "Could not resolve src and dest paths")
 	}
 
-	if visited == nil {
-		visited = make(map[string]bool)
-	}
-	if _, exists := visited[src]; exists && srcWasSymlink {
-		// We've encountered a recursive link. This is most often the case when the resolved src has
-		// already been visited. In that case, just link the dest to the src (which may be a directory;
-		// this is fine).
-		return linkFile(src, dest)
-	}
-	visited[src] = true
-
 	if fileutils.IsDir(src) {
+		if isSymlink(src) {
+			// Links to directories are okay on Linux and macOS, but will fail on Windows.
+			// If we ever get here on Windows, the artifact being deployed is bad and there's nothing we
+			// can do about it except receive the report from Rollbar and report it internally.
+			return linkFile(src, dest)
+		}
+
 		if err := fileutils.Mkdir(dest); err != nil {
 			return errs.Wrap(err, "could not create directory %s", dest)
 		}
@@ -68,7 +62,7 @@ func Link(src, dest string, visited map[string]bool) error {
 			return errs.Wrap(err, "could not read directory %s", src)
 		}
 		for _, entry := range entries {
-			if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name()), visited); err != nil {
+			if err := Link(filepath.Join(src, entry.Name()), filepath.Join(dest, entry.Name())); err != nil {
 				return errs.Wrap(err, "sub link failed")
 			}
 		}
@@ -153,11 +147,11 @@ func isSymlink(src string) bool {
 // This is to ensure that we're always comparing apples to apples when doing string comparisons on paths.
 func resolvePaths(src, dest string) (string, string, error) {
 	var err error
-	src, err = fileutils.ResolveUniquePath(src)
+	src, err = filepath.Abs(filepath.Clean(src))
 	if err != nil {
 		return "", "", errs.Wrap(err, "Could not resolve src path")
 	}
-	dest, err = fileutils.ResolveUniquePath(dest)
+	dest, err = filepath.Abs(filepath.Clean(dest))
 	if err != nil {
 		return "", "", errs.Wrap(err, "Could not resolve dest path")
 	}

internal/smartlink/smartlink_test.go

@@ -42,7 +42,7 @@ func TestLinkContentsWithCircularLink(t *testing.T) {
 	err = os.Symlink(subDir, circularLink)
 	require.NoError(t, err)
 
-	err = LinkContents(srcDir, destDir, nil)
+	err = LinkContents(srcDir, destDir)
 	if runtime.GOOS == "windows" {
 		require.Error(t, err)
 		return // hard links to directories are not allowed on Windows

pkg/runtime/depot.go

@@ -177,7 +177,7 @@ func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string)
 	}
 
 	// Copy or link the artifact files, depending on whether the artifact in question relies on file transformations
-	if err := smartlink.LinkContents(absoluteSrc, absoluteDest, nil); err != nil {
+	if err := smartlink.LinkContents(absoluteSrc, absoluteDest); err != nil {
 		return errs.Wrap(err, "failed to link artifact")
 	}
 
@@ -295,7 +295,7 @@ func (d *depot) Undeploy(id strfmt.UUID, relativeSrc, path string) error {
 	for sharedFile, relinkSrc := range redeploys {
 		switch deploy.Type {
 		case deploymentTypeLink:
-			if err := smartlink.Link(relinkSrc, sharedFile, nil); err != nil {
+			if err := smartlink.Link(relinkSrc, sharedFile); err != nil {
 				return errs.Wrap(err, "failed to relink file")
 			}
 		case deploymentTypeCopy:

pkg/runtime/links_windows.go

@@ -43,7 +43,7 @@ func supportsHardLinks(path string) (supported bool) {
 	}
 
 	logging.Debug("Attempting to link '%s' to '%s'", lnk, target)
-	err = smartlink.Link(target, lnk, nil)
+	err = smartlink.Link(target, lnk)
 	if err != nil {
 		logging.Debug("Test link creation failed: %v", err)
 		return false