Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builds failing on Go 1.16rc1 #446

Open
kainosnoema opened this issue Feb 5, 2021 · 3 comments
Open

Builds failing on Go 1.16rc1 #446

kainosnoema opened this issue Feb 5, 2021 · 3 comments

Comments

@kainosnoema
Copy link

kainosnoema commented Feb 5, 2021

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
Copy link
Contributor

danp commented Feb 5, 2021

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
Copy link
Contributor

danp commented Feb 5, 2021

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
Copy link
Contributor

danp commented Feb 17, 2021

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.

@edmorley edmorley closed this as completed May 8, 2024
@edmorley edmorley reopened this May 8, 2024
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

No branches or pull requests

4 participants
@danp @kainosnoema @edmorley and others