Skip to content

Commit

Permalink
Fix FATAL, ERROR, MATCH and NOMATCH to use
Browse files Browse the repository at this point in the history
The variables $@ and $* differ subtly in how they are expanded.

- $* expands to a single value with all the arguments of the program or
  function joined with spaces, or by the first character of the $IFS variable,
  to be precise.

  Usually $* is used in messages where the individual arguments and their count
  is irrelevant.

- $@ is just like $*, _except_ when in double-quoted string "$@". This form
  expands to multiple values, properly quoted, so that one set of arguments can
  be passed safely and correctly into another function.

The aforementioned macros, FATAL, ERROR, MATCH and NOMATCH all used $@ where $* was
expected, and thus only really worked with a single value. For most cases this was okay
but the little used FATAL and ERROR macros can plausibly take a message like:

  FATAL "cannot allocate instance"

Due to the way arguments were passed inside, that would not work as the parsing
routines looking for FATAL and ERROR in spread would only take the first word
into account: "cannot", which usually strips the user of vital error detail.

The case of MATCH and NOMATCH is more subtle, as usually a single pattern is
provided. In general, $* should be used in those cases as $@ does not really do
anything different in the error case (the message is the same) but the code is
somewhat on the edge of correctness so I chose to adjust them as well.

Signed-off-by: Zygmunt Krynicki <[email protected]>
  • Loading branch information
zyga committed Dec 11, 2024
1 parent da133cb commit 9be753a
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 4 deletions.
8 changes: 4 additions & 4 deletions spread/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -785,10 +785,10 @@ func (s *localScript) run() (stdout, stderr []byte, err error) {
var buf bytes.Buffer
buf.WriteString("set -eu\n")
buf.WriteString("ADDRESS() { { set +xu; } 2> /dev/null; [ -z \"$1\" ] && echo '<ADDRESS>' || echo \"<ADDRESS $1>\"; }\n")
buf.WriteString("FATAL() { { set +xu; } 2> /dev/null; [ -z \"$1\" ] && echo '<FATAL>' || echo \"<FATAL $@>\"; exit 213; }\n")
buf.WriteString("ERROR() { { set +xu; } 2> /dev/null; [ -z \"$1\" ] && echo '<ERROR>' || echo \"<ERROR $@>\"; exit 213; }\n")
buf.WriteString("MATCH() { { set +xu; } 2> /dev/null; local stdin=$(cat); echo $stdin | grep -q -E \"$@\" || { echo \"error: pattern not found on stdin:\\n$stdin\">&2; return 1; }; }\n")
buf.WriteString("NOMATCH() { { set +xu; } 2> /dev/null; local stdin=$(cat); if echo $stdin | grep -q -E \"$@\"; then echo \"NOMATCH pattern='$@' found in:\n$stdin\">&2; return 1; fi }\n")
buf.WriteString("FATAL() { { set +xu; } 2> /dev/null; [ -z \"$1\" ] && echo '<FATAL>' || echo \"<FATAL $*>\"; exit 213; }\n")
buf.WriteString("ERROR() { { set +xu; } 2> /dev/null; [ -z \"$1\" ] && echo '<ERROR>' || echo \"<ERROR $*>\"; exit 213; }\n")
buf.WriteString("MATCH() { { set +xu; } 2> /dev/null; local stdin=$(cat); echo $stdin | grep -q -E \"$*\" || { echo \"error: pattern not found on stdin:\\n$stdin\">&2; return 1; }; }\n")
buf.WriteString("NOMATCH() { { set +xu; } 2> /dev/null; local stdin=$(cat); if echo $stdin | grep -q -E \"$*\"; then echo \"NOMATCH pattern='$*' found in:\n$stdin\">&2; return 1; fi }\n")
buf.WriteString("export DEBIAN_FRONTEND=noninteractive\n")
buf.WriteString("export DEBIAN_PRIORITY=critical\n")
buf.WriteString("export PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin\n")
Expand Down
6 changes: 6 additions & 0 deletions tests/adhoc-fatal/fake/task/task.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
summary: fake task
details: |
This task will never execute because the allocation
of the ad-hoc system always fail.
execute: |
echo "Should not be reached"
15 changes: 15 additions & 0 deletions tests/adhoc-fatal/spread.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
project: spread
backends:
adhoc:
allocate: |
FATAL "I cannot do that, Dave"
systems:
- ubuntu-16.04:

path: /home/adhoc
restore:
suites:
fake/:
summary: Fake test suite

# vim:ts=4:sw=4:et
4 changes: 4 additions & 0 deletions tests/adhoc-fatal/task.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
summary: Test failure to allocate ad-hoc system

execute: |
spread | MATCH 'I cannot do that, Dave'

0 comments on commit 9be753a

Please sign in to comment.