asdf-golang icon indicating copy to clipboard operation
asdf-golang copied to clipboard

Don't compare extracted version with list of currently installed versions

Open pclalv opened this issue 3 years ago • 1 comments

background

I installed this plugin and tried using it to install a golang version. I had no existing golang installations. I wasn't able to install any golang version already specified in a legacy file. Here's a reproducible illustration of what happens:

[~/code/go-test]
# ls -la
total 0
drwxr-xr-x  2 me  64 Feb 18 14:16 .
drwxr-xr-x 17 me 544 Feb 18 14:05 ..

[~/code/go-test]
# cat ~/.asdfrc
legacy_version_file = yes

[~/code/go-test]
# echo 'go 1.17.2' > go.mod

[~/code/go-test]
# asdf install golang
  No versions installed
No versions specified for golang in config files or environment

[~/code/go-test]
# asdf list golang
  No versions installed

note that asdf install golang shows the same stderr output at asdf list golang.

analysis

turns out, that's because parse-legacy-file invokes asdf list golang in order to compare the go version it extracts to that list of versions.

this is a problem for me, because I don't have any golang versions installed; even if I did, the behavior seems incorrect, as it seems to require that a version to be installed already is installed.

solution

I opted to nix that whole asdf list golang pipeline, and simplify the sed command that strips go from the version string.

I've also provided some basic tests to validate the behavior before and after making changes to the script.

notes

  • I'm not sure what the expected format .go-version is, but this code should be compatible with the only example of `.go-version that I could find: https://github.com/hashicorp/terraform/blob/main/.go-version

pclalv avatar Feb 18 '22 19:02 pclalv

@pclalv I appreciate the PR and the tests. (I really should write more of those for this).

The current behavior is actually noted in the README and is actually in keeping with the Go module reference. In the case where you don't have any go version installed I could see a case for falling back to the latest compatible from list-all, but that might be confusing to folks too. Maybe error if there's no version installed?

kennyp avatar Feb 28 '22 19:02 kennyp

The actual bug here should be fixed by #93, but if you have time to refactor, I'd love to include your tests with the updated bin/parse-legacy-file.

kennyp avatar Dec 17 '22 05:12 kennyp

Closing this and continuing discussion in issue #99

kennyp avatar May 09 '23 14:05 kennyp