From f639f8a4d0a3fcc3cc175def3b3b2d1ebdac1ade Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Fri, 29 Nov 2024 15:13:00 -0500 Subject: [PATCH] feat(golang-rewrite): more shim exec fixes * Create `toolversions.ParseSlice` function * Create `toolversions.Format` function * Refactor `shims` package to use `toolversions.Version` struct * Fix handling of path versions in `shims.FindExecutable` function --- cmd/cmd.go | 2 +- internal/shims/shims.go | 53 +++++++++++------- internal/shims/shims_test.go | 63 ++++++++++++++-------- internal/toolversions/toolversions.go | 20 +++++++ internal/toolversions/toolversions_test.go | 53 ++++++++++++++++++ 5 files changed, 149 insertions(+), 42 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 5dd036c5..9f4de841 100644 --- a/cmd/cmd.go +++ b/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 { diff --git a/internal/shims/shims.go b/internal/shims/shims.go index 2ec80745..837eab4f 100644 --- a/internal/shims/shims.go +++ b/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 { - err := hook.RunWithOutput(conf, fmt.Sprintf("pre_asdf_reshim_%s", plugin.Name), []string{version}, stdOut, stdErr) +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{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) { - paths, err := ExecutablePaths(conf, plugin, toolversions.Version{Type: versionType, Value: version}) +func ToolExecutables(conf config.Config, plugin plugins.Plugin, version toolversions.Version) (executables []string, err error) { + paths, err := ExecutablePaths(conf, plugin, version) if err != nil { return []string{}, err } diff --git a/internal/shims/shims_test.go b/internal/shims/shims_test.go index ed5afbc9..cc790076 100644 --- a/internal/shims/shims_test.go +++ b/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" - version2 := "2.0.0" + version := toolversions.Version{Type: "version", Value: "1.1.0"} + version2 := toolversions.Version{Type: "version", Value: "2.0.0"} conf, plugin := generateConfig(t) - installVersion(t, conf, plugin, version) - installVersion(t, conf, plugin, version2) - executables, err := ToolExecutables(conf, plugin, "version", version) + installVersion(t, conf, plugin, version.Value) + installVersion(t, conf, plugin, version2.Value) + 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", version2, &stdout, &stderr)) + assert.Nil(t, GenerateForVersion(conf, plugin, version, &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" - version2 := "2.0.0" + version := toolversions.Version{Type: "version", Value: "1.1.0"} + version2 := toolversions.Version{Type: "version", Value: "2.0.0"} conf, plugin := generateConfig(t) - installVersion(t, conf, plugin, version) - installVersion(t, conf, plugin, version2) - executables, err := ToolExecutables(conf, plugin, "version", version) + installVersion(t, conf, plugin, version.Value) + installVersion(t, conf, plugin, version2.Value) + 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 diff --git a/internal/toolversions/toolversions.go b/internal/toolversions/toolversions.go index 8319fd4b..e8bb63aa 100644 --- a/internal/toolversions/toolversions.go +++ b/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 { diff --git a/internal/toolversions/toolversions_test.go b/internal/toolversions/toolversions_test.go index 5de32b49..e4667aab 100644 --- a/internal/toolversions/toolversions_test.go +++ b/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")