Shell script setup.sh not POSIX compliant
unforced opened this issue · 3 comments
While it's unclear if we necessarily need to be POSIX compliant, it seems it would be a good idea to be so, especially as the setup script currently fails on Chromebooks due to this.
The major issue I've discovered thus far is [[
vs [
, although a fix is a bit more complicated than just replacing [
with [[
, as the [[
adds some secret sauce, allowing easier use of certain comparators.
Initial error:
./scripts/configure_delivery.sh: 32: [[: not found
./scripts/configure_delivery.sh: 35: [[: not found
./scripts/configure_delivery.sh: 38: [[: not found
Connect your repos: https://console.cloud.google.com/cloud-build/triggers/connect?project=wildsun-emblem-ops
Once your forked emblem repo is connected, please continue by typing any key../scripts/configure_delivery.sh: 45: read: arg count
Exited [2] at line 275 ./setup.sh:
The errors at line 32, 35, and 38 denote the lack of the support for [[
, more information on this here:
https://www.baeldung.com/linux/bash-single-vs-double-brackets
The error at line 45, read: arg count
demonstrates another posix compliance issue, in which a variable has to be specified to receive the input, even if it's not going to be used. More on this here:
https://unix.stackexchange.com/questions/581409/error-sh-1-read-arg-count
One of the weirdest lines I came across where there was an issue was this one:
https://github.com/GoogleCloudPlatform/emblem/blob/main/scripts/configure_delivery.sh#L47
if [[ -z "${REPO_NAME}" || -z "${REPO_NAME}" ]]
Anyone have any insight into why this is a thing?
Intended replacement is if [-z "${REPO_NAME}"]
This looks like it would be helpful in this context:
https://github.com/koalaman/shellcheck
Since we recommend using Cloud shell for deploying emblem, I think we can hold off on making our shell scripts posix compliant. I created a new issue however citing your recommendation of shellcheck #728
Thanks!!