Bash Script Conditionals for Commands and Testing

Question

I know this is basic, being a Bash beginner, I like to learn the level of checking and testing when calling commands like; cp, mkdir, wget,... in a Bash function, and returning a result for the function execution status.

For example, consider the following function.

# copy a file to a directory
# ${1} : full file path to be copied
# ${2} : directory file is copied to too
# return: 0, for successful copy function execution. 1, not copied. 2, function exception
function file_copy() {
 local source="${1}"
 local target="${2}"
 local file_name
 file_name=$(basename "${source}")
 if cp --force "${source}" "${target}" &>/dev/null ;then
 if [[ -f "${target}${file_name}" ]]; then
 return 0
 else
 return 1 # copy not successful
 fi
 else
 return 2 # function exception
 fi
}

My questions for this case, is it possible for the cp function not to error, but the copy didn't happen? Can I leave out the inner conditional, since the cp function success implies the copy did indeed happen? Is it better not to have cp as a conditional, and rather test for the target's existence?

Similarly, with wget

# download a package, tarball, appimage, ...
# ${1} : directory to download too, set to ~/Downloads
# ${2} : package, tarball, appimage, ... filename without directory
# ${3} : source url, repos
# return: 0, successful download. 1, otherwise
function package_download() {
 local download_dir="${1:-${USER_DOWNLOAD}}"
 local file_path="${download_dir}${2}"
 local repos="${3}"
 # --output-document = -O, --directory-prefix = -P
 if wget --directory-prefix "${download_dir}" --output-document "${file_path}" "${repos}"; then
 if [[ -f "${file_path}" ]]; then
 return 0
 else
 return 1 # file didn't download
 fi
 else
 return 1 # function exception
 fi
}

Can wget not error, yet the package didn't download? Am I over thinking things?

I know this is basic. Also, if the code looks a bit stringent, I'm using that ShellCheck extension for visual studio code.

• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! Are you the author or maintainer of these scripts?
• user246514: @SAM, yes I am. I have few more functions as such. I am making some common routines that download, expand to, update, and remove packages that go into the /opt/ directory, like firefox, thunderbird, vscode, etc. Also, copy those package's icons and application launchers to their relevent directory, like /home/$(logname)/.local/share/applications/
• Martin Baulig: If you want this to be portable, then you might want to use "cp -f" instead of "--force" (which is Linux-specific). I don't see how the "cp" could return success if the copy failed. However, there is a small possibilty of the inner check failing even after the outer copy succeeded - due to the possible race condition of the file being deleted by some other process between the copy and the check.

Answer 1

Your function file_copy is basically a simple wrapper around the built-in cp function.

So I searched the known error codes for cp, to find out how your code does align with that command. It appears that error codes are not really standardized or even consistent for this command: https://stackoverflow.com/questions/23579492/all-possible-exit-codes-for-cp

I haven't searched the matter extensively though. That being said, many Unix-like utilities return just 0 or 1. In your code you define a hypothetical return code of 2:

return 2 # function exception

But at this point we don't know what could trigger this condition. Either the file copy succeeded, or it didn't. Unless you decide to implement some logic to distinguish between causes of failure, I would just drop this part.

On the other hand, man wget indicates that possible error codes range from 0 to 8, at least on my machine and Linux implementation (Arch Linux):

EXIT STATUS
 Wget may return one of several error codes if it encounters problems.
 0 No problems occurred.
 1 Generic error code.
 2 Parse error---for instance, when parsing command-line options, the .wgetrc or .netrc...
 3 File I/O error.
 4 Network failure.
 5 SSL verification failure.
 6 Username/password authentication failure.
 7 Protocol errors.
 8 Server issued an error response.
 With the exceptions of 0 and 1, the lower-numbered exit codes take precedence over higher-numbered ones, when multiple types of errors are encountered.
 In versions of Wget prior to 1.12, Wget's exit status tended to be unhelpful and inconsistent. Recursive downloads would virtually always return 0 (success), regardless of any
 issues encountered, and non-recursive fetches only returned the status corresponding to the most recently-attempted download.

