Skip to content

GEOMESA-3561 install-dependencies.sh script should exit with non-zero status if a download fails#3516

Open
cptlobster wants to merge 1 commit intolocationtech:mainfrom
cptlobster:geomesa3561-install-deps-fix
Open

GEOMESA-3561 install-dependencies.sh script should exit with non-zero status if a download fails#3516
cptlobster wants to merge 1 commit intolocationtech:mainfrom
cptlobster:geomesa3561-install-deps-fix

Conversation

@cptlobster
Copy link
Contributor

In the functions.sh helper:

downloads+=("(echo >&2 fetching $fname && curl -LsSfo '$tmpfile' '$url' && mv '$tmpfile' '${dest}/${fname}' && chmod 644 '${dest}/${fname}') || echo [ERROR] Failed to fetch $fname")

If the subshell running curl and the associated commands fails, it runs the following echo command. This changes the exit code outputted by the subshell to 0, and as such all commands sent to xargs will return 0. Therefore, any failures in this process will be silently ignored, and the install-dependencies.sh script (and others) will exit as if they succeeded.

This PR saves the exit code from the subshell to a variable and then exits with that code after sending the echo. xargs will then properly exit with code 123 if any of the downloads fail.

@cptlobster cptlobster force-pushed the geomesa3561-install-deps-fix branch from 5c009ac to e15c00e Compare February 17, 2026 17:37
@elahrvivaz
Copy link
Contributor

the exit code seems fixed, but now it always prints out [ERROR] Failed to fetch xxx.jar for every single dependency, even when the jar was actually downloaded correctly

tmpfile=$(mktemp)
# -sS disables progress meter but keeps error messages, -f don't save failed files, -o write to destination file, -L follow redirects
downloads+=("(echo >&2 fetching $fname && curl -LsSfo '$tmpfile' '$url' && mv '$tmpfile' '${dest}/${fname}' && chmod 644 '${dest}/${fname}') || echo [ERROR] Failed to fetch $fname")
downloads+=("(echo >&2 fetching $fname && curl -LsSfo '$tmpfile' '$url' && mv '$tmpfile' '${dest}/${fname}' && chmod 644 '${dest}/${fname}') || export error=\$?; echo [ERROR] Failed to fetch $fname; exit \$error")
Copy link
Contributor

@elahrvivaz elahrvivaz Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
downloads+=("(echo >&2 fetching $fname && curl -LsSfo '$tmpfile' '$url' && mv '$tmpfile' '${dest}/${fname}' && chmod 644 '${dest}/${fname}') || export error=\$?; echo [ERROR] Failed to fetch $fname; exit \$error")
downloads+=("(echo >&2 fetching $fname && curl -LsSfo '$tmpfile' '$url' && mv '$tmpfile' '${dest}/${fname}' && chmod 644 '${dest}/${fname}') || (export error=\$?; echo [ERROR] Failed to fetch $fname; exit \$error)")

() seems to fix it

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

Successfully merging this pull request may close these issues.

2 participants