Merge pull request #83 from asdf-vm/tb/shim-exec-fixes

fix(golang-rewrite): `asdf exec` and `asdf env` command fixes

cmd/cmd.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"log"
 	"os"
-	osexec "os/exec"
 	"path/filepath"
 	"slices"
 	"strings"
@@ -20,7 +19,6 @@ import (
 	"asdf/internal/help"
 	"asdf/internal/info"
 	"asdf/internal/installs"
-	"asdf/internal/paths"
 	"asdf/internal/plugins"
 	"asdf/internal/resolve"
 	"asdf/internal/shims"
@@ -381,36 +379,23 @@ func envCommand(logger *log.Logger, shimmedCommand string, args []string) error
 	if err != nil {
 		return err
 	}
-	callbackEnv := map[string]string{
+	env := map[string]string{
 		"ASDF_INSTALL_TYPE":    parsedVersion.Type,
 		"ASDF_INSTALL_VERSION": parsedVersion.Value,
 		"ASDF_INSTALL_PATH":    installs.InstallPath(conf, plugin, parsedVersion),
-		"PATH":                 setPath(conf, execPaths),
+		"PATH":                 setPath(execPaths),
 	}
 
-	var env map[string]string
-	var fname string
-
-	if parsedVersion.Type == "system" {
-		env = execute.SliceToMap(os.Environ())
-		newPath := paths.RemoveFromPath(env["PATH"], shims.Directory(conf))
-		env["PATH"] = newPath
-		var found bool
-		fname, found = shims.FindSystemExecutable(conf, command)
-		if !found {
-			fmt.Println("not found")
-			return err
-		}
-	} else {
-		env, err = execenv.Generate(plugin, callbackEnv)
+	if parsedVersion.Type != "system" {
+		env, err = execenv.Generate(plugin, env)
 		if _, ok := err.(plugins.NoCallbackError); !ok && err != nil {
 			return err
 		}
+	}
 
-		fname, err = osexec.LookPath(command)
-		if err != nil {
-			return err
-		}
+	fname, err := shims.ExecutableOnPath(env["PATH"], command)
+	if err != nil {
+		return err
 	}
 
 	err = exec.Exec(fname, realArgs, execute.MapToSlice(env))
@@ -420,9 +405,8 @@ func envCommand(logger *log.Logger, shimmedCommand string, args []string) error
 	return err
 }
 
-func setPath(conf config.Config, pathes []string) string {
-	currentPath := os.Getenv("PATH")
-	return strings.Join(pathes, ":") + ":" + paths.RemoveFromPath(currentPath, shims.Directory(conf))
+func setPath(paths []string) string {
+	return strings.Join(paths, ":") + ":" + os.Getenv("PATH")
 }
 
 func execCommand(logger *log.Logger, command string, args []string) error {
@@ -438,8 +422,6 @@ func execCommand(logger *log.Logger, command string, args []string) error {
 	}
 
 	executable, plugin, version, err := getExecutable(logger, conf, command)
-	fmt.Printf("version %#+v\n", version)
-	fmt.Println("here")
 	if err != nil {
 		return err
 	}
@@ -451,19 +433,26 @@ func execCommand(logger *log.Logger, command string, args []string) error {
 	}
 
 	parsedVersion := toolversions.Parse(version)
-	fmt.Printf("parsedVersion %#+v\n", parsedVersion)
-	paths, err := shims.ExecutablePaths(conf, plugin, parsedVersion)
+	execPaths, err := shims.ExecutablePaths(conf, plugin, parsedVersion)
 	if err != nil {
 		return err
 	}
-	callbackEnv := map[string]string{
+	env := map[string]string{
 		"ASDF_INSTALL_TYPE":    parsedVersion.Type,
 		"ASDF_INSTALL_VERSION": parsedVersion.Value,
 		"ASDF_INSTALL_PATH":    installs.InstallPath(conf, plugin, parsedVersion),
-		"PATH":                 setPath(conf, paths),
+		"PATH":                 setPath(execPaths),
+	}
+
+	if parsedVersion.Type != "system" {
+		env, err = execenv.Generate(plugin, env)
+		if _, ok := err.(plugins.NoCallbackError); !ok && err != nil {
+			return err
+		}
 	}
 
-	env, _ := execenv.Generate(plugin, callbackEnv)
+	env = execenv.MergeEnv(execenv.SliceToMap(os.Environ()), env)
+
 	return exec.Exec(executable, args, execute.MapToSlice(env))
 }
 

internal/execenv/execenv.go

@@ -4,6 +4,7 @@ package execenv
 
 import (
 	"fmt"
+	"os"
 	"strings"
 
 	"asdf/internal/execute"
@@ -12,6 +13,20 @@ import (
 
 const execEnvCallbackName = "exec-env"
 
+// CurrentEnv returns the current environment as a map
+func CurrentEnv() map[string]string {
+	return SliceToMap(os.Environ())
+}
+
+// MergeEnv takes two maps with string keys and values and merges them.
+func MergeEnv(map1, map2 map[string]string) map[string]string {
+	for key, value := range map2 {
+		map1[key] = value
+	}
+
+	return map1
+}
+
 // Generate runs exec-env callback if available and captures the environment
 // variables it sets. It then parses them and returns them as a map.
 func Generate(plugin plugins.Plugin, callbackEnv map[string]string) (env map[string]string, err error) {
@@ -47,3 +62,17 @@ func envMap(env string) map[string]string {
 
 	return slice
 }
+
+// SliceToMap converts an env map to env slice suitable for syscall.Exec
+func SliceToMap(env []string) map[string]string {
+	envMap := map[string]string{}
+
+	for _, envVar := range env {
+		varValue := strings.Split(envVar, "=")
+		if len(varValue) == 2 {
+			envMap[varValue[0]] = varValue[1]
+		}
+	}
+
+	return envMap
+}

internal/execenv/execenv_test.go

@@ -15,6 +15,39 @@ const (
 	testPluginName2 = "ruby"
 )
 
+func TestCurrentEnv(t *testing.T) {
+	t.Run("returns map of current environment", func(t *testing.T) {
+		envMap := CurrentEnv()
+		path, found := envMap["PATH"]
+		assert.True(t, found)
+		assert.NotEmpty(t, path)
+	})
+}
+
+func TestMergeEnv(t *testing.T) {
+	t.Run("merges two maps", func(t *testing.T) {
+		map1 := map[string]string{"Key": "value"}
+		map2 := map[string]string{"Key2": "value2"}
+		map3 := MergeEnv(map1, map2)
+		assert.Equal(t, map3["Key"], "value")
+		assert.Equal(t, map3["Key2"], "value2")
+	})
+
+	t.Run("doesn't change original map", func(t *testing.T) {
+		map1 := map[string]string{"Key": "value"}
+		map2 := map[string]string{"Key2": "value2"}
+		_ = MergeEnv(map1, map2)
+		assert.Equal(t, map1["Key2"], "value2")
+	})
+
+	t.Run("second map overwrites values in first", func(t *testing.T) {
+		map1 := map[string]string{"Key": "value"}
+		map2 := map[string]string{"Key": "value2"}
+		map3 := MergeEnv(map1, map2)
+		assert.Equal(t, map3["Key"], "value2")
+	})
+}
+
 func TestGenerate(t *testing.T) {
 	testDataDir := t.TempDir()
 

internal/execute/execute.go

@@ -66,20 +66,6 @@ func MapToSlice(env map[string]string) (slice []string) {
 	return slice
 }
 
-// SliceToMap converts an env map to env slice suitable for syscall.Exec
-func SliceToMap(env []string) map[string]string {
-	envMap := map[string]string{}
-
-	for _, envVar := range env {
-		varValue := strings.Split(envVar, "=")
-		if len(varValue) == 2 {
-			envMap[varValue[0]] = varValue[1]
-		}
-	}
-
-	return envMap
-}
-
 func formatArgString(args []string) string {
 	var newArgs []string
 	for _, str := range args {

internal/paths/paths_test.go

@@ -12,6 +12,11 @@ func TestRemoveFromPath(t *testing.T) {
 		assert.Equal(t, got, "/foo/bar:/home/user/bin")
 	})
 
+	t.Run("returns PATH string with multiple matching paths removed", func(t *testing.T) {
+		got := RemoveFromPath("/foo/bar:/baz/bim:/baz/bim:/home/user/bin", "/baz/bim")
+		assert.Equal(t, got, "/foo/bar:/home/user/bin")
+	})
+
 	t.Run("returns PATH string unchanged when no matching path found", func(t *testing.T) {
 		got := RemoveFromPath("/foo/bar:/baz/bim:/home/user/bin", "/path-not-present/")
 		assert.Equal(t, got, "/foo/bar:/baz/bim:/home/user/bin")

internal/shims/shims.go

@@ -99,7 +99,7 @@ func FindExecutable(conf config.Config, shimName, currentDirectory string) (stri
 	for plugin, toolVersions := range existingPluginToolVersions {
 		for _, version := range toolVersions.Versions {
 			if version == "system" {
-				if executablePath, found := FindSystemExecutable(conf, shimName); found {
+				if executablePath, found := SystemExecutableOnPath(conf, shimName); found {
 					return executablePath, plugin, version, true, nil
 				}
 
@@ -122,16 +122,24 @@ func FindExecutable(conf config.Config, shimName, currentDirectory string) (stri
 	return "", plugins.Plugin{}, "", false, NoExecutableForPluginError{shim: shimName, tools: tools, versions: versions}
 }
 
-// FindSystemExecutable returns the path to the system
-// executable if found
-func FindSystemExecutable(conf config.Config, executableName string) (string, bool) {
+// SystemExecutableOnPath returns the path to the system executable if found,
+// removes asdf shim directory from search
+func SystemExecutableOnPath(conf config.Config, executableName string) (string, bool) {
 	currentPath := os.Getenv("PATH")
-	defer os.Setenv("PATH", currentPath)
-	os.Setenv("PATH", paths.RemoveFromPath(currentPath, Directory(conf)))
-	executablePath, err := exec.LookPath(executableName)
+	executablePath, err := ExecutableOnPath(paths.RemoveFromPath(currentPath, Directory(conf)), executableName)
 	return executablePath, err == nil
 }
 
+// ExecutableOnPath returns the path to an executable if one is found on the
+// provided paths. `path` must be in the same format as the `PATH` environment
+// variable.
+func ExecutableOnPath(path, command string) (string, error) {
+	currentPath := os.Getenv("PATH")
+	defer os.Setenv("PATH", currentPath)
+	os.Setenv("PATH", path)
+	return exec.LookPath(command)
+}
+
 // GetExecutablePath returns the path of the executable
 func GetExecutablePath(conf config.Config, plugin plugins.Plugin, shimName, version string) (string, error) {
 	path, err := getCustomExecutablePath(conf, plugin, shimName, version)