If you are writing a wrapper around that function, then I think it would be wise to return the actual error code that you got from the invocation of wget. In package_download you are actually throwing away valuable troubleshooting information here. A wrapper should bring some added value, so think about it. What problem are you trying to solve or improve. The wrapper could do some stuff to make code more resilient, like retry on error for example. There are wget parameters for that, and in fact you could even create a function alias instead of a full-fledged function.

If you need to figure out why it didn't work, you'd want to see the output of stderr as well. Instead of just discarding the information you could send it to a log file perhaps, when the exit code != 0.

Answer 2

There's an incorrect comment here:

 if [[ -f "${target}${file_name}" ]]; then
 return 0
 else
 return 1 # copy not successful
 fi

If cp was successful, then we're not even in this branch (because cp exited with non-zero status). Instead, this fails only when the filename was removed between after it was created and before the test.

Answer 3

As @TobySpeight helpfully points out, it can be hard to predict what the code you authored actually does. So testing is important -- exercise each branch and verify the Right Thing happened.

good shell hygiene

A proliferation of branches yields a proliferation of things to test, or you can cross your fingers if you choose to leave some cases untested. I try hard to have exactly one path through a script. Here is the simplest way to accomplish that for several commands:

#! /bin/bash
set -e -u
cmd1
cmd2
cmd3

The -e says: "If any command fails, bail out at once." That is, forking off a cmd2 child might return a non-zero failure status, and then -e will ensure we don't even attempt to run cmd3 or subsequent commands. Related to -e is -o pipeline -- consult the bash man page for details.

The -u has no effect in this brief example, but it often is useful to throw it in. It triggers an error for Uninitialized variables, which typically are due to typographic errors. For example, we might have started with singular FILE=readme.txt, and then refactored to this:

set -u
FILES=*.txt
ls -l ${FILE}

Oh, too bad! Failed refactor, typo on the ls reference. We wrote singular but intended plural. Ordinarily the ls command would be invoked with empty argument, showing the long listing of everything. But the -u triggers a fatal error, so ls or a more destructive command never runs.

---

level of abstraction

Building an application, such as a shell script, is about assembling generic basic building blocks into abstractions that fit your Use Case, until eventually you have bridged from low level vocabulary to a high level one where a single utterance accomplishes your business goal, such as installing from remote media.

When you define a copy function, you don't merely want a better "copy", you want one tailored to your higher level goal. The building block is happy to e.g. $ cp /dev/null empty.txt to create a zero-byte file. In your use case, it appears the goal is never helped by producing an empty file. So you might specialize the function to give a narrower range of functionality, requiring that it shall always produce a longer output or will die trying.

Once you've written a function, ask if it really solves a problem one level up from what the base utility solves. If not, perhaps defining DEFAULT_ARGS=... and passing that into the utility would suffice.

---

modern utility

cp was written long ago for a simple, specific function, and later it accreted several options.

rsync builds on decades of experience to solve a related, more general problem that sometimes includes distant servers. But it is very nice for local file copying, as well.

Here are the "copy" failure modes that tend to be the biggest practical concerns:

1. no permission to write to (root-owned?) destination
2. no free disk space
3. hardware: power fail or disk error

If either of the first two happen, the app shouldn't blithely soldier on assuming success; it should fail or recover. For the last scenario, we would hope a repeated task like a cron job would deal gracefully with the files it finds scattered around a directory following a reboot.

After set -e, both cp and rsync cope with the first two items perfectly well, issuing a close() and failing with non-zero exit status. For the last item an fsync() offers stronger guarantees than a close(). Fsync waits for data to hit the platter, while close does not.

Depending on your paranoia level and performance goals, you might prefer rsync -a --fsync src dst over cp -p src dst. Another nice aspect of rsync is it spends time writing the needed number of megabytes to a temp name like .dst123, and after seeing that succeed it does an atomic mv .dst123 dst. Consumers of dst will only see a good copy, never a partial one.

---

reveal the diagnostic

if cp ... &>/dev/null

When cp produces a diagnostic error message on stderr, it may prove helpful to humans. Reconsider whether you really need to discard that message.

I will sometimes anticipate a specific error which is common enough that the code knows how to cope with it, and discard just that:

cp ... 2>&1 | grep -v ': Permission denied$'

The idea is to not hide an unanticipated "surprise" when it eventually happens in production.