Hacker News Reader

Comment by muxator | Hacker News Reader

I wouldn't trust that script too much: there is no error checking at all.

What happens when s3cmd fails and after two months you discover the "Vault sync succeded" emails you have been getting were all illusory?

If the author is reading: please take the time to update your example, including proper error checking (at least "set -eu"), otherwise the people on the internet that are going to copy your script are in for a hard time.

Skeleton of the "incriminated" script:

  #!/bin/bash
  [...]
  s3cmd sync ...
  [...]
  notify-send [...] "Vault sync succeded [...]"

View on HN

I haven't looked at the code but the author would benefit from using shellcheck.

https://www.shellcheck.net

I agree with you. I should add a disclaimer that this is still being worked on.

I will add what you proposed.

Thanks for your input.

Always start all your bash scripts with

set -e

You'll never regret it and might be very very very happy it was there. And if you start with that, then as you work on the shell script, you'll be more likely to make the script be idempotent (checking for files before copying, checking for lines in files before appending/sed'ing etc.) Idempotent >> non-idempotent for "alter the state on an end point" things.

Will do. Thanks for this.

Totally agreed. You can also temporarily turn it off and turn it back on again if you really need to by doing "set +e ... set -e". Sometimes it is more convenient to check errors explicitly, i.e. with test code where the test is expected to fail, but errors should fail fast by default.

I don’t know about never regretting. “set -e” is the right choice the vast majority of the time, but definitely has some gotchas to it. For instance, what does the following script print?

  #!/bin/bash

  set -e

  (
   echo "Start of Subshell"
   (exit 1)
   echo "This shouldn’t run, right?"
  ) || echo "Subshell failed"

Also, "unofficial bash strict mode" is terrific, especially if you intend to distribute a script:

http://redsymbol.net/articles/unofficial-bash-strict-mode/

    set -euo pipefail
    IFS=$'\n\t'

(You can always turn, for example, error checking back off for an individual portion of the script with `set +e`)

Adding "-E" should be considered as well, so traps are also applied in functions.

The options are good, but I don't like the IFS suggestion. Bash has a perfectly good way of dealing with spaces in array elements. Fixing the example in the article is as simple as adding double quotes around the array:

    for name in "${names[@]}"; do
      echo "$name"
    done

The will not result in a single element, but will destructure each element of names, as if each were double-quoted, and will print the same output that the "strict mode IFS setting" does.

What's more, the double-quoted array solution will work even if the elements contain newlines or tabs.

Whyyyyyy? Just use rclone.

Reply on news.ycombinator.com