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

feat(golang-rewrite): more shim exec fixes

cmd/cmd.go

@@ -1110,7 +1110,7 @@ func whereCommand(logger *log.Logger, tool, versionStr string) error {
 
 func reshimToolVersion(conf config.Config, tool, versionStr string, out io.Writer, errOut io.Writer) error {
 	version := toolversions.Parse(versionStr)
-	return shims.GenerateForVersion(conf, plugins.New(conf, tool), version.Type, version.Value, out, errOut)
+	return shims.GenerateForVersion(conf, plugins.New(conf, tool), version, out, errOut)
 }
 
 func latestForPlugin(conf config.Config, toolName, pattern string, showStatus bool) error {

internal/shims/shims.go

@@ -86,6 +86,13 @@ func FindExecutable(conf config.Config, shimName, currentDirectory string) (stri
 					tempVersions = append(tempVersions, "system")
 				}
 
+				parsedVersions := toolversions.ParseSlice(versions.Versions)
+				for _, parsedVersion := range parsedVersions {
+					if parsedVersion.Type == "path" {
+						tempVersions = append(tempVersions, toolversions.Format(parsedVersion))
+					}
+				}
+
 				versions.Versions = tempVersions
 				existingPluginToolVersions[plugin] = versions
 			}
@@ -98,14 +105,25 @@ func FindExecutable(conf config.Config, shimName, currentDirectory string) (stri
 
 	for plugin, toolVersions := range existingPluginToolVersions {
 		for _, version := range toolVersions.Versions {
-			if version == "system" {
+			parsedVersion := toolversions.Parse(version)
+			if parsedVersion.Type == "system" {
 				if executablePath, found := SystemExecutableOnPath(conf, shimName); found {
 					return executablePath, plugin, version, true, nil
 				}
 
 				break
 			}
-			executablePath, err := GetExecutablePath(conf, plugin, shimName, version)
+
+			if parsedVersion.Type == "path" {
+				executablePath, err := GetExecutablePath(conf, plugin, shimName, parsedVersion)
+				if err == nil {
+					return executablePath, plugin, version, true, nil
+				}
+
+				break
+			}
+
+			executablePath, err := GetExecutablePath(conf, plugin, shimName, parsedVersion)
 			if err == nil {
 				return executablePath, plugin, version, true, nil
 			}
@@ -141,13 +159,13 @@ func ExecutableOnPath(path, command string) (string, error) {
 }
 
 // GetExecutablePath returns the path of the executable
-func GetExecutablePath(conf config.Config, plugin plugins.Plugin, shimName, version string) (string, error) {
+func GetExecutablePath(conf config.Config, plugin plugins.Plugin, shimName string, version toolversions.Version) (string, error) {
 	path, err := getCustomExecutablePath(conf, plugin, shimName, version)
 	if err == nil {
 		return path, err
 	}
 
-	executables, err := ToolExecutables(conf, plugin, "version", version)
+	executables, err := ToolExecutables(conf, plugin, version)
 	if err != nil {
 		return "", err
 	}
@@ -177,11 +195,11 @@ func GetToolsAndVersionsFromShimFile(shimPath string) (versions []toolversions.T
 	return versions, err
 }
 
-func getCustomExecutablePath(conf config.Config, plugin plugins.Plugin, shimName, version string) (string, error) {
+func getCustomExecutablePath(conf config.Config, plugin plugins.Plugin, shimName string, version toolversions.Version) (string, error) {
 	var stdOut strings.Builder
 	var stdErr strings.Builder
 
-	installPath := installs.InstallPath(conf, plugin, toolversions.Version{Type: "version", Value: version})
+	installPath := installs.InstallPath(conf, plugin, version)
 	env := map[string]string{"ASDF_INSTALL_TYPE": "version"}
 
 	err := plugin.RunCallback("exec-path", []string{installPath, shimName}, env, &stdOut, &stdErr)
@@ -234,27 +252,24 @@ func GenerateForPluginVersions(conf config.Config, plugin plugins.Plugin, stdOut
 	}
 
 	for _, version := range installedVersions {
-		GenerateForVersion(conf, plugin, "version", version, stdOut, stdErr)
+		parsedVersion := toolversions.Parse(version)
+		GenerateForVersion(conf, plugin, parsedVersion, stdOut, stdErr)
 	}
 	return nil
 }
 
 // GenerateForVersion loops over all the executable files found for a tool and
 // generates a shim for each one
-func GenerateForVersion(conf config.Config, plugin plugins.Plugin, versionType, version string, stdOut io.Writer, stdErr io.Writer) error {
+func GenerateForVersion(conf config.Config, plugin plugins.Plugin, version toolversions.Version, stdOut io.Writer, stdErr io.Writer) error {
-	err := hook.RunWithOutput(conf, fmt.Sprintf("pre_asdf_reshim_%s", plugin.Name), []string{version}, stdOut, stdErr)
+	err := hook.RunWithOutput(conf, fmt.Sprintf("pre_asdf_reshim_%s", plugin.Name), []string{toolversions.Format(version)}, stdOut, stdErr)
 	if err != nil {
 		return err
 	}
-	executables, err := ToolExecutables(conf, plugin, versionType, version)
+	executables, err := ToolExecutables(conf, plugin, version)
 	if err != nil {
 		return err
 	}
 
-	if versionType == "path" {
-		version = fmt.Sprintf("path:%s", version)
-	}
-
 	for _, executablePath := range executables {
 		err := Write(conf, plugin, version, executablePath)
 		if err != nil {
@@ -262,7 +277,7 @@ func GenerateForVersion(conf config.Config, plugin plugins.Plugin, versionType,
 		}
 	}
 
-	err = hook.RunWithOutput(conf, fmt.Sprintf("post_asdf_reshim_%s", plugin.Name), []string{version}, stdOut, stdErr)
+	err = hook.RunWithOutput(conf, fmt.Sprintf("post_asdf_reshim_%s", plugin.Name), []string{toolversions.Format(version)}, stdOut, stdErr)
 	if err != nil {
 		return err
 	}
@@ -270,7 +285,7 @@ func GenerateForVersion(conf config.Config, plugin plugins.Plugin, versionType,
 }
 
 // Write generates a shim script and writes it to disk
-func Write(conf config.Config, plugin plugins.Plugin, version, executablePath string) error {
+func Write(conf config.Config, plugin plugins.Plugin, version toolversions.Version, executablePath string) error {
 	err := ensureShimDirExists(conf)
 	if err != nil {
 		return err
@@ -278,7 +293,7 @@ func Write(conf config.Config, plugin plugins.Plugin, version, executablePath st
 
 	shimName := filepath.Base(executablePath)
 	shimPath := Path(conf, shimName)
-	versions := []toolversions.ToolVersions{{Name: plugin.Name, Versions: []string{version}}}
+	versions := []toolversions.ToolVersions{{Name: plugin.Name, Versions: []string{toolversions.Format(version)}}}
 
 	if _, err := os.Stat(shimPath); err == nil {
 		oldVersions, err := GetToolsAndVersionsFromShimFile(shimPath)
@@ -307,8 +322,8 @@ func ensureShimDirExists(conf config.Config) error {
 }
 
 // ToolExecutables returns a slice of executables for a given tool version
-func ToolExecutables(conf config.Config, plugin plugins.Plugin, versionType, version string) (executables []string, err error) {
+func ToolExecutables(conf config.Config, plugin plugins.Plugin, version toolversions.Version) (executables []string, err error) {
-	paths, err := ExecutablePaths(conf, plugin, toolversions.Version{Type: versionType, Value: version})
+	paths, err := ExecutablePaths(conf, plugin, version)
 	if err != nil {
 		return []string{}, err
 	}

internal/shims/shims_test.go

@@ -80,6 +80,25 @@ func TestFindExecutable(t *testing.T) {
 		assert.Equal(t, filepath.Base(executable), "ls")
 		assert.NotEqual(t, executable, path)
 	})
+
+	t.Run("returns path to executable on path when path version set", func(t *testing.T) {
+		// write system version to version file
+		toolpath := filepath.Join(currentDir, ".tool-versions")
+		dir := installs.InstallPath(conf, plugin, toolversions.Version{Type: "version", Value: "1.1.0"})
+		pathVersion := fmt.Sprintf("path:%s/./", dir)
+		assert.Nil(t, os.WriteFile(toolpath, []byte(fmt.Sprintf("lua %s\n", pathVersion)), 0o666))
+		assert.Nil(t, GenerateAll(conf, &stdout, &stderr))
+
+		executable, gotPlugin, version, found, err := FindExecutable(conf, "dummy", currentDir)
+		assert.Equal(t, plugin, gotPlugin)
+		assert.Equal(t, version, pathVersion)
+		assert.True(t, found)
+		assert.Nil(t, err)
+
+		// see that it actually returns path to system ls
+		assert.Equal(t, filepath.Base(executable), "dummy")
+		assert.True(t, strings.HasPrefix(executable, dir))
+	})
 }
 
 func TestGetExecutablePath(t *testing.T) {
@@ -88,14 +107,14 @@ func TestGetExecutablePath(t *testing.T) {
 	installVersion(t, conf, plugin, version.Value)
 
 	t.Run("returns path to executable", func(t *testing.T) {
-		path, err := GetExecutablePath(conf, plugin, "dummy", version.Value)
+		path, err := GetExecutablePath(conf, plugin, "dummy", version)
 		assert.Nil(t, err)
 		assert.Equal(t, filepath.Base(path), "dummy")
 		assert.Equal(t, filepath.Base(filepath.Dir(filepath.Dir(path))), version.Value)
 	})
 
 	t.Run("returns error when executable with name not found", func(t *testing.T) {
-		path, err := GetExecutablePath(conf, plugin, "foo", version.Value)
+		path, err := GetExecutablePath(conf, plugin, "foo", version)
 		assert.ErrorContains(t, err, "executable not found")
 		assert.Equal(t, path, "")
 	})
@@ -104,7 +123,7 @@ func TestGetExecutablePath(t *testing.T) {
 		// Create exec-path callback
 		installDummyExecPathScript(t, conf, plugin, version, "dummy")
 
-		path, err := GetExecutablePath(conf, plugin, "dummy", version.Value)
+		path, err := GetExecutablePath(conf, plugin, "dummy", version)
 		assert.Nil(t, err)
 		assert.Equal(t, filepath.Base(filepath.Dir(path)), "custom")
 	})
@@ -114,7 +133,7 @@ func TestRemoveAll(t *testing.T) {
 	version := "1.1.0"
 	conf, plugin := generateConfig(t)
 	installVersion(t, conf, plugin, version)
-	executables, err := ToolExecutables(conf, plugin, "version", version)
+	executables, err := ToolExecutables(conf, plugin, toolversions.Version{Type: "version", Value: version})
 	assert.Nil(t, err)
 	stdout, stderr := buildOutputs()
 
@@ -137,7 +156,7 @@ func TestGenerateAll(t *testing.T) {
 	installVersion(t, conf, plugin, version)
 	installPlugin(t, conf, "dummy_plugin", "ruby")
 	installVersion(t, conf, plugin, version2)
-	executables, err := ToolExecutables(conf, plugin, "version", version)
+	executables, err := ToolExecutables(conf, plugin, toolversions.Version{Type: "version", Value: version})
 	assert.Nil(t, err)
 	stdout, stderr := buildOutputs()
 
@@ -166,7 +185,7 @@ func TestGenerateForPluginVersions(t *testing.T) {
 	conf, plugin := generateConfig(t)
 	installVersion(t, conf, plugin, version)
 	installVersion(t, conf, plugin, version2)
-	executables, err := ToolExecutables(conf, plugin, "version", version)
+	executables, err := ToolExecutables(conf, plugin, toolversions.Version{Type: "version", Value: version})
 	assert.Nil(t, err)
 	stdout, stderr := buildOutputs()
 
@@ -198,17 +217,17 @@ func TestGenerateForPluginVersions(t *testing.T) {
 }
 
 func TestGenerateForVersion(t *testing.T) {
-	version := "1.1.0"
+	version := toolversions.Version{Type: "version", Value: "1.1.0"}
-	version2 := "2.0.0"
+	version2 := toolversions.Version{Type: "version", Value: "2.0.0"}
 	conf, plugin := generateConfig(t)
-	installVersion(t, conf, plugin, version)
+	installVersion(t, conf, plugin, version.Value)
-	installVersion(t, conf, plugin, version2)
+	installVersion(t, conf, plugin, version2.Value)
-	executables, err := ToolExecutables(conf, plugin, "version", version)
+	executables, err := ToolExecutables(conf, plugin, version)
 	assert.Nil(t, err)
 
 	t.Run("generates shim script for every executable in version", func(t *testing.T) {
 		stdout, stderr := buildOutputs()
-		assert.Nil(t, GenerateForVersion(conf, plugin, "version", version, &stdout, &stderr))
+		assert.Nil(t, GenerateForVersion(conf, plugin, version, &stdout, &stderr))
 
 		// check for generated shims
 		for _, executable := range executables {
@@ -220,8 +239,8 @@ func TestGenerateForVersion(t *testing.T) {
 
 	t.Run("updates existing shims for every executable in version", func(t *testing.T) {
 		stdout, stderr := buildOutputs()
-		assert.Nil(t, GenerateForVersion(conf, plugin, "version", version, &stdout, &stderr))
+		assert.Nil(t, GenerateForVersion(conf, plugin, version, &stdout, &stderr))
-		assert.Nil(t, GenerateForVersion(conf, plugin, "version", version2, &stdout, &stderr))
+		assert.Nil(t, GenerateForVersion(conf, plugin, version2, &stdout, &stderr))
 
 		// check for generated shims
 		for _, executable := range executables {
@@ -233,12 +252,12 @@ func TestGenerateForVersion(t *testing.T) {
 }
 
 func TestWrite(t *testing.T) {
-	version := "1.1.0"
+	version := toolversions.Version{Type: "version", Value: "1.1.0"}
-	version2 := "2.0.0"
+	version2 := toolversions.Version{Type: "version", Value: "2.0.0"}
 	conf, plugin := generateConfig(t)
-	installVersion(t, conf, plugin, version)
+	installVersion(t, conf, plugin, version.Value)
-	installVersion(t, conf, plugin, version2)
+	installVersion(t, conf, plugin, version2.Value)
-	executables, err := ToolExecutables(conf, plugin, "version", version)
+	executables, err := ToolExecutables(conf, plugin, version)
 	executable := executables[0]
 	assert.Nil(t, err)
 
@@ -298,7 +317,7 @@ func TestToolExecutables(t *testing.T) {
 	installVersion(t, conf, plugin, version.Value)
 
 	t.Run("returns list of executables for plugin", func(t *testing.T) {
-		executables, err := ToolExecutables(conf, plugin, "version", version.Value)
+		executables, err := ToolExecutables(conf, plugin, version)
 		assert.Nil(t, err)
 
 		var filenames []string
@@ -313,7 +332,7 @@ func TestToolExecutables(t *testing.T) {
 	t.Run("returns list of executables for version installed in arbitrary directory", func(t *testing.T) {
 		// Reference regular install by path to validate this behavior
 		path := installs.InstallPath(conf, plugin, version)
-		executables, err := ToolExecutables(conf, plugin, "path", path)
+		executables, err := ToolExecutables(conf, plugin, toolversions.Version{Type: "path", Value: path})
 		assert.Nil(t, err)
 
 		var filenames []string
@@ -329,7 +348,7 @@ func TestToolExecutables(t *testing.T) {
 		// foo is first in list returned by list-bin-paths but doesn't exist, do
 		// we still get the executables in the bin/ dir?
 		repotest.WritePluginCallback(plugin.Dir, "list-bin-paths", "#!/usr/bin/env bash\necho 'foo bin'")
-		executables, err := ToolExecutables(conf, plugin, "version", version.Value)
+		executables, err := ToolExecutables(conf, plugin, version)
 		assert.Nil(t, err)
 
 		var filenames []string

internal/toolversions/toolversions.go

@@ -130,6 +130,26 @@ func Parse(version string) Version {
 	return Version{Type: "version", Value: version}
 }
 
+// ParseSlice takes a slice of strings and returns a slice of parsed versions.
+func ParseSlice(versions []string) (parsedVersions []Version) {
+	for _, version := range versions {
+		parsedVersions = append(parsedVersions, Parse(version))
+	}
+	return parsedVersions
+}
+
+// Format takes a Version struct and formats it as a string
+func Format(version Version) string {
+	switch version.Type {
+	case "system":
+		return "system"
+	case "path":
+		return fmt.Sprintf("path:%s", version.Value)
+	default:
+		return version.Value
+	}
+}
+
 // FormatForFS takes a versionType and version strings and generate a version
 // string suitable for the file system
 func FormatForFS(version Version) string {

internal/toolversions/toolversions_test.go

@@ -222,6 +222,59 @@ func TestParseFromCliArg(t *testing.T) {
 	})
 }
 
+func TestParseSlice(t *testing.T) {
+	t.Run("returns slice of parsed tool versions", func(t *testing.T) {
+		versions := ParseSlice([]string{"1.2.3"})
+		assert.Equal(t, []Version{{Type: "version", Value: "1.2.3"}}, versions)
+	})
+
+	t.Run("returns empty slice when empty slice provided", func(t *testing.T) {
+		versions := ParseSlice([]string{})
+		assert.Empty(t, versions)
+	})
+
+	t.Run("parses special versions", func(t *testing.T) {
+		versions := ParseSlice([]string{"ref:foo", "system", "path:/foo/bar"})
+		assert.Equal(t, []Version{{Type: "ref", Value: "foo"}, {Type: "system"}, {Type: "path", Value: "/foo/bar"}}, versions)
+	})
+}
+
+func TestFormat(t *testing.T) {
+	tests := []struct {
+		desc   string
+		input  Version
+		output string
+	}{
+		{
+			desc:   "with regular version",
+			input:  Version{Type: "version", Value: "foobar"},
+			output: "foobar",
+		},
+		{
+			desc:   "with ref version",
+			input:  Version{Type: "ref", Value: "foobar"},
+			output: "foobar",
+		},
+		{
+			desc:   "with system version",
+			input:  Version{Type: "system", Value: "system"},
+			output: "system",
+		},
+		{
+			desc:   "with system version",
+			input:  Version{Type: "path", Value: "/foo/bar"},
+			output: "path:/foo/bar",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			got := Format(tt.input)
+			assert.Equal(t, got, tt.output)
+		})
+	}
+}
+
 func TestFormatForFS(t *testing.T) {
 	t.Run("returns version when version type is not ref", func(t *testing.T) {
 		assert.Equal(t, FormatForFS(Version{Type: "version", Value: "foobar"}), "foobar")