0

I do little shell scripting so hopefully I am doing something obviously wrong!

This hook is intended to run whenever you push. If you are on the designated branch, in this case 'githook', it should run npm run testbuild and if that fails, stop the push.

If you are on another branch it should not interfere, and if you are on that branch and that test completes without error, it should let the push go ahead.

Here is the content of the script pre-push:

#!/bin/zsh

current_branch=$(git symbolic-ref HEAD | sed -e 's,.*/\(.*\),\1,')

CMD="npm run testbuild"

if [[ $current_branch = "githook"]]; then
    eCHO "You are on $branch, running build test"
    eval $CMD
    RESULT=$?
    if [ $RESULT -ne 0 ]; then
        echo "failed $CMD"
        exit 1
    fi
fi
exit 0

Currently, when I push from that branch, I get this:

.git/hooks/pre-push:7: parse error near `;'~

But I don't see anything obviously wrong there?

isherwood
  • 58,414
  • 16
  • 114
  • 157
Ben Frain
  • 2,510
  • 1
  • 29
  • 44
  • 3
    `eCHO` ?? Please fix cut-n-paste errors so we know that you have the correct code. – William Pursell Sep 29 '22 at 16:13
  • 2
    In zsh, I get a similar error with `[[ 1 = 1]]` (no space between 1 and ]]). It's probably best to just stick with `[` (or test). – William Pursell Sep 29 '22 at 16:15
  • @isherwood it's these because the bit of code I originally copied had it there :D Are you telling me it is unneeded? – Ben Frain Sep 29 '22 at 16:15
  • 1
    Weird, the same error in `bash`. I thought the whole point of `[[/]]` was to avoid annoying errors like this. This is why I continue to use `[`. It has warts, but it's a better tool. – William Pursell Sep 29 '22 at 16:17
  • @WilliamPursell yes, it was that simple! didn't realise whitespace was important there, if you want to add that as an answer I can accept. thanks – Ben Frain Sep 29 '22 at 16:17
  • 1
    Few minor nits. You don't need `sed`; use `git symbolic-ref --short HEAD` instead. Add a space: `"githook" ]]`. You don't need `if [ $RESULT -ne 0 ]; then` at all; do it this way: `if ! eval $CMD; then`. `eCHO` — fix case; it's `echo`. `"You are on $branch, running build test"` — ­what is `$branch`? should be `$current_branch`? – phd Sep 29 '22 at 16:18
  • 1
    `eval $CMD` should *always* be `eval "$CMD"`, if you truly have a compelling need to use `eval` at all (which certainly isn't the case here; you could use `cmd() { npm run testbuild; }` and then run `cmd`). – Charles Duffy Sep 29 '22 at 16:40

1 Answers1

2

You need to have whitespace before ]] so that it is recognized as a separate token. eg:

if [[ "$current_branch" = githook ]]; then

Note that there is no need to quote the bare string githook, but you should always quote variables unless you have a reason not to. (Within [[, it's not strictly necessary, but it's good practice.) Also, you can simplify some of the RESULT code and just do:

if ! eval "$CMD"; then
    echo "failed $CMD" >&2
    exit 1
fi

Note also that "failed $CMD" is almost certainly redundant, since you should get a decent error message from the command itself. It can be handy to see the full command that was executed, but generally shell scripts should not duplicate error messages that should be written by the tools they call. If you do write error messages, make sure they are written to stderr (hence the >&2 redirect).

William Pursell
  • 204,365
  • 48
  • 270
  • 300
  • 3
    While you're at it, there's no need for `eval` with this particular command. (And if there *were* a need, it would probably be better to restructure the code to avoid the need.) – chepner Sep 29 '22 at 16:21