`script/server` requires specific shells

[noah-weingarden]

When I run ./script/server as-is, I see the following error:

trap: SIGINT: bad trap

I believe this is because sh on my machine doesn't support SIG-prefixed signal names. On my machine, sh is dash. Is it problematic to use bash instead? Also, would dropping the prefixes make the script more cross-platform?

[seshrs]

Good call! I agree with you, I think we should just drop the SIG prefix if it works across shells. I'd welcome a PR if you have the bandwidth to do so 😃

[noah-weingarden]

Just made one!

On a similar note, .githooks/pre-commit doesn't work with dash, as set -o pipefail is an illegal option. When I tried switching to bash, the script exited 1 when it reached the line MINOR_VERSION=$(cat $PROJECT_ROOT/VERSION | grep -o "\d\.\d").

[noah-weingarden]

It's because dirname /home/noah/primer-spec outputs /home/noah, which doesn't have a VERSION file.

[seshrs]

On a similar note, .githooks/pre-commit doesn't work with dash, as set -o pipefail is an illegal option.

I think we can safely remove the pipefail option. Do you see other errors in dash after removing that?

It's because dirname /home/noah/primer-spec outputs /home/noah, which doesn't have a VERSION file.

Hmm… the pre-commit hook is supposed to find its own directory and then find the project root from there.

https://github.com/eecs485staff/primer-spec/blob/83b3d4bb3f18fb79d71101358aef9ea985c4446d/.githooks/pre-commit#L7-L11

Could you try adding echo statements to check the value of the variables and working directories? I’m currently suspecting that dirname $0 isn’t working as I expected, but I don’t know for sure…

(I’ve only ever tested the builds and scripts on bash on my Mac 😅 )

[noah-weingarden]

Could you try adding echo statements to check the value of the variables and working directories?

Good call! Sorry, I was lazy last night and misinterpreted the error: dirname is working fine. The real issue is that my version of grep doesn't recognize \d, since it's not part of basic regular expressions. Switching to grep -o "[[:digit:]]\.[[:digit:]]" or grep -o "[0-9]\.[0-9]" might make the script more cross-platform.

[noah-weingarden]

I think we can safely remove the pipefail option. Do you see other errors in dash after removing that?

Apparently dash doesn't have a pushd command. 🤷

[seshrs]

This will be fixed when #195 is merged.