Replace Bash scripts with POSIX sh scripts - #102
Conversation
These changes are unverified/untested. This is the next TODO item.
|
Does this need to be reorganized into a series of smaller pull requests, perhaps one file at a time, to ensure that they can be individually checked for correctness and merged before continued development leads to conflicting changes? Perhaps too many changes in a single commit will result in "race conditions" with commits for feature additions. |
|
Actually it looks pretty trivial to me, and great to see folks actually caring about POSIX compliant shell scripting (FYI: I'm an outsider not a mod) |
| for filepath in $(find "$DIR" -maxdepth 1 -name '*.kaos'); do | ||
| filename=$(basename "$filepath") | ||
| testname="${filename%.*}" | ||
| out=$(<"$DIR/$testname.out") |
There was a problem hiding this comment.
Shouldn't this be POSIX-compliant? I don't think cat is needed, but I didn't look it up in the spec...
Edit: Needless to say using cat here is always better because of better error handling. So this is just my thinking out loud.
I've checked it and the POSIX manual isn't explicit about this being disallowed or "non-POSIX" but e.g. dash doesn't seem to do the right thing, so I suppose it's definitely not portable and probably generally not considered POSIX compliant.
So thanks for spotting this (I myself wouldn't use $(<... because as I wrote above - error handling is better with cat).
| #!/bin/sh | ||
|
|
||
| DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" | ||
| OSTYPE=$(uname --kernel-name) |
There was a problem hiding this comment.
| testname="${filename%.*}" | ||
|
|
||
| if [[ "$testname" == "exit_"* ]]; then | ||
| if [ "$testname" = "exit_*" ]; then |
There was a problem hiding this comment.
This is weird - can a file exit_* actually exist? Or is it a forgotten "stop iteration over an empty list" relic before for filepath in $(find "$DIR" -maxdepth 1 -name '*.kaos'); do was introduced (instead of presumably for filepath in exit_*; do)?
| valgrind --tool=memcheck --leak-check=full --show-reachable=yes --num-callers=20 --track-fds=yes --track-origins=yes --error-exitcode=1 chaos -c $DIR/$filename -o $testname || exit 1 | ||
| elif [[ "$OSTYPE" == "darwin"* ]]; then | ||
| /tmp/DrMemory/bin64/drmemory -no_follow_children -exit_code_if_errors 1 -- chaos -c $DIR/$filename -o $testname || exit 1 | ||
| if [ "$OSTYPE" = "Linux*" ]; then |
There was a problem hiding this comment.
Same as above - this doesn't seem to do what was intended...
4 participants
Pull Request Details
Description
This PR replaces the Bash scripts with POSIX sh scripts.
Related Issue
This is to address PR #100.
Motivation and Context
Bash inherently has portability problems because it breaks POSIX compliance. Upgrading our shell scripts to be POSIX compliant will help with portability and drawing in new users.
How Has This Been Tested
TODO
Types of changes
Checklist