diff --git a/internal/smartlink/smartlink.go b/internal/smartlink/smartlink.go index 49fbe2d01d..3df90034dd 100644 --- a/internal/smartlink/smartlink.go +++ b/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,7 +39,7 @@ 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 { @@ -58,15 +47,19 @@ func Link(src, dest string, visited map[string]bool) error { } 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 { diff --git a/internal/smartlink/smartlink_lin_mac.go b/internal/smartlink/smartlink_lin_mac.go index edc065f280..b19f45d411 100644 --- a/internal/smartlink/smartlink_lin_mac.go +++ b/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) } diff --git a/internal/smartlink/smartlink_test.go b/internal/smartlink/smartlink_test.go index 5b52fbb0d3..1222a0413d 100644 --- a/internal/smartlink/smartlink_test.go +++ b/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)) } diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index c53a1ac1e6..8443fcb3ee 100644 --- a/pkg/runtime/depot.go +++ b/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") }