Shell Script while loop: [ around a pipeline missing `]'

Question

The shell script I'm trying to use keeps giving this error:

$ ./script.sh: line 2: [: missing `]' 
grep: ]: No such file or directory

The line is part of a section trying to check if a particular process is going to have a file locked:

COUNTER=0
while [ ps aux | grep "[r]elayevent.sh" ] && [ "$COUNTER" -lt 10 ]; do
    sleep 3
    let COUNTER+=1
done

Obviously I've checked that the brackets all pair up correctly - which looks fine to me. Also the common white space around the condition issue doesn't apply.

What am I missing here?

Answer 1

The error is that you should remove first [ because of you want to check the exit status then use command directly.

The Wiki pages of the Shellcheck tool have an explanation for this (issue SC1014):

[ .. ] is not part of shell syntax like if statements. It is not equivalent to parentheses in C-like languages, if (foo) { bar; }, and should not be wrapped around commands to test.

[ is just regular command, like whoami or grep, but with a funny name (see ls -l /bin/[). It's a shorthand for test.

If you want to check the exit status of a certain command, use that command directly.

If you want to check the output of a command, use "$(..)" to get its output, and then use test or [/[[ to do a string comparison:

Also use ps aux | grep -q "[r]elayevent.sh" so that you will get the exit status silently instead of printing anything to stdout.

Or you can use pgrep and direct it's output to /dev/null.

Use second condition first because it will be more efficient for the last case.

So final script will be like:

#!/bin/bash
COUNTER=0

while [ "$COUNTER" -lt 10 ] && ps aux | grep -q "[r]elayevent.sh"   ; do

    sleep 3

    let COUNTER+=1

done

Or

#!/bin/bash
COUNTER=0

while [ "$COUNTER" -lt 10 ] && pgrep  "[r]elayevent.sh" >/dev/null  ; do

    sleep 3

    let COUNTER+=1

done

Answer 2

You can't have a pipe inside [ ... ]. It's also better to use pgrep than to try to parse the output of ps:

count=0
while [ "$count" -lt 10 ] && pgrep relayevent.sh >/dev/null; then
    sleep 3
    count=$(( count + 1 ))
done

BSD systems could use pgrep -q ... instead of pgrep ... >/dev/null to discard the actual output of pgrep, just as with ordinary grep (we're only interested in the exit status).

Note how we don't put the pgrep command within [ ... ]. That's because we're not interested in its output, only its exit status. With [ ... ] you commonly compare strings or numbers. The [ ... ] will result in an exit status that is zero (true) or non-zero (false), just like the pgrep execution.

However, this does not check for any locking mechanisms, only whether a particular process is running or not.

If you are trying to get only a single instance of a script running, then it's better to do something like this (assuming the EXIT trap is executed whenever the script is terminating orderly):

lockdir=dir.lock

if mkdir "$lockdir"; then
    trap 'rmdir "$lockdir"' EXIT
else
    echo 'Only one instance of this script allowed' >&2
    exit 1
fi

With a number of tries and sleep:

lockdir=dir.lock

count=0
while [ "$count" -lt 10 ]; then
    if mkdir "$lockdir"; then
        trap 'rmdir "$lockdir"' EXIT
        break
    else
        echo 'Locked. Sleeping...' >&2
        sleep 3
    fi

    count=$(( count + 1 ))
done

if [ "$count" -eq 10 ]; then
    echo 'Giving up.' >&2
    exit 1
fi

Related:

• How to make sure only one instance of a bash script runs?