Remove jq #854
Remove jq #854
Conversation
|
Since node, I'll defer to Danielle for review - so removing the languages-review label for now :-) |
| JQ="$BP_DIR/lib/vendor/jq-$(get_os)" | ||
| JQ="/usr/bin/jq" | ||
| if ! test -f "$JQ"; then | ||
| curl -Ls https://github.com/stedolan/jq/releases/download/jq-1.5/jq-linux64 > "/usr/bin/jq" \ |
There was a problem hiding this comment.
jq is always available on all Heroku stack images as of heroku/base-images/pull/174, meaning this codepath is unused for Heroku builds.
The reason this is needed to make the unit tests pass for this repo, is that the unit tests (unlike the Hatchet tests) use the Docker images, which in the case of Cedar-14 haven't been updated for some time (due to restrictions about distributing ESM updates), so the heroku/cedar:14 docker image doesn't have heroku/base-images/pull/174 so doesn't have jq.
Also /usr/bin can only be written to by root, so this line would fail should it ever be needed for builds on the Heroku platform. It seems that Dokku doesn't run builds as root either, and also doesn't have jq, which is why there were reports of failures in #789.
Other options for this implementation would be:
- To assume jq always exists (since it does on Heroku), so remove this download and instead modify the tests to
apt-get update && apt-get install jqon Cedar-14 only, here - Or, to update the step to install into the buildpack directory (or build directory) instead of
/usr/bin, and ensure that that chosen directory is on PATH
For reduced complexity I would lean towards (1). Dokku users will still need to update their base images to include jq (for parity with the Heroku stack images), but at least the error message they get would be clearer ("jq not found" etc).
There was a problem hiding this comment.
Thanks for this insight!
There was a problem hiding this comment.
Dokku users will still need to update their base images to include
jq(for parity with the Heroku stack images)
Hmm looks like they do base their images on the Heroku stack images:
https://github.com/gliderlabs/herokuish/blob/master/Dockerfile
...so perhaps it just needs a new herokuish release, or for Dokku users to update their install to a newer version?
Either way, this sounds like something Dokku users should file against dokku (needing jq/newer image) :-)
jqis now added to the build stack so we are able to remove the dependency when cedar-14 gets deprecated. The$JQenvironment variable is reassigned to the binary path, in case someone has scripts that reference the variable. Until Cedar-14 gets deprecated, we will have to check forjqexistence on the stack image and download if missing.