GoogleCloudPlatform/emblem

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!!