Skip to content

Conversation

loreto
Copy link
Contributor

@loreto loreto commented Aug 31, 2022

Summary

Add version detection to the go planner based on the contents of go.mod. When we don't have a corresponding package in nixpkgs we default to 1.19

How was it tested?

Compiled and ran against local project.

@loreto loreto requested a review from gcurtis August 31, 2022 22:52
@RickyGrassmuck
Copy link

Awesome!

It's worth noting that if a user adds an additional go package(with a version that's different from the one in the go.mod file) to their devbox config, they will still run into a build error complaining about the conflict. IIRC, Nix has a way to set which package has priority so you can install multiple versions of a package so it may be worth adding a step to the build to handle that.

Not really sure how often someone will want multiple versions of a language in a devbox container but since it's a valid option it's probably worth having something in place to either handle it or warn the user when the scenario is detected.

Comment on lines +61 to +62
// Should we be throwing an error instead, if we don't have a nix package
// for the specified version of go?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be a good idea if we don't have a newer enough version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decided to keep: as long as we're updating this to always have the latest version of go, it should work ok. Later we can parse the versions and compare, so we only give the error if the version is greater.

}
}

func getVersion(gomodPath string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think getVersion is kind of vague because this lives in the same package as other planners. Maybe parseGoVersion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can also be a method on GoPlanner

python will have similar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing to parseGoVersion as requested.

@loreto loreto merged commit 36333f9 into main Sep 1, 2022
@loreto loreto deleted the daniel/go-versions branch September 1, 2022 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants