Replace /bin/bash with /bin/sh

[TheLonelyGhost]

This also addresses idiosyncrasies with echo versus printf (see StackExchange answer about this).

Since bash is a more fully-featured shell and sh (in Ubuntu, this is aliased to /bin/dash) is lighter weight, it makes more sense to use sh to run the realpath script in ./bin/crystal. I've benchmarked it on my system and there's a very slight increase in performance. It also, in part, reduces dependencies to run crystal.

Changes were made to the escape sequence coloring to support more shells, also per StackExchange.

[drhuffman12]

Some bash vs sh vs [etc] comments are noted at: http://stackoverflow.com/questions/5725296/difference-between-sh-and-bash

[ysbaddaden]

I'm all for using sh vs bash, which would drop a dependency on some platforms, e.g. Alpine, FreeBSD, OpenBSD.

[TheLonelyGhost]

One thing to note is that it is still required to run ./bin/ci. I found it difficult to test since it checked for Travis-CI specific values and I didn't want to mutate the script too much for my purposes.

Since there were assumptions made that it was on MacOS and running an Ubuntu docker container, should we be concerned by the following lines?

  on_linux docker run \
    --rm \
    -u $(id -u) \
    -v $(pwd):/mnt \
    -w /mnt \
    -e LIBRARY_PATH="/opt/crystal/embedded/lib/" \
    -e CRYSTAL_CACHE_DIR="/tmp/crystal" \
    "jhass/crystal-build-$ARCH" \
    "$ARCH_CMD" /bin/bash -c "'$command'"

  on_osx PATH="/usr/local/opt/llvm/bin:\$PATH" \
    CRYSTAL_CACHE_DIR="/tmp/crystal" \
    /bin/bash -c "'$command'"

[ysbaddaden]

bin/ci is, as the name implies, tied to the CI solution we use. You may directly call make spec, make std_spec or bin/crystal spec [path/to/spec] instead when developing locally.

[TheLonelyGhost]

Let me clarify. I had concerns with us calling bash on the lines mentioned earlier if we're moving forward assuming bash isn't available on the target platform (linux, darwin, etc.). Should these be changed, or are we making the assumption that bash will always installed in the CI environment?

[ysbaddaden]

We'll rework bin/ci when we leave Travis for another platform, where maybe we'll like to avoid bash... or not. It's one thing to require users to have bash installed to hack on Crystal (if we can avoid it, we should) it's another thing to have bash installed in the VMs running our automated test suite.

[TheLonelyGhost]

I thought similarly, but also considered that the build environment containing bash might open us up for testing removal of the dependency entirely. If we say it's not required and some deeply nested script calls it, we won't know in the CI tests but users who don't have it installed will see things explode.

Personally, I view this as an acceptable risk. I just wanted to confirm that others viewed it similarly.

[miketheman]

#codetriage

We appear to have left this conversation in agreement that sh is acceptable a replacement for bash in all modified scripts, leaving the CI-related bashisms in place for now.

Recommend for Merge, per 👍 in comments.

[RX14]

Is there anything blocking this merge? I've reviewed this PR and it LGTM.

[mverzilli]

Nothing blocking it! Thanks everyone, especially @TheLonelyGhost for crafting the PR, @ysbaddaden for providing feedback, @miketheman for taking the time to triage it and @RX14 for reviewing and nudging it ❤️

Sorry @miketheman for the horrible omission before :(.