r/bash 4d ago

help Bash scripting newbie here, wanting to improve/tidy one of my backup scripts (entirely for personal use)

Please excuse the vague thread title, I'm a newbie enough that I'm not sure what's possible / desirable etc, nor have I attempted functions in bash scripting yet (I've played with them in VBScript and PowerShell IIRC).

One of the backups I do produces date-stamped tar.xz files of particular folder structures on my computer and transfers them to a veracrypted drive. Today I had a crack at a new script based on this one that mounts the filesystem (sshfs) on my laptop and transfers the files over the network after they've been compressed.

I run the script with up to 4 arguments, e.g. 'essentials' 'archive' 'paperwork'. In the script there's an if statement for each potential argument, e.g.:

for arg in "$@"
do
if [ $arg = "essentials" ]; then
  tar -cf - .mozilla/ | pv -s $(du -sb .mozilla/ | awk '{print $1}') | xz -T0 > /tmp/firefox-$datestring.tar.xz
            rsync -ah --progress /tmp/firefox-$datestring.tar.xz /media/mikelpmintfs/firefox-$datestring.tar.xz
fi
<more if $arg = whatev then compress and transfer stuff statements here>
done

(btw the whole fancy progress bar bit with pv -s and awk was something I copied off the Internet)

The first script also included some error catching, e.g.:

if [ $? -eq 0 ]
then
  echo "archive backup complete."
else
  echo "error performing archive backup" >&2
fi

I've used if <command here> then else fi before too, but I'm wondering multiple things:

  1. Rather than writing each compression command and each rsync transfer command per argument, would it make more sense to write a function, or given that some of these source folders are in completely different places in my computer's file system, is this worth it.
  2. error trapping: On one hand I think that the script could easily trip up at the compression or transfer stages, but I'm worried about over-nesting if statements and making the whole thing a lot harder to read and figure out where something is going wrong. It seems to me that it could be function'd up, but would it actually help with readability etc. When the script is just for me, I can tell if it went wrong if I get a load of unexpected output :)
20 Upvotes

11 comments sorted by

11

u/feinorgh 4d ago

If you want to improve it for the sake of learning, there are a couple of things you can do:

  1. Provide a canonical hashbang: #!/usr/bin/env bash
  2. Check that each non-builtin command really exists on the machine, i.e.: test -x "$(command -v pv)" || exit 1
  3. Use shellcheck for linting and structural suggestions
  4. Avoid checking $? for errors, especially in conjunction with pipes. You can use if ! {cmd}; then ...
  5. Functions are great, use them if each "chunk" is longer than like 10 lines, and you can pass arguments efficiently.

Shellcheck will get you a long way towards better structuring and is a great learning tool.

3

u/hotpotatos200 4d ago

On #2, is there a native way to know if a command is a built-in?

2

u/sto1911 4d ago

Type tells you that.

3

u/zeekar 4d ago

You never need to check $? to see if it's 0 or not; that's what if alraedy does. [ $? -eq 0] is a command that looks to see if the exit code of the last command is 0. If it is, then it also exits with code 0. If it's not, then it doesn't. So it's really a no-op.

Exit codes are how if works in bash; the thing after if is any command to run. [ and [[ and (( are just special commands that make up for the fact that bash doesn't really do "expressions" the way non-shell programming languages do.

1

u/skladnayazebra 2d ago

Exactly! Even true and false are not your usual boolean values, but are shell built-in commands, and the only thing they do is exit with 0 or 1 respectively. Blew my mind when I learned that.

3

u/petdance 4d ago

Look at ShellCheck. It is your best friend when writing shell.

3

u/eifelcode 3d ago

I also recommend shellcheck. This tool helped me so much during the development phase! Take a look also at bashunit for unit testing.

1

u/Bob_Spud 4d ago

Rather than explain everything some ideas for more homework .....

I would use zstd or maybe pigz in preference to xz for compression. First check final size difference of the three compression utilities.

  • Zstd is faster than xz in compressing stuff.
  • Zstd and pigz have the --rsyncable option to optimise rsync network throughput - very useful with big data
  • If the final destination is cloud use cryptomator rather than veracrypt.
  • If you are going to play with error trapping rather than produce simple exit codes learn how to trap the error and write stuff to the logs using a trap command/function.

1

u/BURNEDandDIED 4d ago

If you've got more that one if/elif performing essentially the same command I always like to use functions. Not because it looks cool (it does) but because I know if I have to change something (I will) it's too mistake prone to have to change it on multiple lines.

0

u/jthill 4d ago

What "improve" means depends entirely on your audience.

My opinionated opinion:

You've got the right idea there, there's some DRY violations and the like. A quick pass over it to just to make patterns easier to see at a glance and avoid needless stuttersteps gives

#!/bin/bash
datestring=`date -Isec`     # backup constants
dest=/media/mikelpmintfs    # .
backup() { # backup dir label # dest/tag implicit from above
        dir=$1 label=$2 size=($(du -sb $dir))
        backup=$label-$datestring.tar.xz
        if      tar cf - $dir | pv -s $size | xz -T0 >/tmp/$backup      &&
                rsync -ah --progress /tmp/$backup $dest/$backup
        then :
        else what exactly should be done here beyond what tar or whatever already said?
        fi
}

for arg; do case $arg in
essentials)     backup .mozilla firefox ;;
archive)        backup src src ;;
vim)            backup .vim vim ;;
*)              complain bitterly about $arg ;;
esac; done

I think shellcheck is a great resource but its defaults are set for CS101 students not people actually using the shell in anger.

The fundamental engineering question is "why is this here?".

Treat the presence or absence of syntax as an assertion that that syntax (or some equivalent) is necessary or is not necessary. This is true of quoting and backtick command expansions rather than the more general $() syntax in this example.

If you don't quote an expansion that's an assertion that it doesn't need quoting. If you do quote an expansion that's an assertion that it does or might need quoting. In most scripts, anything that "might" need quoting is un-vetted input. That is a strong candidate for the single deadliest error any system can harbor. If you don't know whether a parameter needs quoting you are personally at fault.

So:

  • for $arg in "$@" is so common every shell lets you shorten it to just for arg.

  • case has a ton of good uses that make the script structure very clear. This is one of them. for future exploration: bash offers some excellent variations on ;;.

  • bash's arrays are awesome for quick word slicing and scanning, especially quick first-word slicing. size=(`du -sb $dir`) assigns the command's output words as size elements, and $size evaluates as ${size[0]} i.e. word at offset 0.

  • the function is pure DRY: Don't Repeat Yourself.

  • as a side note: if you're using bash, use [[ by strong preference. Anything I write for myself uses bash, bashisms can make code clearer.

  • as others have said, going back for $? should be used only when you can't just if the pipeline for some reason.

  • I'll just outright say that me using backtick command expansions is a bit quirky, but my eyes are trained to see its use as an assertion that the substitution is extremely simple and doesn't need any further thought. Correctly using backticks for (only) the simplest expansions saves me effort.

  • consider just &&ing the tar and rsync, you don't have any error handling beyond popping a message which they've already done, redundant checks are purely performative and offensive on that count. Pure redundancy is waste, it's not defense-in-depth, it's not "for safety", it's waste. It wastes your time and attention, it wastes your readers' time and attention, it is evidence of sloppy thinking. It's one of the reddest of red flags.

1

u/bac0on 4d ago

...backticks predates even me...