# Bash script - automate an upload image + description process on a server

I was tasked at work to improve a system where through a web interface a client, which eventually became us, uploads a bunch of images with descriptions (latter coming from csv files), the images are then automatically resized yada yada for the site.

Nowt wrong with the latter, but the upload and image description data entry is unbelievably tedious - the latter having to be filled out by hand and the uploads done through http.

The usual way to do this would be for me to create a more sensible interface. However, since it's us that end up doing it and me being inexperienced at *nix sysadmin type stuff and wanting to gain some experience, I thought it would be more interesting to write a shell script to do it accessed by SSH, on the basis we can upload the CSVs and image files easily enough. Have also used sed, perl, and a line of php to help me encode data as it comes from the CSV.

With the exception of removing my company name from the logo, here is the unaltered code:

How could this shell script be improved?

#! /bin/bash

# Yes, the below is ludicrous overkill.

# B
# U
# T

# I need to become good enough at shell scripting that I don't have to look up
# elementary syntax before I write anything that is not just a collection of
# commands.

# It was fun :)

function quitTheBestShellScriptInTheWorld {
tput setaf 0
tput sgr0
if [[ $tempFile != "" ]]; then rm$tempFile
fi
exit
}

function randomColour {
tput setaf $[ RANDOM % 8] if [[$1 == "e" ]]; then
echo
fi
}

do
randomColour
echo $eachLine done } trap quitTheBestShellScriptInTheWorld 0 1 2 3 15 #let's be tidy clear tput bold randomColour if [[$1 == "-l" ]]; then
echo "Er, logging feature not actually programmed. Add it yourself? Will be reasonably straightforward [y/n]"

if [[ $dolog == "y" ]]; then (nano coolscript.sh) else echo 'Wise choice, if you change your mind then run again and answer yes, or just edit this script (which is all that pressing y would have done)' fi exit 0 fi echo ' .___ | | _____ _____ ____ ____ | |/ \\__ \ / ___\_/ __ \ | | Y Y \/ __ \_/ /_/ > ___/ |___|__|_| (____ /\___ / \___ > \/ \//_____/ \/ __________ .__ \______ \ ____ _____|__|_______ ___________ | _// __ \ / ___/ \___ // __ \_ __ \ | | \ ___/ \___ \| |/ /\ ___/| | \/ |____|_ /\___ >____ >__/_____ \\___ >__| \/ \/ \/ \/ \/ ' randomColour echo Enter settings, or press enter for defaults: randomColour e echo Loading directory paths from Perl success=failure cd ../www/properties/lib/ uploadDataStore=$(perl -e 'use WIMSConfig; $z = WIMSConfig->new(); print$z->{uploadDataStore}')

if [[ $? == 0 ]] && [[$uploadDataStore != "" ]]; then
currentDataStore=$(perl -e 'use WIMSConfig;$z = WIMSConfig->new(); print $z->{currentDataStore}') if [[$? == 0 ]] && [[ $currentDataStore != "" ]]; then success=success fi fi if [[$success != "success" ]]; then
randomColour
echo "Failure: Problem setting directory paths from perl code"
exit 100
fi

tput setaf 4
randomColour e
echo fyi, I have uploadDataStore of $uploadDataStore and currentDataStore of$currentDataStore
echo
success=no
while [[ $success == "no" ]]; do randomColour echo 'Which directory from within uploadDataStore to get files from? (default = subfolder)' randomColour read theDirectory if [[$theDirectory == "" ]]; then
theDirectory=subfolder
fi

if [[ -e $uploadDataStore$theDirectory ]]; then
success=yes
else
randomColour
echo "Can't find a folder called $uploadDataStore$theDirectory"
echo
fi
done

declare -i NameCounter
declare -A fileNameToNumberMap
declare -A numberToFileNameMap

NameBuilder='Submit=Save+and+Process'

success=no
while [[ $success == "no" ]]; do randomColour e echo 'What is name of CSV file? (default = csvfile.csv)' randomColour read CSVFilename if [[$CSVFilename == "" ]]; then
CSVFilename=csvfile.csv
fi
CSVFilename=$uploadDataStore$theDirectory/$CSVFilename if [[ -e$CSVFilename ]]; then
success=yes
else
randomColour
echo Counld not find file by name of $CSVFilename, try again fi done tempfile=$(mktemp)

echo -n $NameBuilder >$tempfile
cat $CSVFilename | php -r 'while(($a = fgets(STDIN))!==false) echo str_replace("---___---","\n",str_replace("-_.-_.",",",urlencode(str_replace("\n","---___---",str_replace(",","-_.-_.",$a)))));' | awk -F "," '{prefix=sprintf("%d_",NR);printf("&%s_Name=%s&%sdataStore=%s&%sbranch=%s&%sdevelopmentName=%s&%saltDevelopmentName=%s&%slocation=%s&%scompletionYear=%s&%sdevelopmentType=%s&%sdevelopmentComprises=%s&%sstageOfCompletion=%s&%sshotType=%s&%sdetail%s&%simageType=%s&%scopyright=%s&%sphotoExists=%s&%soriginalFilename=%s",prefix,$1,prefix,$3,prefix,$4,prefix,$5,prefix,$6,prefix,$7,prefix,$8,prefix,$9,prefix,$10,prefix,$11,prefix,$12,prefix,$13,prefix,$14,prefix,$15,prefix,$16,prefix,$17)}' >>$tempfile
tempfileSize=$(stat -c%s$tempfile)
echo Starting Perl Script
randomColour e

cd ../

cat $tempfile | QUERY_STRING="Folder=$theDirectory&action=doProcessUploads" CONTENT_LENGTH=$tempfileSize perl -T index.cgi 2>&1 | sed -u -n '/NEW IMAGE/ {s/$$.*$$IMAGE$$.*$$/Perl looks like it is processing image\2/p}' | everyLineADifferentColour cd ../../upload/ randomColour e echo 'Hope you enjoyed the ****** Image Resizer Experience!' tput setaf 0 tput sgr0  ## 1 Answer Your code is so full of irrelevant content that distracts from the purpose of the code. The most Beautiful shell scripts are the ones that do a job, and do it well. They do it in a standard way with the least surprise. They accept input from pipes, redirects, and files, and they output in a way that is redirectable, loggable, and readable. The following is my long-winded-way of reviewing the superfluous fluff that makes what could otherwise be a Beautiful script (there is more serious content after that...) ## Meant to be in Jest stuff O *V* E *R* K *I* L *L* _________ __ \_ ___ \ ____ _____ _____ ____ _____/ |_ ______ / \ \/ / _ \ / \ / \_/ __ \ / \ __\/ ___/ \ \___( <_> ) Y Y \ Y Y \ ___/| | \ | \___ \ \______ /\____/|__|_| /__|_| /\___ >___| /__| /____ > \/ \/ \/ \/ \/ \/ .__ .__ .___ ___. _____| |__ ____ __ __| | __| _/ \_ |__ ____ / ___/ | \ / _ \| | \ | / __ | | __ \_/ __ \ \___ \| Y ( <_> ) | / |__/ /_/ | | \_\ \ ___/ /____ >___| /\____/|____/|____/\____ | |___ /\___ > \/ \/ \/ \/ \/ .___ ___. .__ .___ _______ ____ _____ __| _/____ \_ |__ | | ____ _____ ____ __| _/ \_ __ \_/ __ \\__ \ / __ |\__ \ | __ \| | _/ __ \ \__ \ / \ / __ | | | \/\ ___/ / __ \_/ /_/ | / __ \| \_\ \ |_\ ___/ / __ \| | \/ /_/ | |__| \___ >____ /\____ |(____ /___ /____/\___ > (____ /___| /\____ | \/ \/ \/ \/ \/ \/ \/ \/ \/ _____ .__ ._._._. __ __ ______ _____/ ____\_ __| || | | | | | \/ ___// __ \ __\ | \ || | | | | | /\___ \\ ___/| | | | / |_\|\|\| |____//____ >\___ >__| |____/|____/_____ \/ \/ \/\/\/  Gimmicks are fun, but, they get tiring fast. And, they break the 'spirit' of UNIX scripting, which is that they can do things for you in as many situations as possible. so, what if you wanted to use this in a pipe?  what if you are running this from a screen terminal? What if you are using a small mobile device to access things? What if the clutter simply overwhelms the rest of it, and the point of your program is lost. That is what has happened here • B • U • T Somewhere 1n th3r3 1s A re4l pr0gr4m, although... I was convinced at: quitTheBestShellScriptInTheWorld.  Who am I to be critical? At least you spelled colour correctly ;-) ## Serious stuff If you cut through the crap, you have an incomplete script... Lets go through some things.. Traps This is your trap: function quitTheBestShellScriptInTheWorld { tput setaf 0 tput sgr0 if [[$tempFile != "" ]]; then
rm $tempFile fi exit }  And you enable 20 lines later with: trap quitTheBestShellScriptInTheWorld 0 1 2 3 15 #let's be tidy  Apart from the humble function name, the system suffers because: • you should set the trap in the same code-location as the method so that it is obvious they belong together • there is no need for the exit inside the trap • It is more readable to use the signal numbers instead of the names • you only need to trap EXIT (which will be invoked whenever the shell exits (successful or not) • you should set a the tempFile variable so it has a clear value before the trap is set. It should look like: # Setting up the exit Clean-Up trap. tempfile="" function exiting { if [[ -f$tempFile ]]; then
rm $tempFile fi } trap exiting EXIT  Error Handling You have none.... and, it's really easy... do: set -e  near the top of your script. If any command fails then your program will terminate (with a non-zero exit code). If you have commands that you expect to fail, you can test them with if statements, or put conditionals after them so that the status is handled....  set -e Exit immediately if a pipeline (which may consist of a single simple command), a subshell command enclosed in parentheses, or one of the commands executed as part of a command list enclosed by braces (see SHELL GRAMMAR above) exits with a non-zero status. The shell does not exit if the command that fails is part of the command list immediately following a while or until keyword, part of the test following the if or elif reserved words, part of any command executed in a && or || list except the command following the final && or ||, any command in a pipeline but the last, or if the command's return value is being inverted with !. A trap on ERR, if set, is executed before the shell exits. This option applies to the shell environment and each subshell enviâ ronment separately (see COMMAND EXECUTION ENVIRONMENT above), and may cause subshells to exit before executing all the commands in the subshell.  Terminal Funkiness You clear the terminal.... and you play with terminal colour and boldness, etc. What if you are on a simple terminal that does not support these features? Your program will either fail, or will look really ugly. A user operating in a terminal is looking for efficiency and usability. The trates you have added are not efficient, and ruin the usability experience the user wants... (otherwise they would be in a GUI...) Printing out a huge banner is also not useful... what if the user has a small screen... it's ugly, randomColour echo Enter settings, or press enter for defaults: randomColour e  The above implies that you are expecting user input, but then you don't... odd. Directory Requirements You expect your user to be inside a specific directory, and this is not 'friendly'. It means a user cannot add the program to their $PATH and expect it to work:

cd ../www/properties/lib/


You can solve this problem by putting the script itself in a know directory, and then getting the actual directory name from there, and then using relative directories:

SCRIPT_PATH=$(cd dirname "${BASH_SOURCE[0]}" && pwd)


Then you can use that variable like:

cd $SCRIPT_PATH/../www/properties/lib/  although, that is not ideal, you should not need to be changing directories in the main part of the script. User Input You get a directory from the user.... inside a loop: while [[$success == "no" ]]; do


But, your indentation seems to die inside that loop. Makes it hard to read.

Also, you should have a mechanism for the stript to take the argument from the commandline, and only prompt for input if there was no commandline argument...

# check first argument.
theDirectory=$1 if [[ -d$theDirectory ]] ; then
success=yes
fi


Same thing for the CSVFile.

• s/quitTheBestSheelScriptInTheWorld/quitTheBestShellScriptInTheWorld/, or did I miss some joke? The question spelled Shell correctly ;-) – amon Feb 21 '14 at 22:41
• Although it can be helpful to point out flaws, it can be even more helpful to suggest changes for making things better. – Simon Forsberg Feb 21 '14 at 22:41
• After the edit this looks more like the monkey we all know and love! +1. – Simon Forsberg Feb 22 '14 at 11:54