Skip to content

Ensure jq exists before executing the script#19945

Merged
wendigo merged 1 commit intotrinodb:masterfrom
danielyahn:env-check-docker-build-script
Dec 5, 2023
Merged

Ensure jq exists before executing the script#19945
wendigo merged 1 commit intotrinodb:masterfrom
danielyahn:env-check-docker-build-script

Conversation

@danielyahn
Copy link
Member

@danielyahn danielyahn commented Nov 29, 2023

Description

While running the docker image build on a new fresh laptop, I just noticed that build.sh script requires jq to be installed. Added a jq existence check in the earlier portion of the build script, so that the build fails faster and it is more obvious that it's required.

With the change the script outputs the following when jq doesn't exist.

+++ dirname ./build.sh
++ cd .
++ pwd
+ SCRIPT_DIR=<trino-base-dir>/trino/core/docker
+ cd <trino-base-dir>/trino/core/docker
+ SOURCE_DIR=<trino-base-dir>/trino/core/docker/../..
+ ARCHITECTURES=(amd64 arm64 ppc64le)
+ TRINO_VERSION=
++ cat <trino-base-dir>trino/core/docker/../../.java-version
+ JDK_VERSION=21.0
+ getopts :a:h:r:j: o
+ shift 0
+ check_environment
+ command -v jq
+ echo 'Please install jq'
Please install jq
+ exit 1

Additional context and related issues

Before the change, if jq isn't there, the following error message was buried in the log and the docker build actually started.

... <omitted> ...
+++ jq -er '.releases[]'
+++ head -n 1
./build.sh: line 60: jq: command not found
++ RELEASE_NAME=
++ echo 'Failed to determine release name: '
Failed to determine release name: 
++ exit 1
... <omitted> ...

then the docker image build failed with the following error:

ERROR: failed to solve: process "/bin/sh -c set -xeuo pipefail &&     microdnf install -y tar gzip &&     echo \"Downloading JDK from ${JDK_DOWNLOAD_LINK}\" &&     mkdir -p \"${JAVA_HOME}\" &&     curl -#LfS \"${JDK_DOWNLOAD_LINK}\" | tar -zx --strip 1 -C \"${JAVA_HOME}\"" did not complete successfully: exit code: 2

Release notes

(X) This is not user-visible or is docs only, and no release notes are required.
() Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

@cla-bot
Copy link

cla-bot bot commented Nov 29, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to cla@trino.io. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@danielyahn
Copy link
Member Author

danielyahn commented Nov 30, 2023

@electrum I think I just got added to the CLA repo. Could you rerun the check?

@danielyahn
Copy link
Member Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label Nov 30, 2023
@cla-bot
Copy link

cla-bot bot commented Nov 30, 2023

The cla-bot has been summoned, and re-checked this pull request!

@danielyahn danielyahn force-pushed the env-check-docker-build-script branch from 4b9a817 to 9ce8bb2 Compare December 4, 2023 15:52
@wendigo wendigo merged commit d3fbd21 into trinodb:master Dec 5, 2023
@github-actions github-actions bot added this to the 435 milestone Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants