Shell Script while loop: [ around a pipeline missing `]'
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 likeif
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, likewhoami
orgrep
, but with a funny name (seels -l /bin/[
). It's a shorthand fortest
.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 usetest
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
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?