fix: use version in home dir when no version found in root dir #1883
Conversation
* Use version in home dir when no version found in root dir * Correct `FindExecutable` function so returns `NoVersionSetError` even when `.tool-version` file located * Update `resolve` and `versions` package tests to use test plugin name Fixes #1860
Stratus3D
force-pushed
the
tb/home-versions-as-default
branch
from
d686f8c to
864b120
Compare
| break | ||
| } | ||
|
|
||
| versions, found, err = findVersionsInDir(conf, plugin, homeDir) |
There was a problem hiding this comment.
This is the actual fix for the bug.
| func TestVersion(t *testing.T) { | ||
| testDataDir := t.TempDir() | ||
| currentDir := t.TempDir() | ||
| conf := config.Config{DataDir: testDataDir, DefaultToolVersionsFilename: ".tool-versions", ConfigFile: "testdata/asdfrc"} | ||
| _, err := repotest.InstallPlugin("dummy_plugin", conf.DataDir, "lua") |
There was a problem hiding this comment.
Tests were failing for me locally because I actually have the lua plugin installed on my computer. With the change above to make the home directory version the default these tests started picking up my own personal config, so I changed to a test plugin name.
| existingPluginToolVersions[plugin] = versions | ||
| if len(versions.Versions) > 0 { | ||
| existingPluginToolVersions[plugin] = versions | ||
| } |
There was a problem hiding this comment.
The code was setting a tool and version even when tempVersions was empty, which was wrong.
| // If no version found, try current users home directory. I'd like to | ||
| // eventually remove this feature. |
There was a problem hiding this comment.
I'd like to eventually remove this feature.
@Stratus3D It seems very useful — can you share why you want to remove it? If you've already elucidated elsewhere, feel free to just link me.
There was a problem hiding this comment.
@jsejcksn the more code I can remove or improve, the more maintainable this project becomes. In the code this is very clearly a "special case" and not necessary for typical. Can you share why you find it useful? I'm still formulating a plan to remove this and replace it with nothing. I've not written out anything yet.
There was a problem hiding this comment.
Can you share why you find it useful?
@Stratus3D I find it useful to be able to specify "default" versions for installed tools without needing to place a configuration file outside my home directory.
There was a problem hiding this comment.
@jsejcksn ok so I can see two reasons why you might want to do that:
- You're working in a directory outside of
$HOME. Say/var/log. If you are working in a directory off of/you probably should be root anyway, in which case creating a.tool-versionsfile in/should be no big deal. If you aren't root you probably shouldn't be touching anything outside of your own$HOME. - You want to set a tool versions for other tools that get built in temp directories outside of your own directory (say
/tmp/).
I don't think that first reason is valid. If you want to affect things in / you should be root and know what you are doing. For the second, that really sounds like a workaround to the lack of inter-tool dependencies in asdf. I've got a fairly simple solution to that. I've not implemented it yet but it should be a relatively simple change to plugins that need to depend on other plugins. Thoughts?
There was a problem hiding this comment.
If you aren't root you probably shouldn't be touching anything outside of your own
$HOME.
@Stratus3D This seems… opinionated.
I think it's reasonable to want to be able to set the default (global fallback) version of a tool for the current shell user without adding filesystem artifacts outside that user's home directory. I'd love to hear others' thoughts, too.
There was a problem hiding this comment.
Can you provide a specific example of a use case that requires you to set a tool version permanently in a directory outside of your $HOME directory?
There was a problem hiding this comment.
Can you provide a specific example of a use case that requires you to set a tool version permanently in a directory outside of your
$HOMEdirectory?
@Stratus3D I'm not positive whether or not I understand the question, but any tool which works with directories closer to the root than the user's home directory would qualify. For example, a tool that analyses the file system — like a homebrew package analysis tool which needs to work in /opt/homebrew on macOS.
FindExecutablefunction so returnsNoVersionSetErroreven when.tool-versionfile locatedresolveandversionspackage tests to use test plugin nameFixes #1860