Recurse into recursively linked directories not more than once.

This results in a destination without directory links, which are not supported on Windows.

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)
 	}
@@ -24,23 +24,12 @@ func LinkContents(src, dest string, visited map[string]bool) error {
 		return errs.Wrap(err, "Could not resolve src and dest paths")
 	}
 
-	if visited == nil {
-		visited = make(map[string]bool)
-	}
-	if _, exists := visited[src]; exists {
-		// 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
-
 	entries, err := os.ReadDir(src)
 	if err != nil {
 		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()), nil); err != nil {
 			return errs.Wrap(err, "Link failed")
 		}
 	}
@@ -50,23 +39,27 @@ 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 {
+func Link(src, dest string, visited map[string]int) 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)
+		visited = make(map[string]int)
 	}
-	if _, exists := visited[src]; exists {
+	if count, exists := visited[src]; exists {
 		// 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)
+		// already been visited. We will recurse into the directory no more than once, so that any
+		// runtime paths that reference the link will not silently fail.
+		if count > 1 {
+			return nil
+		}
+		visited[src]++
+	} else {
+		visited[src] = 1
 	}
-	visited[src] = true
 
 	if fileutils.IsDir(src) {
 		if err := fileutils.Mkdir(dest); err != nil {

internal/smartlink/smartlink_lin_mac.go

@@ -5,10 +5,16 @@ package smartlink
 
 import (
 	"os"
+
+	"github.com/ActiveState/cli/internal/errs"
+	"github.com/ActiveState/cli/internal/fileutils"
 )
 
 // file will create a symlink from src to dest, and falls back on a hardlink if no symlink is available.
 // This is a workaround for the fact that Windows does not support symlinks without admin privileges.
 func linkFile(src, dest string) error {
+	if fileutils.IsDir(src) {
+		return errs.New("src is a directory, not a file: %s", src)
+	}
 	return os.Symlink(src, dest)
 }

internal/smartlink/smartlink_test.go

@@ -3,10 +3,8 @@ package smartlink
 import (
 	"os"
 	"path/filepath"
-	"runtime"
 	"testing"
 
-	"github.com/ActiveState/cli/internal/fileutils"
 	"github.com/stretchr/testify/require"
 )
 
@@ -42,14 +40,17 @@ func TestLinkContentsWithCircularLink(t *testing.T) {
 	err = os.Symlink(subDir, circularLink)
 	require.NoError(t, err)
 
-	err = LinkContents(srcDir, destDir, nil)
-	if runtime.GOOS == "windows" {
-		require.Error(t, err)
-		return // hard links to directories are not allowed on Windows
-	}
+	err = LinkContents(srcDir, destDir)
 	require.NoError(t, err)
 
 	// Verify file structure.
+	// src/
+	//   ├── regular.txt
+	//   └── subdir/
+	//        ├── circle
+	//        │   │   (no subdir/)
+	//        │   └── subfile.txt
+	//        └── subfile.txt
 	destFile := filepath.Join(destDir, "regular.txt")
 	require.FileExists(t, destFile)
 	content, err := os.ReadFile(destFile)
@@ -62,14 +63,11 @@ func TestLinkContentsWithCircularLink(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "sub content", string(subContent))
 
-	destCircular := filepath.Join(destDir, "subdir", "circle")
-	require.FileExists(t, destCircular)
-	target, err := fileutils.ResolveUniquePath(destCircular)
+	require.NoDirExists(t, filepath.Join(destDir, "subdir", "circle", "circle"))
+
+	destCircularSubFile := filepath.Join(destDir, "subdir", "circle", "subfile.txt")
+	require.FileExists(t, destCircularSubFile)
+	subContent, err = os.ReadFile(destCircularSubFile)
 	require.NoError(t, err)
-	srcCircular := filepath.Join(srcDir, "subdir")
-	if runtime.GOOS == "darwin" {
-		srcCircular, err = fileutils.ResolveUniquePath(srcCircular) // needed for full $TMPDIR resolution
-		require.NoError(t, err)
-	}
-	require.Equal(t, target, srcCircular)
+	require.Equal(t, "sub content", string(subContent))
 }

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")
 	}