From d25f2cdf0dabed1da76113ea3a6969f07aaf8315 Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Sun, 16 Feb 2025 18:51:59 -0500 Subject: [PATCH 1/9] fix: install should report missing plugins --- internal/versions/versions.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/internal/versions/versions.go b/internal/versions/versions.go index bd268c809..a45983fff 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "path" "regexp" "strings" @@ -26,6 +27,7 @@ const ( uninstallableVersionMsg = "uninstallable version: %s" latestFilterRegex = "(?i)(^Available versions:|-src|-dev|-latest|-stm|[-\\.]rc|-milestone|-alpha|-beta|[-\\.]pre|-next|(a|b|c)[0-9]+|snapshot|master)" noLatestVersionErrMsg = "no latest version found" + missingPluginErrMsg = "missing plugin for %s" ) // UninstallableVersionError is an error returned if someone tries to install the @@ -44,6 +46,16 @@ type NoVersionSetError struct { toolName string } +// MissingPluginError is returned whenever an operation expects a plugin, +// but it is not installed. +type MissingPluginError struct { + toolName string +} + +func (e MissingPluginError) Error() string { + return fmt.Sprintf(missingPluginErrMsg, e.toolName) +} + func (e NoVersionSetError) Error() string { // Eventually switch this to a more friendly error message, BATS tests fail // with this improvement @@ -72,16 +84,31 @@ func InstallAll(conf config.Config, dir string, stdOut io.Writer, stdErr io.Writ return []error{fmt.Errorf("unable to list plugins: %w", err)} } + toolNames := map[string]bool{} + filepath := path.Join(dir, conf.DefaultToolVersionsFilename) + toolVersions, err := toolversions.GetAllToolsAndVersions(filepath) + if err == nil { + for _, version := range toolVersions { + toolNames[version.Name] = true + } + } + // Ideally we should install these in the order they are specified in the // closest .tool-versions file, but for now that is too complicated to // implement. for _, plugin := range plugins { + delete(toolNames, plugin.Name) err := Install(conf, plugin, dir, stdOut, stdErr) if err != nil { failures = append(failures, err) } } + for toolName := range toolNames { + err = MissingPluginError{toolName: toolName} + failures = append(failures, err) + } + return failures } From c52e8d065bd473a621adc641a210f1e376db4672 Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Sun, 16 Feb 2025 19:20:50 -0500 Subject: [PATCH 2/9] Add a test --- internal/versions/versions_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/internal/versions/versions_test.go b/internal/versions/versions_test.go index 8b4490774..24a2fae05 100644 --- a/internal/versions/versions_test.go +++ b/internal/versions/versions_test.go @@ -47,6 +47,7 @@ func TestInstallAll(t *testing.T) { writeVersionFile(t, currentDir, content) err := InstallAll(conf, currentDir, &stdout, &stderr) + assert.Len(t, err, 1, "expected 1 install error") assert.ErrorContains(t, err[0], "no version set") assertVersionInstalled(t, conf.DataDir, plugin.Name, version) @@ -70,6 +71,23 @@ func TestInstallAll(t *testing.T) { assertNotInstalled(t, conf.DataDir, secondPlugin.Name, version) assertVersionInstalled(t, conf.DataDir, plugin.Name, version) }) + + t.Run("reports skipped tools due to missing plugin", func(t *testing.T) { + conf, plugin := generateConfig(t) + stdout, stderr := buildOutputs() + currentDir := t.TempDir() + version := "1.0.0" + + // write a version file + content := fmt.Sprintf("%s %s\n%s %s", plugin.Name, version, "non-existant-tool", version) + writeVersionFile(t, currentDir, content) + + err := InstallAll(conf, currentDir, &stdout, &stderr) + assert.Len(t, err, 1, "expected 1 install error") + assert.ErrorContains(t, err[0], "missing plugin for") + + assertVersionInstalled(t, conf.DataDir, plugin.Name, version) + }) } func TestInstall(t *testing.T) { From 27b21d26f5261262ffa6b5bf67e331bc584d63f7 Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Tue, 18 Feb 2025 00:14:15 -0500 Subject: [PATCH 3/9] Recursively find defined plugins --- internal/execenv/execenv_test.go | 1 - internal/resolve/resolve.go | 106 ++++++++++++++++++++++------- internal/versions/versions.go | 26 +++---- internal/versions/versions_test.go | 1 + 4 files changed, 95 insertions(+), 39 deletions(-) diff --git a/internal/execenv/execenv_test.go b/internal/execenv/execenv_test.go index f0bbe3b13..6d631b621 100644 --- a/internal/execenv/execenv_test.go +++ b/internal/execenv/execenv_test.go @@ -49,7 +49,6 @@ func TestMergeEnv(t *testing.T) { } func TestGenerate(t *testing.T) { - t.Run("returns map of environment variables", func(t *testing.T) { testDataDir := t.TempDir() conf := config.Config{DataDir: testDataDir} diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 7f2a2256a..da02dea99 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -5,6 +5,7 @@ package resolve import ( "fmt" + "iter" "os" "path" "strings" @@ -21,50 +22,104 @@ type ToolVersions struct { Source string } +// AllVersions takes a set of plugins and a directory and resolves all tools to one or more +// versions. This includes tools without a corresponding plugin. +func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) (versions map[string]ToolVersions, err error) { + resolvedToolVersions := map[string]ToolVersions{} + for _, plugin := range plugins { + version, envVariableName, found := findVersionsInEnv(plugin.Name) + if found { + resolvedToolVersions[plugin.Name] = ToolVersions{Versions: version, Source: envVariableName} + } + } + + for iterDir := range iterDirectories(conf, directory) { + for _, plugin := range plugins { + if _, isPluginResolved := resolvedToolVersions[plugin.Name]; !isPluginResolved { + version, found, err := findLegacyVersionsInDir(conf, plugin, iterDir) + if err != nil { + return versions, err + } + if found { + resolvedToolVersions[plugin.Name] = version + } + } + } + + filepath := path.Join(iterDir, conf.DefaultToolVersionsFilename) + if _, err = os.Stat(filepath); err == nil { + if allVersions, err := toolversions.GetAllToolsAndVersions(filepath); err == nil { + for _, v := range allVersions { + if _, isPluginResolved := resolvedToolVersions[v.Name]; !isPluginResolved { + resolvedToolVersions[v.Name] = ToolVersions{Versions: v.Versions, Source: conf.DefaultToolVersionsFilename, Directory: iterDir} + } + } + } + } + } + return resolvedToolVersions, nil +} + // Version takes a plugin and a directory and resolves the tool to one or more -// versions. +// versions. Only returns results for the provided plugin. func Version(conf config.Config, plugin plugins.Plugin, directory string) (versions ToolVersions, found bool, err error) { version, envVariableName, found := findVersionsInEnv(plugin.Name) if found { return ToolVersions{Versions: version, Source: envVariableName}, true, nil } - for !found { - versions, found, err = findVersionsInDir(conf, plugin, directory) + for iterDir := range iterDirectories(conf, directory) { + versions, found, err = findVersionsInDir(conf, plugin, iterDir) if err != nil { return versions, false, err } + if found { + return versions, found, err + } + } + return versions, found, err +} - nextDir := path.Dir(directory) - // If current dir and next dir are the same it means we've reached `/` and - // have no more parent directories to search. - if nextDir == directory { - // If no version found, try current users home directory. I'd like to - // eventually remove this feature. - homeDir, osErr := os.UserHomeDir() - if osErr != nil { +func iterDirectories(conf config.Config, directory string) iter.Seq[string] { + return func(yield func(string) bool) { + if !yield(directory) { + return + } + iterDir := directory + for { + nextDir := path.Dir(iterDir) + // If current dir and next dir are the same it means we've reached `/` and + // have no more parent directories to search. + if nextDir == iterDir { break } - - versions, found, err = findVersionsInDir(conf, plugin, homeDir) - break + if !yield(iterDir) { + return + } + iterDir = nextDir + } + // If no version found, try current users home directory. I'd like to + // eventually remove this feature. + homeDir := conf.Home + // homeDir, osErr := os.UserHomeDir() + if homeDir != "" { + if !yield(homeDir) { + return + } } - directory = nextDir } - - return versions, found, err } func findVersionsInDir(conf config.Config, plugin plugins.Plugin, directory string) (versions ToolVersions, found bool, err error) { - filepath := path.Join(directory, conf.DefaultToolVersionsFilename) - - if _, err = os.Stat(filepath); err == nil { - versions, found, err := toolversions.FindToolVersions(filepath, plugin.Name) - if found || err != nil { - return ToolVersions{Versions: versions, Source: conf.DefaultToolVersionsFilename, Directory: directory}, found, err - } + versions, found, err = findLegacyVersionsInDir(conf, plugin, directory) + if found || err != nil { + return versions, found, err } + return versions, found, nil +} + +func findLegacyVersionsInDir(conf config.Config, plugin plugins.Plugin, directory string) (versions ToolVersions, found bool, err error) { legacyFiles, err := conf.LegacyVersionFile() if err != nil { return versions, found, err @@ -77,8 +132,7 @@ func findVersionsInDir(conf config.Config, plugin plugins.Plugin, directory stri return versions, found, err } } - - return versions, found, nil + return versions, false, nil } // findVersionsInEnv returns the version from the environment if present diff --git a/internal/versions/versions.go b/internal/versions/versions.go index a45983fff..d85563185 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "os" - "path" "regexp" "strings" @@ -84,27 +83,30 @@ func InstallAll(conf config.Config, dir string, stdOut io.Writer, stdErr io.Writ return []error{fmt.Errorf("unable to list plugins: %w", err)} } - toolNames := map[string]bool{} - filepath := path.Join(dir, conf.DefaultToolVersionsFilename) - toolVersions, err := toolversions.GetAllToolsAndVersions(filepath) - if err == nil { - for _, version := range toolVersions { - toolNames[version.Name] = true - } + toolVersions, err := resolve.AllVersions(conf, plugins, dir) + if err != nil { + return []error{fmt.Errorf("unable to resolve versions: %w", err)} } // Ideally we should install these in the order they are specified in the // closest .tool-versions file, but for now that is too complicated to // implement. for _, plugin := range plugins { - delete(toolNames, plugin.Name) - err := Install(conf, plugin, dir, stdOut, stdErr) - if err != nil { + if toolVersion, isPluginResolved := toolVersions[plugin.Name]; isPluginResolved { + delete(toolVersions, plugin.Name) + for _, version := range toolVersion.Versions { + err := InstallOneVersion(conf, plugin, version, false, stdOut, stdErr) + if err != nil { + failures = append(failures, err) + } + } + } else { + err := NoVersionSetError{toolName: plugin.Name} failures = append(failures, err) } } - for toolName := range toolNames { + for toolName := range toolVersions { err = MissingPluginError{toolName: toolName} failures = append(failures, err) } diff --git a/internal/versions/versions_test.go b/internal/versions/versions_test.go index 24a2fae05..d748bf85c 100644 --- a/internal/versions/versions_test.go +++ b/internal/versions/versions_test.go @@ -485,6 +485,7 @@ func generateConfig(t *testing.T) (config.Config, plugins.Plugin) { conf, err := config.LoadConfig() assert.Nil(t, err) conf.DataDir = testDataDir + conf.Home = testDataDir _, err = repotest.InstallPlugin("dummy_plugin", testDataDir, testPluginName) assert.Nil(t, err) From e79323d2a0b8b1e01e8cde9a88b253075977648d Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Tue, 18 Feb 2025 21:25:40 -0500 Subject: [PATCH 4/9] Preserve order in which tools are defined --- internal/resolve/resolve.go | 31 +++++++++++++++++++++++-------- internal/versions/versions.go | 23 ++++++++++++----------- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index da02dea99..e86409f7c 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -17,6 +17,7 @@ import ( // ToolVersions represents a tool along with versions specified for it type ToolVersions struct { + Name string Versions []string Directory string Source string @@ -24,12 +25,15 @@ type ToolVersions struct { // AllVersions takes a set of plugins and a directory and resolves all tools to one or more // versions. This includes tools without a corresponding plugin. -func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) (versions map[string]ToolVersions, err error) { - resolvedToolVersions := map[string]ToolVersions{} +func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) (versions []ToolVersions, err error) { + resolvedToolVersions := map[string]bool{} + var finalVersions []ToolVersions + for _, plugin := range plugins { version, envVariableName, found := findVersionsInEnv(plugin.Name) if found { - resolvedToolVersions[plugin.Name] = ToolVersions{Versions: version, Source: envVariableName} + resolvedToolVersions[plugin.Name] = true + finalVersions = append(finalVersions, ToolVersions{Name: plugin.Name, Versions: version, Source: envVariableName}) } } @@ -41,7 +45,8 @@ func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) return versions, err } if found { - resolvedToolVersions[plugin.Name] = version + resolvedToolVersions[plugin.Name] = true + finalVersions = append(finalVersions, version) } } } @@ -51,13 +56,14 @@ func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) if allVersions, err := toolversions.GetAllToolsAndVersions(filepath); err == nil { for _, v := range allVersions { if _, isPluginResolved := resolvedToolVersions[v.Name]; !isPluginResolved { - resolvedToolVersions[v.Name] = ToolVersions{Versions: v.Versions, Source: conf.DefaultToolVersionsFilename, Directory: iterDir} + resolvedToolVersions[v.Name] = true + finalVersions = append(finalVersions, ToolVersions{Name: v.Name, Versions: v.Versions, Source: conf.DefaultToolVersionsFilename, Directory: iterDir}) } } } } } - return resolvedToolVersions, nil + return finalVersions, nil } // Version takes a plugin and a directory and resolves the tool to one or more @@ -65,7 +71,7 @@ func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) func Version(conf config.Config, plugin plugins.Plugin, directory string) (versions ToolVersions, found bool, err error) { version, envVariableName, found := findVersionsInEnv(plugin.Name) if found { - return ToolVersions{Versions: version, Source: envVariableName}, true, nil + return ToolVersions{Name: plugin.Name, Versions: version, Source: envVariableName}, true, nil } for iterDir := range iterDirectories(conf, directory) { @@ -116,6 +122,15 @@ func findVersionsInDir(conf config.Config, plugin plugins.Plugin, directory stri return versions, found, err } + filepath := path.Join(directory, conf.DefaultToolVersionsFilename) + + if _, err = os.Stat(filepath); err == nil { + versions, found, err := toolversions.FindToolVersions(filepath, plugin.Name) + if found || err != nil { + return ToolVersions{Name: plugin.Name, Versions: versions, Source: conf.DefaultToolVersionsFilename, Directory: directory}, found, err + } + } + return versions, found, nil } @@ -165,7 +180,7 @@ func findVersionsInLegacyFile(plugin plugins.Plugin, directory string) (versions if len(versionsSlice) == 0 || (len(versionsSlice) == 1 && versionsSlice[0] == "") { return versions, false, nil } - return ToolVersions{Versions: versionsSlice, Source: filename, Directory: directory}, err == nil, err + return ToolVersions{Name: plugin.Name, Versions: versionsSlice, Source: filename, Directory: directory}, err == nil, err } } diff --git a/internal/versions/versions.go b/internal/versions/versions.go index d85563185..352cd5dd3 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -78,22 +78,23 @@ func (e VersionAlreadyInstalledError) Error() string { // installed, but it may be multiple versions if multiple versions for the tool // are specified in the .tool-versions file. func InstallAll(conf config.Config, dir string, stdOut io.Writer, stdErr io.Writer) (failures []error) { - plugins, err := plugins.List(conf, false, false) + installedPlugins, err := plugins.List(conf, false, false) if err != nil { return []error{fmt.Errorf("unable to list plugins: %w", err)} } + pluginsMap := map[string]plugins.Plugin{} + for _, plugin := range installedPlugins { + pluginsMap[plugin.Name] = plugin + } - toolVersions, err := resolve.AllVersions(conf, plugins, dir) + toolVersions, err := resolve.AllVersions(conf, installedPlugins, dir) if err != nil { return []error{fmt.Errorf("unable to resolve versions: %w", err)} } - // Ideally we should install these in the order they are specified in the - // closest .tool-versions file, but for now that is too complicated to - // implement. - for _, plugin := range plugins { - if toolVersion, isPluginResolved := toolVersions[plugin.Name]; isPluginResolved { - delete(toolVersions, plugin.Name) + for _, toolVersion := range toolVersions { + if plugin, isPluginResolved := pluginsMap[toolVersion.Name]; isPluginResolved { + delete(pluginsMap, plugin.Name) for _, version := range toolVersion.Versions { err := InstallOneVersion(conf, plugin, version, false, stdOut, stdErr) if err != nil { @@ -101,13 +102,13 @@ func InstallAll(conf config.Config, dir string, stdOut io.Writer, stdErr io.Writ } } } else { - err := NoVersionSetError{toolName: plugin.Name} + err = MissingPluginError{toolName: toolVersion.Name} failures = append(failures, err) } } - for toolName := range toolVersions { - err = MissingPluginError{toolName: toolName} + for _, plugin := range pluginsMap { + err := NoVersionSetError{toolName: plugin.Name} failures = append(failures, err) } From 554f6f978960cdd0c09c763d339892135e06a4ea Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Tue, 18 Feb 2025 21:43:15 -0500 Subject: [PATCH 5/9] Resolve tool version before legacy --- internal/resolve/resolve.go | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index e86409f7c..9c0316e7f 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -29,6 +29,7 @@ func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) resolvedToolVersions := map[string]bool{} var finalVersions []ToolVersions + // First: Resolve using environment values for _, plugin := range plugins { version, envVariableName, found := findVersionsInEnv(plugin.Name) if found { @@ -37,7 +38,22 @@ func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) } } + // Iterate from the nearest to furthest directory, ending with the user's home. for iterDir := range iterDirectories(conf, directory) { + // Second: Resolve using the tool versions file + filepath := path.Join(iterDir, conf.DefaultToolVersionsFilename) + if _, err = os.Stat(filepath); err == nil { + if allVersions, err := toolversions.GetAllToolsAndVersions(filepath); err == nil { + for _, version := range allVersions { + if _, isPluginResolved := resolvedToolVersions[version.Name]; !isPluginResolved { + resolvedToolVersions[version.Name] = true + finalVersions = append(finalVersions, ToolVersions{Name: version.Name, Versions: version.Versions, Source: conf.DefaultToolVersionsFilename, Directory: iterDir}) + } + } + } + } + + // Third: Resolve using legacy settings for _, plugin := range plugins { if _, isPluginResolved := resolvedToolVersions[plugin.Name]; !isPluginResolved { version, found, err := findLegacyVersionsInDir(conf, plugin, iterDir) @@ -50,18 +66,6 @@ func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) } } } - - filepath := path.Join(iterDir, conf.DefaultToolVersionsFilename) - if _, err = os.Stat(filepath); err == nil { - if allVersions, err := toolversions.GetAllToolsAndVersions(filepath); err == nil { - for _, v := range allVersions { - if _, isPluginResolved := resolvedToolVersions[v.Name]; !isPluginResolved { - resolvedToolVersions[v.Name] = true - finalVersions = append(finalVersions, ToolVersions{Name: v.Name, Versions: v.Versions, Source: conf.DefaultToolVersionsFilename, Directory: iterDir}) - } - } - } - } } return finalVersions, nil } @@ -117,13 +121,7 @@ func iterDirectories(conf config.Config, directory string) iter.Seq[string] { } func findVersionsInDir(conf config.Config, plugin plugins.Plugin, directory string) (versions ToolVersions, found bool, err error) { - versions, found, err = findLegacyVersionsInDir(conf, plugin, directory) - if found || err != nil { - return versions, found, err - } - filepath := path.Join(directory, conf.DefaultToolVersionsFilename) - if _, err = os.Stat(filepath); err == nil { versions, found, err := toolversions.FindToolVersions(filepath, plugin.Name) if found || err != nil { @@ -131,7 +129,7 @@ func findVersionsInDir(conf config.Config, plugin plugins.Plugin, directory stri } } - return versions, found, nil + return findLegacyVersionsInDir(conf, plugin, directory) } func findLegacyVersionsInDir(conf config.Config, plugin plugins.Plugin, directory string) (versions ToolVersions, found bool, err error) { From 7ea9344e2c489423ddf6bd4b9f42c863e3a678ac Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Thu, 20 Feb 2025 23:18:11 -0500 Subject: [PATCH 6/9] Clean up how home is passed around --- internal/config/config.go | 4 +++- internal/resolve/resolve.go | 20 +++++++++----------- internal/versions/versions_test.go | 3 ++- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index d72907ff1..7e838a04b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -5,10 +5,12 @@ package config import ( "context" "io/fs" + "os" "strconv" "strings" "github.com/mitchellh/go-homedir" + "github.com/sethvargo/go-envconfig" "gopkg.in/ini.v1" ) @@ -100,7 +102,7 @@ func LoadConfig() (Config, error) { return config, err } - homeDir, err := homedir.Dir() + homeDir, err := os.UserHomeDir() if err != nil { return config, err } diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 9c0316e7f..1f224ad6c 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -43,12 +43,14 @@ func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) // Second: Resolve using the tool versions file filepath := path.Join(iterDir, conf.DefaultToolVersionsFilename) if _, err = os.Stat(filepath); err == nil { - if allVersions, err := toolversions.GetAllToolsAndVersions(filepath); err == nil { - for _, version := range allVersions { - if _, isPluginResolved := resolvedToolVersions[version.Name]; !isPluginResolved { - resolvedToolVersions[version.Name] = true - finalVersions = append(finalVersions, ToolVersions{Name: version.Name, Versions: version.Versions, Source: conf.DefaultToolVersionsFilename, Directory: iterDir}) - } + allVersions, err := toolversions.GetAllToolsAndVersions(filepath) + if err != nil { + return versions, err + } + for _, version := range allVersions { + if _, isPluginResolved := resolvedToolVersions[version.Name]; !isPluginResolved { + resolvedToolVersions[version.Name] = true + finalVersions = append(finalVersions, ToolVersions{Name: version.Name, Versions: version.Versions, Source: conf.DefaultToolVersionsFilename, Directory: iterDir}) } } } @@ -80,10 +82,7 @@ func Version(conf config.Config, plugin plugins.Plugin, directory string) (versi for iterDir := range iterDirectories(conf, directory) { versions, found, err = findVersionsInDir(conf, plugin, iterDir) - if err != nil { - return versions, false, err - } - if found { + if found || err != nil { return versions, found, err } } @@ -111,7 +110,6 @@ func iterDirectories(conf config.Config, directory string) iter.Seq[string] { // If no version found, try current users home directory. I'd like to // eventually remove this feature. homeDir := conf.Home - // homeDir, osErr := os.UserHomeDir() if homeDir != "" { if !yield(homeDir) { return diff --git a/internal/versions/versions_test.go b/internal/versions/versions_test.go index d748bf85c..ff4dd2559 100644 --- a/internal/versions/versions_test.go +++ b/internal/versions/versions_test.go @@ -482,10 +482,11 @@ func assertNotInstalled(t *testing.T, dataDir, pluginName, version string) { func generateConfig(t *testing.T) (config.Config, plugins.Plugin) { t.Helper() testDataDir := t.TempDir() + homeDir := t.TempDir() conf, err := config.LoadConfig() assert.Nil(t, err) conf.DataDir = testDataDir - conf.Home = testDataDir + conf.Home = homeDir _, err = repotest.InstallPlugin("dummy_plugin", testDataDir, testPluginName) assert.Nil(t, err) From 8cff79dce906fc6b248d90fbc023518169754e17 Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Thu, 20 Feb 2025 23:47:31 -0500 Subject: [PATCH 7/9] Add tests to AllVersions --- internal/resolve/resolve_test.go | 73 +++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index d4a51f897..32ed53dbd 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -18,7 +18,8 @@ const testPluginName = "test-plugin" func TestVersion(t *testing.T) { testDataDir := t.TempDir() currentDir := t.TempDir() - conf := config.Config{DataDir: testDataDir, DefaultToolVersionsFilename: ".tool-versions", ConfigFile: "testdata/asdfrc"} + homeDir := t.TempDir() + conf := config.Config{Home: homeDir, DataDir: testDataDir, DefaultToolVersionsFilename: ".tool-versions", ConfigFile: "testdata/asdfrc"} _, err := repotest.InstallPlugin("dummy_plugin", conf.DataDir, testPluginName) assert.Nil(t, err) plugin := plugins.New(conf, testPluginName) @@ -72,6 +73,76 @@ func TestVersion(t *testing.T) { }) } +func TestAllVersions(t *testing.T) { + testDataDir := t.TempDir() + currentDir := t.TempDir() + homeDir := t.TempDir() + conf := config.Config{Home: homeDir, DataDir: testDataDir, DefaultToolVersionsFilename: ".tool-versions", ConfigFile: "testdata/asdfrc"} + _, err := repotest.InstallPlugin("dummy_plugin", conf.DataDir, testPluginName) + assert.Nil(t, err) + allPlugins := []plugins.Plugin{plugins.New(conf, testPluginName)} + + t.Run("returns empty slice when non-existent version passed", func(t *testing.T) { + toolVersions, err := AllVersions(conf, allPlugins, t.TempDir()) + assert.Nil(t, err) + assert.Empty(t, toolVersions) + }) + + t.Run("returns single version from .tool-versions file", func(t *testing.T) { + // write a version file + data := []byte(fmt.Sprintf("%s 1.2.3", testPluginName)) + err = os.WriteFile(filepath.Join(currentDir, ".tool-versions"), data, 0o666) + + toolVersions, err := AllVersions(conf, allPlugins, currentDir) + assert.Nil(t, err) + assert.Equal(t, toolVersions[0].Versions, []string{"1.2.3"}) + }) + + t.Run("returns version from env when env variable set", func(t *testing.T) { + // Set env + t.Setenv(fmt.Sprintf("ASDF_%s_VERSION", strings.ToUpper(testPluginName)), "2.3.4") + + // write a version file + data := []byte(fmt.Sprintf("%s 1.2.3", testPluginName)) + err = os.WriteFile(filepath.Join(currentDir, ".tool-versions"), data, 0o666) + + // assert env variable takes precedence + toolVersions, err := AllVersions(conf, allPlugins, currentDir) + assert.Nil(t, err) + assert.Equal(t, toolVersions[0].Versions, []string{"2.3.4"}) + }) + + t.Run("returns single version from .tool-versions file in parent directory", func(t *testing.T) { + // write a version file + data := []byte(fmt.Sprintf("%s 1.2.3", testPluginName)) + err = os.WriteFile(filepath.Join(currentDir, ".tool-versions"), data, 0o666) + + subDir := filepath.Join(currentDir, "subdir") + err = os.MkdirAll(subDir, 0o777) + assert.Nil(t, err) + + toolVersion, err := AllVersions(conf, allPlugins, subDir) + assert.Nil(t, err) + assert.Equal(t, toolVersion[0].Versions, []string{"1.2.3"}) + }) + + t.Run("returns unknown plugin version from .tool-versions file in parent directory", func(t *testing.T) { + // write a version file + unknownPluginName := "dummy_unknown_plugin" + data := []byte(fmt.Sprintf("%s 1.2.3", unknownPluginName)) + err = os.WriteFile(filepath.Join(currentDir, ".tool-versions"), data, 0o666) + + subDir := filepath.Join(currentDir, "subdir") + err = os.MkdirAll(subDir, 0o777) + assert.Nil(t, err) + + toolVersion, err := AllVersions(conf, allPlugins, subDir) + assert.Nil(t, err) + assert.Equal(t, toolVersion[0].Name, unknownPluginName) + assert.Equal(t, toolVersion[0].Versions, []string{"1.2.3"}) + }) +} + func TestFindVersionsInDir(t *testing.T) { testDataDir := t.TempDir() conf := config.Config{DataDir: testDataDir, DefaultToolVersionsFilename: ".tool-versions", ConfigFile: "testdata/asdfrc"} From 5272a177058cb1bfdaf052f92731ac25968e4092 Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Fri, 21 Feb 2025 00:06:38 -0500 Subject: [PATCH 8/9] Add more test coverage --- .gitignore | 1 + internal/config/config.go | 1 - internal/resolve/resolve.go | 2 +- internal/resolve/resolve_test.go | 19 +++++++++++++++++++ 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index b1d67e8e2..80479d845 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ repository .vagrant keyrings /tmp +.idea dist/ diff --git a/internal/config/config.go b/internal/config/config.go index 7e838a04b..51e1e78ac 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/mitchellh/go-homedir" - "github.com/sethvargo/go-envconfig" "gopkg.in/ini.v1" ) diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 1f224ad6c..3932af987 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -38,7 +38,7 @@ func AllVersions(conf config.Config, plugins []plugins.Plugin, directory string) } } - // Iterate from the nearest to furthest directory, ending with the user's home. + // Iterate from the current towards the root directory, ending with the user's home. for iterDir := range iterDirectories(conf, directory) { // Second: Resolve using the tool versions file filepath := path.Join(iterDir, conf.DefaultToolVersionsFilename) diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index 32ed53dbd..05f670463 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -141,6 +141,25 @@ func TestAllVersions(t *testing.T) { assert.Equal(t, toolVersion[0].Name, unknownPluginName) assert.Equal(t, toolVersion[0].Versions, []string{"1.2.3"}) }) + + t.Run("returns results in order from .tool-versions file", func(t *testing.T) { + testPluginTwoName := "dummy_plugin_two" + _, err := repotest.InstallPlugin("dummy_plugin", conf.DataDir, testPluginTwoName) + assert.Nil(t, err) + allPlugins := []plugins.Plugin{plugins.New(conf, testPluginName), plugins.New(conf, testPluginTwoName)} + + // write a version file + unknownPluginName := "dummy_unknown_plugin" + data := []byte(fmt.Sprintf("%s 1.2.3\n%s 1.2.3\n%s 1.2.3", testPluginTwoName, testPluginName, unknownPluginName)) + err = os.WriteFile(filepath.Join(currentDir, ".tool-versions"), data, 0o666) + assert.Nil(t, err) + + toolVersion, err := AllVersions(conf, allPlugins, currentDir) + assert.Nil(t, err) + assert.Equal(t, toolVersion[0].Name, testPluginTwoName) + assert.Equal(t, toolVersion[1].Name, testPluginName) + assert.Equal(t, toolVersion[2].Name, unknownPluginName) + }) } func TestFindVersionsInDir(t *testing.T) { From 0551296e4c9ca3fc46942fde793b5fcd9ea82b89 Mon Sep 17 00:00:00 2001 From: Andre Cloutier Date: Fri, 21 Feb 2025 18:26:40 -0500 Subject: [PATCH 9/9] Add a homedir test, and cleanup --- internal/resolve/resolve.go | 15 +++++++-------- internal/resolve/resolve_test.go | 10 ++++++++++ internal/shims/shims_test.go | 1 + 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 3932af987..0e8a1ef20 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -121,9 +121,12 @@ func iterDirectories(conf config.Config, directory string) iter.Seq[string] { func findVersionsInDir(conf config.Config, plugin plugins.Plugin, directory string) (versions ToolVersions, found bool, err error) { filepath := path.Join(directory, conf.DefaultToolVersionsFilename) if _, err = os.Stat(filepath); err == nil { - versions, found, err := toolversions.FindToolVersions(filepath, plugin.Name) - if found || err != nil { - return ToolVersions{Name: plugin.Name, Versions: versions, Source: conf.DefaultToolVersionsFilename, Directory: directory}, found, err + foundVersions, found, err := toolversions.FindToolVersions(filepath, plugin.Name) + if err != nil { + return versions, found, err + } + if found { + return ToolVersions{Name: plugin.Name, Versions: foundVersions, Source: conf.DefaultToolVersionsFilename, Directory: directory}, found, err } } @@ -137,11 +140,7 @@ func findLegacyVersionsInDir(conf config.Config, plugin plugins.Plugin, director } if legacyFiles { - versions, found, err := findVersionsInLegacyFile(plugin, directory) - - if found || err != nil { - return versions, found, err - } + return findVersionsInLegacyFile(plugin, directory) } return versions, false, nil } diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index 05f670463..03b572715 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -126,6 +126,16 @@ func TestAllVersions(t *testing.T) { assert.Equal(t, toolVersion[0].Versions, []string{"1.2.3"}) }) + t.Run("returns single version from .tool-versions file in home directory", func(t *testing.T) { + // write a version file + data := []byte(fmt.Sprintf("%s 1.2.3", testPluginName)) + err = os.WriteFile(filepath.Join(homeDir, ".tool-versions"), data, 0o666) + + toolVersion, err := AllVersions(conf, allPlugins, currentDir) + assert.Nil(t, err) + assert.Equal(t, toolVersion[0].Versions, []string{"1.2.3"}) + }) + t.Run("returns unknown plugin version from .tool-versions file in parent directory", func(t *testing.T) { // write a version file unknownPluginName := "dummy_unknown_plugin" diff --git a/internal/shims/shims_test.go b/internal/shims/shims_test.go index 598b1bed3..c8c98ee6c 100644 --- a/internal/shims/shims_test.go +++ b/internal/shims/shims_test.go @@ -442,6 +442,7 @@ func generateConfig(t *testing.T) (config.Config, plugins.Plugin) { conf, err := config.LoadConfig() assert.Nil(t, err) conf.DataDir = testDataDir + conf.Home = t.TempDir() return conf, installPlugin(t, conf, "dummy_plugin", testPluginName) }