search

heroku / heroku-buildpack-go

Heroku Go Buildpack
https://devcenter.heroku.com/categories/go
MIT License
792 stars 511 forks source link

Builds failing on Go 1.16rc1 #446

Open kainosnoema opened 3 years ago

kainosnoema commented 3 years ago

Ran into two separate issues with Go 1.16rc1, both related to changes in go install.

1. The +heroku install directive no longer works with packages outside of the module.

For example, I'm currently using it to install additional packages such as golang-migrate:

In go.mod: // +heroku install -tags postgres . github.com/golang-migrate/migrate/v4/cmd/migrate

But build setup is now failing with an error:

no required module provides package github.com/golang-migrate/migrate/v4/cmd/migrate; to add it:
    go get github.com/golang-migrate/migrate/v4/cmd/migrate

One workaround is to force the package to be included in the module by adding a tools.go package, but this wasn't previously required. Might be helpful to add some docs about this somewhere.

2. The bin/test command is failing because a package version isn't specified:

-----> Running Go buildpack tests...
go install: version is required when current directory is not in a module
    Try 'go install github.com/apg/patter@latest' to install the latest version
-----> Go buildpack tests failed with exit status 1
danp commented 3 years ago

Hey, thanks for this report. I believe these are both from the changes described in the Modules section of the (draft) release notes.

The bin/test issue is pretty straightforward to fix, I think, and I have an initial take at that locally.

For the +heroku install issue:

For your example, I think ultimately we'd like to run go install . for your app's module/command and go install -tags postgres github.com/golang-migrate/migrate/v4/cmd/migrate@latest (note @latest at the end) to get you the migrate command.

The changes to the go command in 1.16 won't allow mixing of versionless and versioned packages to go get, so for example taking your install line and adding @latest to the end of the migrate package path won't work. So, it seems they have to be separate go install invocations.

The current+heroku install processing only considers the first such line. It might be good to support 1+ of these, so then you could write:

// +heroku install .
// +heroku install -tags postgres github.com/golang-migrate/migrate/v4/cmd/migrate@latest

And each line would result in a separate invocation of go install.

I'll try that out locally. What do you think?

danp commented 3 years ago

Sorry I didn't call it out explicitly but you are totally right: another option for this is to say that with 1.16+ something like tools.go must be used. That is not my favorite since it does clutter your module with deps/etc related to the installed tool(s).

Another similar option is to not support installing extra things in 1.16+, instead recommending some other method be used to fetch needed binaries. For example, the inline buildpack could be used in addition to the Go buildpack to fetch and install the latest migrate binary.

danp commented 3 years ago

Hey there,

v152 of the buildpack was just published. That should fix the bin/test issue, please let me know if not.

I haven't had a chance to create a change/fix for +heroku install but hope to do that soon.