Multiple issues with bootstrap shell script #337

New Issue

claunia · 2026-01-29T20:42:11Z

claunia commented

2026-01-29 20:42:11 +00:00

Originally created by @rgh-sas on GitHub (Aug 27, 2020).

This has been an issue for some time and still exists in version 1.0.9.
There is space between the # and !/bin/sh that should be removed as it causes errors on some platforms.
The use of the "-e" option tells the script to bail on error. The problem is that the mkdir command will return an error if the m4 directory exists in the brotli source tree thus causing the script to bail early. Wrapping this mkdir with the following would resolve this issue:

if [ ! -e "./m4" ]; then
mkdir m4 2>/dev/null
fi

Next there appears to be a flaw with the "expr" command logic used in this script. Note that the "expr" command returns an exit status of 0 if EXPRESSION is neither null nor 0, 1 if EXPRESSION is null or 0, 2 if EXPRESSION is syntactically invalid, and 3 if an error occurred. The problem is that an expression returning "0" is valid and is exiting with a 1 and thus setting the exit status to 1 in which case sh/bash believes that an error has occurred and it bails before completing the rest of the script. You could probably use built-in bash arithmetic operations rather than expr or at the very least you could consider adding a "|| true" to the end of each expr line. Example:
BROTLI_ABI_CURRENT=expr $BROTLI_ABI_INT / 16777216 || true

It is frustrating when brotli is part of a larger project and bootstrap gets called and fails because expr returns "1" in a legal expression causing the entire build to fail with a unhelpful return code of "1".

Originally created by @rgh-sas on GitHub (Aug 27, 2020). This has been an issue for some time and still exists in version 1.0.9. There is space between the # and !/bin/sh that should be removed as it causes errors on some platforms. The use of the "-e" option tells the script to bail on error. The problem is that the mkdir command will return an error if the m4 directory exists in the brotli source tree thus causing the script to bail early. Wrapping this mkdir with the following would resolve this issue: if [ ! -e "./m4" ]; then mkdir m4 2>/dev/null fi Next there appears to be a flaw with the "expr" command logic used in this script. Note that the "expr" command returns an exit status of 0 if EXPRESSION is neither null nor 0, 1 if EXPRESSION is null or 0, 2 if EXPRESSION is syntactically invalid, and 3 if an error occurred. The problem is that an expression returning "0" is valid and is exiting with a 1 and thus setting the exit status to 1 in which case sh/bash believes that an error has occurred and it bails before completing the rest of the script. You could probably use built-in bash arithmetic operations rather than expr or at the very least you could consider adding a "|| true" to the end of each expr line. Example: BROTLI_ABI_CURRENT=`expr $BROTLI_ABI_INT / 16777216` || true It is frustrating when brotli is part of a larger project and bootstrap gets called and fails because expr returns "1" in a legal expression causing the entire build to fail with a unhelpful return code of "1".

claunia closed this issue

2026-01-29 20:42:12 +00:00

claunia commented

2026-01-29 20:42:12 +00:00

@eustas commented on GitHub (Aug 28, 2020):

Hello. Thanks for the report. Going to fix the mentioned issues very soon.

@eustas commented on GitHub (Aug 28, 2020): Hello. Thanks for the report. Going to fix the mentioned issues very soon.

claunia commented

2026-01-29 20:42:12 +00:00

@eustas commented on GitHub (Aug 28, 2020):

Is it better now?

@eustas commented on GitHub (Aug 28, 2020): Is it better now?

claunia commented

2026-01-29 20:42:13 +00:00

@rgh-sas commented on GitHub (Aug 28, 2020):

Thank you @eustas. That is much better. I grabbed your changes and tried them out and all work well. Appreciate the fix!

@rgh-sas commented on GitHub (Aug 28, 2020): Thank you @eustas. That is much better. I grabbed your changes and tried them out and all work well. Appreciate the fix!

claunia commented

2026-01-29 20:42:13 +00:00

@rgh-sas commented on GitHub (Aug 28, 2020):

I'm closing this issue. Thanks again for the fix.

@rgh-sas commented on GitHub (Aug 28, 2020): I'm closing this issue. Thanks again for the fix.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/brotli#337