Skip to content

Conversation

@drornir
Copy link
Contributor

@drornir drornir commented Nov 22, 2024

According to https://go.dev/ref/mod#go-mod-file-go, prior to Go 1.21, the go directive was option.
This seems to not be supported in bingo. This commit only changes the fallback value used when a go.mod file does not contain the version. This semantically means that any go version should be able to compile this.

I encountered the issue while trying to use bingo to install github.com/betacraft/easytags Their go.mod is completely empty, which causes a panic on

if semver.MustParse(targetModParsed.GoVersion()).GreaterThan(runnable.GoVersion()) {

get.go:723


Steps to recreate:

bingo get github.com/betacraft/easytags@latest

Thanks for the great tool!

According to https://go.dev/ref/mod#go-mod-file-go, prior to Go 1.21,
the go directive was option.
This seems to not be supported in bingo. This commit only changes the fallback value used when a go.mod file does not contain the version. This semantically means that any go version should be able to compile this.

I encountered the issue while trying to use bingo to install github.com/betacraft/easytags
Their go.mod is completely empty, which causes a panic on

if semver.MustParse(targetModParsed.GoVersion()).GreaterThan(runnable.GoVersion()) {

`get.go:723`
@drornir drornir changed the title Fix: workaround for using tools with no ge directive in their go.mod Fix: workaround for using tools with no go directive in their go.mod Dec 8, 2024
@drornir
Copy link
Contributor Author

drornir commented Dec 8, 2024

@bwplotka do i need to improve something in order to get this merged? thanks!

Copy link
Owner

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bwplotka do i need to improve something in order to get this merged? thanks!

no, but ping on CNCF Slack would help, E_TOO_MANY_NOTIFICATIONS (:

Thanks, test would be nice, but I trust you here. Thanks!

@bwplotka
Copy link
Owner

bwplotka commented Dec 9, 2024

Do you mind pushing empty commit or adding a comment why 1.0? We need to retrigger CI, sorry for that ):

@drornir
Copy link
Contributor Author

drornir commented Dec 9, 2024

@bwplotka heya! thanks!
I'll hop on slack if you miss this comment ;)

I pushed a short doc and a test fix, but I think you need to approve the github actions to run, which AFAIK is a separate thing from approving the PR itself

See point no 5 in this link

@bwplotka bwplotka merged commit eebab19 into bwplotka:main Dec 9, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants