Use the go version in go.mod to set the minimal Go version to use

[edmorley]

Currently this buildpack requires that the Go version be specified in a Heroku-specific format in go.mod, otherwise the build will output something like:

 !!    The go.mod file for this project does not specify a Go version
 !!    
 !!    Defaulting to go1.12.12
 !!    
 !!    For more details see: https://devcenter.heroku.com/articles/go-apps-with-modules#build-configuration
 !!    

At first glance it seems like the buildpack should just be using the go 1.NN style annotations now supported by Go in go.mod.

However as discussed in #301, those annotations are meant to indicate the minimum language feature version required, and not necessarily the compiler version to use. (For example go.mod might say it requires go 1.12, which a go 1.13 compiler could support by running in "go 1.12 mode").

Whilst that may be true, I suspect in the common case, people just want their entire app to run using the same version in production, CI, ... - and will never need to use a mixture of language/compiler versions.

The current implementation (requiring Heroku-specific annotations) not only adds to the boilerplate required for projects using this buildpack, but can also mean projects unknowingly run a different version of Go in production.

For example in https://github.com/heroku/release-dyno-watcher/pull/191 that service was upgraded to Go 1.13, and whilst CI is now testing against Go 1.13, production is still running Go 1.12 per the buildpack default (which was not spotted since the build "The go.mod file for this project does not specify a Go version" warning is only a warning not fatal).

As such, would it be possible to either:

1. Have the standard go.mod version set the minimal version of the Go compiler to use (iff the user hasn't overridden with the // +heroku goVersion style syntax)
2. Have the standard go.mod version set the specific major version of the Go compiler to use (iff the user hasn't overridden with the // +heroku goVersion style syntax)

Whilst (1) might be closer to the intent of the upstream Go project (see #301 (comment)), I personally would prefer (2) since it offers the least surprise.

[ghost]

I've re-read all of the posts involved with this directive that I can find and I think the only "safe" thing to do is, in the absence of the // +heroku goVersion directive, utilize an existing go directive as the minimal major Go version. This will allow users to specify, via the // +heroku goVersion directive, a newer Go version than the go directive indicates, allowing for a toolchain update w/o requiring a "language" version change, protecting against a future where that would be necessary.

I am concerned about users trying to pin to a minor version using the go directive though. Support for minor versions is ambiguous at best, although appear to work ATM on a few random versions of go that I tested locally with. I am worried that there will be pressure to support pinning via the go directive,
it working for some amount of time and then broken by the Go team because version selection of the toolchain is not the intended use case.

[ghost]

Also, if its the minimal version, then when we default to go1.14 and their module file still contains go 1.13, what do we do? I expect users will expect that their compiles continue to use the latest go1.13.

So maybe it needs to be option 2 - specific major version.

[edmorley]

Yeah I agree that option 2 sounds best.

[ghost]

Internal Work Item: W-6993075

[abitrolly]

I am using go 1.14 with go 1.13 line in go.mod without any Heroku specific modifications. And while everything works ok on my machine, the buildpack fails.

        ./main.go:34:6: undefined: errors.As
       note: module requires Go 1.13

I think it would fix all the problems if the buildpack could just detect and bump the minimal viable version to 1.13 without CVEs.

[abitrolly]

@edmorley I fixed the issue in #411, but I need help correcting the proxy tests, which may rely on some specific features of Go 1.12.

[abitrolly]

#411 is merged, so this issue will be fixed in the next buildpack version aka v162.

[joshwlewis]

This has shipped with v163!

[edmorley]

(I forgot it was me that filed this! 😆 )