search

heroku / heroku-buildpack-go

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

don't hide error messages from go list #464

Closed fabjan closed 2 years ago

fabjan commented 3 years ago

Why?

I had a repo in a bad state (I missed comitting an updated go.sum) which failed to build but figuring out why took some time because the error messages were hidden.

Result

Before this change:

-----> Determining packages to install
remote: 2021/05/05 21:32:22 exit status 1

After this change:

-----> Determining packages to install
remote: go: github.com/jackc/pgx/v4@v4.11.0: missing go.sum entry; to add it:
remote:         go mod download github.com/jackc/pgx/v4
remote: 2021/05/05 21:41:10 exit status 1

That output makes identifying my error much quicker.

When go.sum is fixed there is no extra error output from go list due to this change.

Comments

The /dev/null redirection was added with the commit "make go list use of -mod=vendor more dynamic", which does not mention anything about the error output, so if it was not strictly necessary maybe that little redirection change can be reverted. I see a few parts of the script redirecting to stdout as well but go list did just not do any redirection prior to that commit.

Also, jq is required for running make test. I can add that info to the README in this PR if you want.

fabjan commented 3 years ago

Running tests I see that non-error cases also seem to write to stderr making tests fail ... I should have let them run to completion before creating the PR.

fabjan commented 3 years ago

@joshwlewis my bad, I can resolve the conflict. Is a rebase in my fork the preferred way?

Also, I added a note about jq being required to run the tests (I think because the test target depends on test-assets which runs sbin/fetch-test-assets which calls out to jq). I can drop that change from this PR if you prefer.

fabjan commented 3 years ago

conflicts resolved