This post was extracted from https://alexschroeder.ch/wiki/Comments_on_2014-06-23_Oddmuse_Migration
It all started like a simple bash refactoring, but look! This time was special because someone decided to post a link to google style guide and claimed that it is a "nice resource". Well... I just had to explain everything. At first, I didn't think that my answer is worth a blog post. But hey, it seems like a lot of people worship these guides... So I decided to extract it into my blog.
It is plain bullshit.
Well, I can quickly go through most of the mistakes:
for dir in ${dirs_to_cleanup}; do
Clearly $dirs_to_cleanup is not quoted, this indicates that there is something wrong. Better use array and "${dirs_to_cleanup[@]}".
Also, what is the point of using curlies like ${var} everywhere??? They'd better teach to put quotes everywhere. Such style is propagated only by Google style guide (a guide written by people who think that you're stupid enough not to understand the two exceptions when ${} is actually required, but at the same time these guys don't even know how to quote variables correctly). If that made any sense then we would be doing the same thing in perl and php, but we don't.
rm "${dir}/${ORACLE_SID}/"*
That's rm without -- . Well, not a huge issue, but that's a styleguide or what?
if [[ "$?" -ne 0 ]]; then
Funny. Left side of the expression is quoted when it does not have to be. But even if it had to be quoted, what would be the benefit to quote $? ? It is always an integer. Oh, and if it is an integer, then it would be better to use (( )) for arithmetic expressions, like: if (( $? != 0 )); then
case "${expression}" in
Does not have to be quoted, actually, but at least it does not hurt.
while read f; do
echo "file=${f}"
done < <(ls -l /tmp)
WHOA! Holy sh...!!! That's amazing... Bash pitfall #1, parsing ls, read without -r. It just couldn't be worse.
# -z (string length is zero) and -n (string length is not zero) are
# preferred over testing for an empty string
if [[ -z "${my_var}" ]]; then
do_something
fi
But hey, it is as simlpe as if [[ ! $my_var ]]; then , why would they complicate things so much?
# Instead of this as errors can occur if ${my_var} expands to a test
# flag
if [[ "${my_var}" ]]; then
do_something
fi
What??? Which error?? Am I missing something?
zip_version="$(dpkg --status zip | grep Version: | cut -d ' ' -f 2)"
A rule of thumb is that you should never pipe any of these commands together: sed, awk, grep, perl, cut and some others. The reason is that we usually need such functionality that is present in most of them, and indeed with a little trickery these tools will usually do each others' job. However, they use completely different approaches.
In this particular case you can use grep with -P flag (perl regexes! Woohoo!). Like this:
zip_version=$(dpkg --status zip | grep -Pom1 'Version: \K.*')
But there are other solutions as well.
if ! mv "${file_list}" "${dest_dir}/" ; then
Just look at that variable. It says "file list". Aha, sure, this would definitely work.
addition=$((${X} + ${Y}))
Stupidity all over the place. This should be:
((addition = X + Y))
At least that "$file_list" example will give me a good laugh for the next few days. 😂
Also, I feel like this post is lacking some image. Well, you can look at this image, if you wish.
