Merging upstream version 126.0.

Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>

6 files changed, 161 insertions, 101 deletions

diff --git a/tools/update-verify/release/common/cached_download.sh b/tools/update-verify/release/common/cached_download.sh
index 7cb3c42f8d..4522b5504d 100644
--- a/tools/update-verify/release/common/cached_download.sh
+++ b/tools/update-verify/release/common/cached_download.sh
@@ -1,10 +1,12 @@
+#!/bin/bash
 # this library works like a wrapper around wget, to allow downloads to be cached
 # so that if later the same url is retrieved, the entry from the cache will be
 # returned.
 
-pushd `dirname $0` &>/dev/null
+pushd "$(dirname "$0")" &>/dev/null || exit
 cache_dir="$(pwd)/cache"
-popd &>/dev/null
+popd &>/dev/null || exit
+retry="$MY_DIR/../../../../mach python -m redo.cmd -s 1 -a 3"
 
 # Deletes all files in the cache directory
 # We don't support folders or .dot(hidden) files
@@ -12,7 +14,7 @@ popd &>/dev/null
 # which are the only workaround to poor mount r/w performance on MacOS
 # Reference: https://forums.docker.com/t/file-access-in-mounted-volumes-extremely-slow-cpu-bound/8076/288
 clear_cache () {
-    rm -rf "${cache_dir}/*"
+    rm -rf "${cache_dir:?}/*"
 }
 
 # download method - you pass a filename to save the file under, and the url to call
@@ -20,9 +22,10 @@ cached_download () {
     local output_file="${1}"
     local url="${2}"
 
-    if fgrep -x "${url}" "${cache_dir}/urls.list" >/dev/null; then
+    if grep -Fx "${url}" "${cache_dir}/urls.list" >/dev/null; then
         echo "Retrieving '${url}' from cache..."
-        local line_number="$(fgrep -nx  "${url}" "${cache_dir}/urls.list" | sed 's/:.*//')"
+        local line_number
+        line_number="$(grep -Fnx  "${url}" "${cache_dir}/urls.list" | sed 's/:.*//')"
         cp "${cache_dir}/obj_$(printf "%05d\n" "${line_number}").cache" "${output_file}"
     else
         echo "Downloading '${url}' and placing in cache..."
@@ -31,7 +34,8 @@ cached_download () {
         local exit_code=$?
         if [ "${exit_code}" == 0 ]; then
             echo "${url}" >> "${cache_dir}/urls.list"
-            local line_number="$(fgrep -nx  "${url}" "${cache_dir}/urls.list" | sed 's/:.*//')"
+            local line_number
+            line_number="$(grep -Fnx  "${url}" "${cache_dir}/urls.list" | sed 's/:.*//')"
             cp "${output_file}" "${cache_dir}/obj_$(printf "%05d\n" "${line_number}").cache"
         else
             return "${exit_code}"
diff --git a/tools/update-verify/release/common/check_updates.sh b/tools/update-verify/release/common/check_updates.sh
index acf06d8b4e..f03d8a069d 100644
--- a/tools/update-verify/release/common/check_updates.sh
+++ b/tools/update-verify/release/common/check_updates.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
 check_updates () {
   # called with 10 args - platform, source package, target package, update package, old updater boolean,
   # a path to the updater binary to use for the tests, a file to write diffs to, the update channel,
@@ -6,24 +8,34 @@ check_updates () {
   source_package=$2
   target_package=$3
   locale=$4
-  use_old_updater=$5
-  updater=$6
-  diff_file=$7
-  channel=$8
-  mar_channel_IDs=$9
-  update_to_dep=${10}
+  updater=$5
+  diff_file=$6
+  channel=$7
+  mar_channel_IDs=$8
+  update_to_dep=$9
+  local mac_update_settings_dir_override
+  mac_update_settings_dir_override=${10}
+  local product
+  product=${11}
 
   # cleanup
   rm -rf source/*
   rm -rf target/*
 
-  unpack_build $update_platform source "$source_package" $locale '' $mar_channel_IDs
-  if [ "$?" != "0" ]; then
+  # $mac_update_settings_dir_override allows unpack_build to find a host platform appropriate
+  # `update-settings.ini` file, which is needed to successfully run the updater later in this
+  # function.
+  if ! unpack_build "$update_platform" source "$source_package" "$locale" '' "$mar_channel_IDs" "$mac_update_settings_dir_override" "$product"; then
     echo "FAILED: cannot unpack_build $update_platform source $source_package"
     return 1
   fi
-  unpack_build $update_platform target "$target_package" $locale
-  if [ "$?" != "0" ]; then
+  # Unlike unpacking the `source` build, we don't actually _need_ $mac_update_settings_dir_override
+  # here to succesfully apply the update, but its usage in `source` causes an `update-settings.ini`
+  # file to be present in the directory we diff, which means we either also need it present in the
+  # `target` directory, or to remove it after the update is applied. The latter was chosen
+  # because it keeps the workaround close together (as opposed to just above this, and then much
+  # further down).
+  if ! unpack_build "$update_platform" target "$target_package" "$locale" '' '' "$mac_update_settings_dir_override" "$product"; then
     echo "FAILED: cannot unpack_build $update_platform target $target_package"
     return 1
   fi
@@ -36,30 +48,35 @@ check_updates () {
           platform_dirname="bin"
           ;;
       Linux_x86-gcc | Linux_x86-gcc3 | Linux_x86_64-gcc3)
-          platform_dirname=`echo $product | tr '[A-Z]' '[a-z]'`
+          platform_dirname=$(echo "$product" | tr '[:upper:]' '[:lower:]')
           ;;
   esac
 
   if [ -f update/update.status ]; then rm update/update.status; fi
   if [ -f update/update.log ]; then rm update/update.log; fi
 
+  # This check is disabled because we rely on glob expansion here
+  # shellcheck disable=SC2086
   if [ -d source/$platform_dirname ]; then
-    if [ `uname | cut -c-5` == "MINGW" ]; then
+    if [ "$(uname | cut -c-5)" == "MINGW" ]; then
       # windows
       # change /c/path/to/pwd to c:\\path\\to\\pwd
-      four_backslash_pwd=$(echo $PWD | sed -e 's,^/\([a-zA-Z]\)/,\1:/,' | sed -e 's,/,\\\\,g')
-      two_backslash_pwd=$(echo $PWD | sed -e 's,^/\([a-zA-Z]\)/,\1:/,' | sed -e 's,/,\\,g')
+      two_backslash_pwd=$(echo "$PWD" | sed -e 's,^/\([a-zA-Z]\)/,\1:/,' | sed -e 's,/,\\,g')
       cwd="$two_backslash_pwd\\source\\$platform_dirname"
       update_abspath="$two_backslash_pwd\\update"
     else
       # not windows
       # use ls here, because mac uses *.app, and we need to expand it
+      # This check is disabled because we rely on glob expansion here
+      # shellcheck disable=SC2086
       cwd=$(ls -d $PWD/source/$platform_dirname)
       update_abspath="$PWD/update"
     fi
 
+    # This check is disabled because we rely on glob expansion here
+    # shellcheck disable=SC2086
     cd_dir=$(ls -d ${PWD}/source/${platform_dirname})
-    cd "${cd_dir}"
+    cd "${cd_dir}" || (echo "TEST-UNEXPECTED-FAIL: couldn't cd to ${cd_dir}" && return 1)
     set -x
     "$updater" "$update_abspath" "$cwd" "$cwd" 0
     set +x
@@ -70,7 +87,7 @@ check_updates () {
   fi
 
   cat update/update.log
-  update_status=`cat update/update.status`
+  update_status=$(cat update/update.status)
 
   if [ "$update_status" != "succeeded" ]
   then
@@ -87,13 +104,17 @@ check_updates () {
   # positive from failing the tests, we simply remove it before diffing.
   # The precomplete file in Contents/Resources is still diffed, so we
   # don't lose any coverage by doing this.
-  cd `echo "source/$platform_dirname"`
+  # This check is disabled because we rely on glob expansion here
+  # shellcheck disable=SC2086
+  cd source/$platform_dirname || (echo "TEST-UNEXPECTED-FAIL: couldn't cd to source/${platform_dirname}" && exit 1)
   if [[ -f "Contents/Resources/precomplete" && -f "precomplete" ]]
   then
     rm "precomplete"
   fi
   cd ../..
-  cd `echo "target/$platform_dirname"`
+  # This check is disabled because we rely on glob expansion here
+  # shellcheck disable=SC2086
+  cd target/$platform_dirname || (echo "TEST-UNEXPECTED-FAIL: couldn't cd to target/${platform_dirname}" && exit 1)
   if [[ -f "Contents/Resources/precomplete" && -f "precomplete" ]]
   then
     rm "precomplete"
@@ -110,7 +131,28 @@ check_updates () {
     ignore_coderesources=
   fi
 
-  ../compare-directories.py source/${platform_dirname} target/${platform_dirname} ${channel} ${ignore_coderesources} > "${diff_file}"
+  # On Mac, there are two Frameworks that are not included with updates, and
+  # which change with every build. Because of this, we ignore differences in
+  # them in `compare-directories.py`. The best verification we can do for them
+  # is that they still exist.
+  if [[ $update_platform == Darwin_* ]]; then
+    # This check is disabled because we rely on glob expansion here
+    # shellcheck disable=SC2086
+    if ! compgen -G source/${platform_dirname}/Contents/MacOS/updater.app/Contents/Frameworks/UpdateSettings.framework >/dev/null; then
+      echo "TEST-UNEXPECTED-FAIL: UpdateSettings.framework doesn't exist after update"
+      return 4
+    fi
+    # This check is disabled because we rely on glob expansion here
+    # shellcheck disable=SC2086
+    if ! compgen -G source/${platform_dirname}/Contents/Frameworks/ChannelPrefs.framework >/dev/null; then
+      echo "TEST-UNEXPECTED-FAIL: ChannelPrefs.framework doesn't exist after update"
+      return 5
+    fi
+  fi
+
+  # This check is disabled because we rely on glob expansion here
+  # shellcheck disable=SC2086
+  ../compare-directories.py source/${platform_dirname} target/${platform_dirname} "${channel}" ${ignore_coderesources} > "${diff_file}"
   diffErr=$?
   cat "${diff_file}"
   if [ $diffErr == 2 ]
diff --git a/tools/update-verify/release/common/download_builds.sh b/tools/update-verify/release/common/download_builds.sh
index e279c808db..0374ecaffc 100644
--- a/tools/update-verify/release/common/download_builds.sh
+++ b/tools/update-verify/release/common/download_builds.sh
@@ -1,7 +1,4 @@
-pushd `dirname $0` &>/dev/null
-MY_DIR=$(pwd)
-popd &>/dev/null
-retry="$MY_DIR/../../../../mach python -m redo.cmd -s 1 -a 3"
+#!/bin/bash
 
 download_builds() {
   # cleanup
@@ -13,15 +10,15 @@ download_builds() {
 
   if [ -z "$source_url" ] || [ -z "$target_url" ]
   then
-    "download_builds usage: <source_url> <target_url>"
+    echo "download_builds usage: <source_url> <target_url>"
     exit 1
   fi
 
   for url in "$source_url" "$target_url"
     do
-    source_file=`basename "$url"`
+    source_file=$(basename "$url")
     if [ -f "$source_file" ]; then rm "$source_file"; fi
-    cd downloads 
+    cd downloads || exit 
     if [ -f "$source_file" ]; then rm "$source_file"; fi
     cached_download "${source_file}" "${url}"
     status=$?
diff --git a/tools/update-verify/release/common/download_mars.sh b/tools/update-verify/release/common/download_mars.sh
index d2dab107d2..84cbfe2ccc 100644
--- a/tools/update-verify/release/common/download_mars.sh
+++ b/tools/update-verify/release/common/download_mars.sh
@@ -1,3 +1,5 @@
+#!/bin/bash
+
 download_mars () {
     update_url="$1"
     only="$2"
@@ -23,42 +25,49 @@ download_mars () {
         fi
         echo "Empty response, sleeping"
         sleep 5
-        try=$(($try+1))
+        try=$((try+1))
     done
 
     echo; echo;  # padding
 
-    update_line=`fgrep "<update " update.xml`
+    update_line=$(grep -F "<update " update.xml)
     grep_rv=$?
     if [ 0 -ne $grep_rv ]; then
         echo "TEST-UNEXPECTED-FAIL: no <update/> found for $update_url"
         return 1
     fi
-    command=`echo $update_line | sed -e 's/^.*<update //' -e 's:>.*$::' -e 's:\&amp;:\&:g'`
+    command=$(echo "$update_line" | sed -e 's/^.*<update //' -e 's:>.*$::' -e 's:\&amp;:\&:g')
     eval "export $command"
 
-    if [ ! -z "$to_build_id" -a "$buildID" != "$to_build_id" ]; then
+    # buildID and some other variables further down are gathered by eval'ing
+    # the massaged `update.xml` file a bit further up. Because of this, shellcheck
+    # cannot verify their existence, and gets grumpy. Ideally we would do this
+    # differently, but it's not worth the trouble at the time of writing.
+    # shellcheck disable=SC2154
+    if [ -n "$to_build_id" ] && [ "$buildID" != "$to_build_id" ]; then
         echo "TEST-UNEXPECTED-FAIL: expected buildID $to_build_id does not match actual $buildID"
         return 1
     fi
 
-    if [ ! -z "$to_display_version" -a "$displayVersion" != "$to_display_version" ]; then
+    # shellcheck disable=SC2154
+    if [ -n "$to_display_version" ] && [ "$displayVersion" != "$to_display_version" ]; then
         echo "TEST-UNEXPECTED-FAIL: expected displayVersion $to_display_version does not match actual $displayVersion"
         return 1
     fi
 
-    if [ ! -z "$to_app_version" -a "$appVersion" != "$to_app_version" ]; then
+    # shellcheck disable=SC2154
+    if [ -n "$to_app_version" ] && [ "$appVersion" != "$to_app_version" ]; then
         echo "TEST-UNEXPECTED-FAIL: expected appVersion $to_app_version does not match actual $appVersion"
         return 1
     fi
 
     mkdir -p update/
-    if [ -z $only ]; then
+    if [ -z "$only" ]; then
       only="partial complete"
     fi
     for patch_type in $only
       do
-      line=`fgrep "patch type=\"$patch_type" update.xml`
+      line=$(grep -F "patch type=\"$patch_type" update.xml)
       grep_rv=$?
 
       if [ 0 -ne $grep_rv ]; then
@@ -66,40 +75,44 @@ download_mars () {
         return 1
       fi
 
-      command=`echo $line | sed -e 's/^.*<patch //' -e 's:/>.*$::' -e 's:\&amp;:\&:g'`
+      command=$(echo "$line" | sed -e 's/^.*<patch //' -e 's:/>.*$::' -e 's:\&amp;:\&:g')
       eval "export $command"
 
       if [ "$test_only" == "1" ]
       then
         echo "Testing $URL"
-        curl -s -I -L $URL
+        curl -s -I -L "$URL"
         return
       else
-        cached_download "update/${patch_type}.mar" "${URL}"
-      fi
-      if [ "$?" != 0 ]; then
-        echo "Could not download $patch_type!"
-        echo "from: $URL"
+        if ! cached_download "update/${patch_type}.mar" "${URL}"; then
+          echo "Could not download $patch_type!"
+          echo "from: $URL"
+        fi
       fi
-      actual_size=`perl -e "printf \"%d\n\", (stat(\"update/$patch_type.mar\"))[7]"`
-      actual_hash=`openssl dgst -$hashFunction update/$patch_type.mar | sed -e 's/^.*= //'`
+      actual_size=$(perl -e "printf \"%d\n\", (stat(\"update/$patch_type.mar\"))[7]")
+      # shellcheck disable=SC2154
+      actual_hash=$(openssl dgst -"$hashFunction" update/"$patch_type".mar | sed -e 's/^.*= //')
 
-      if [ $actual_size != $size ]; then
+      # shellcheck disable=SC2154
+      if [ "$actual_size" != "$size" ]; then
           echo "TEST-UNEXPECTED-FAIL: $patch_type from $update_url wrong size"
+          # shellcheck disable=SC2154
           echo "TEST-UNEXPECTED-FAIL: update.xml size: $size"
           echo "TEST-UNEXPECTED-FAIL: actual size: $actual_size"
           return 1
       fi
 
-      if [ $actual_hash != $hashValue ]; then
+      # shellcheck disable=SC2154
+      if [ "$actual_hash" != "$hashValue" ]; then
           echo "TEST-UNEXPECTED-FAIL: $patch_type from $update_url wrong hash"
+          # shellcheck disable=SC2154
           echo "TEST-UNEXPECTED-FAIL: update.xml hash: $hashValue"
           echo "TEST-UNEXPECTED-FAIL: actual hash: $actual_hash"
           return 1
       fi
 
-      cp update/$patch_type.mar update/update.mar
-      echo $actual_size > update/$patch_type.size
+      cp update/"$patch_type".mar update/update.mar
+      echo "$actual_size" > update/"$patch_type".size
 
     done
 }
diff --git a/tools/update-verify/release/common/unpack-diskimage.sh b/tools/update-verify/release/common/unpack-diskimage.sh
index b647a69c4d..760fe0bd33 100755
--- a/tools/update-verify/release/common/unpack-diskimage.sh
+++ b/tools/update-verify/release/common/unpack-diskimage.sh
@@ -53,43 +53,43 @@ TIMEOUT=90
 
 # If the mount point already exists, then the previous run may not have cleaned
 # up properly.  We should try to umount and remove the its directory.
-if [ -d $MOUNTPOINT ]; then
+if [ -d "$MOUNTPOINT" ]; then
     echo "$MOUNTPOINT already exists, trying to clean up"
-    hdiutil detach $MOUNTPOINT -force
-    rm -rdfv $MOUNTPOINT
+    hdiutil detach "$MOUNTPOINT" -force
+    rm -rdfv "$MOUNTPOINT"
 fi
 
 # Install an on-exit handler that will unmount and remove the '$MOUNTPOINT' directory
-trap "{ if [ -d $MOUNTPOINT ]; then hdiutil detach $MOUNTPOINT -force; rm -rdfv $MOUNTPOINT; fi; }" EXIT
+trap '{ if [ -d $MOUNTPOINT ]; then hdiutil detach $MOUNTPOINT -force; rm -rdfv $MOUNTPOINT; fi; }' EXIT
 
-mkdir -p $MOUNTPOINT
+mkdir -p "$MOUNTPOINT"
 
-hdiutil attach -verbose -noautoopen -mountpoint $MOUNTPOINT "$DMG_PATH" &> $LOGFILE
+hdiutil attach -verbose -noautoopen -mountpoint "$MOUNTPOINT" "$DMG_PATH" &> $LOGFILE
 # Wait for files to show up
 # hdiutil uses a helper process, diskimages-helper, which isn't always done its
 # work by the time hdiutil exits.  So we wait until something shows up in the
 # mount point directory.
 i=0
-while [ "$(echo $MOUNTPOINT/*)" == "$MOUNTPOINT/*" ]; do
-    if [ $i -gt $TIMEOUT ]; then
+while [ "$(echo "$MOUNTPOINT"/*)" == "$MOUNTPOINT/*" ]; do
+    if [ "$i" -gt $TIMEOUT ]; then
         echo "No files found, exiting"
         exit 1
     fi
     sleep 1
-    i=$(expr $i + 1)
+    i=$((i+1))
 done
 # Now we can copy everything out of the $MOUNTPOINT directory into the target directory
-rsync -av $MOUNTPOINT/* $MOUNTPOINT/.DS_Store $MOUNTPOINT/.background $MOUNTPOINT/.VolumeIcon.icns $TARGETPATH/ > $LOGFILE
+rsync -av "$MOUNTPOINT"/* "$MOUNTPOINT"/.DS_Store "$MOUNTPOINT"/.background "$MOUNTPOINT"/.VolumeIcon.icns "$TARGETPATH"/ > $LOGFILE
 # sometimes hdiutil fails with "Resource busy"
-hdiutil detach $MOUNTPOINT || { sleep  10; \
-    if [ -d $MOUNTPOINT ]; then hdiutil detach $MOUNTPOINT -force; fi; }
+hdiutil detach "$MOUNTPOINT" || { sleep  10; \
+    if [ -d "$MOUNTPOINT" ]; then hdiutil detach "$MOUNTPOINT" -force; fi; }
 i=0
-while [ "$(echo $MOUNTPOINT/*)" != "$MOUNTPOINT/*" ]; do
-    if [ $i -gt $TIMEOUT ]; then
+while [ "$(echo "$MOUNTPOINT"/*)" != "$MOUNTPOINT/*" ]; do
+    if [ "$i" -gt $TIMEOUT ]; then
         echo "Cannot umount, exiting"
         exit 1
     fi
     sleep 1
-    i=$(expr $i + 1)
+    i=$((i+1))
 done
-rm -rdf $MOUNTPOINT
+rm -rdf "$MOUNTPOINT"
diff --git a/tools/update-verify/release/common/unpack.sh b/tools/update-verify/release/common/unpack.sh
index 3249936493..a0e1204487 100755
--- a/tools/update-verify/release/common/unpack.sh
+++ b/tools/update-verify/release/common/unpack.sh
@@ -1,11 +1,5 @@
 #!/bin/bash
 
-function cleanup() { 
-    hdiutil detach ${DEV_NAME} || 
-      { sleep 5 && hdiutil detach ${DEV_NAME} -force; }; 
-    return $1 && $?; 
-};
-
 unpack_build () {
     unpack_platform="$1"
     dir_name="$2"
@@ -13,27 +7,35 @@ unpack_build () {
     locale=$4
     unpack_jars=$5
     update_settings_string=$6
+    # If provided, must be a directory containing `update-settings.ini` which
+    # will be used instead of attempting to find this file in the unpacked
+    # build. `update_settings_string` modifications will still be performed on
+    # the file.
+    local mac_update_settings_dir_override
+    mac_update_settings_dir_override=$7
+    local product
+    product=$8
 
     if [ ! -f "$pkg_file" ]; then
       return 1
     fi 
-    mkdir -p $dir_name
-    pushd $dir_name > /dev/null
+    mkdir -p "$dir_name"
+    pushd "$dir_name" > /dev/null || exit
     case $unpack_platform in
         # $unpack_platform is either
         # - a balrog platform name (from testing/mozharness/scripts/release/update-verify-config-creator.py)
         # - a simple platform name (from tools/update-verify/release/updates/verify.sh)
         mac|Darwin_*)
-            os=`uname`
+            os=$(uname)
             # How we unpack a dmg differs depending on which platform we're on.
             if [[ "$os" == "Darwin" ]]
             then
                 cd ../
                 echo "installing $pkg_file"
-                ../common/unpack-diskimage.sh "$pkg_file" mnt $dir_name
+                ../common/unpack-diskimage.sh "$pkg_file" mnt "$dir_name"
             else
                 7z x ../"$pkg_file" > /dev/null
-                if [ `ls -1 | wc -l` -ne 1 ]
+                if [ "$(find . -mindepth 1 -maxdepth 1 | wc -l)" -ne 1 ]
                 then
                     echo "Couldn't find .app package"
                     return 1
@@ -43,14 +45,18 @@ unpack_build () {
                 mv "${unpack_dir}"/*.app .
                 rm -rf "${unpack_dir}"
                 appdir=$(ls -1)
-                appdir=$(ls -d *.app)
-                # The updater guesses the location of these files based on
-                # its own target architecture, not the mar. If we're not
-                # unpacking mac-on-mac, we need to copy them so it can find
-                # them. It's important to copy (and not move), because when
-                # we diff the installer vs updated build afterwards, the
-                # installer version will have them in their original place.
-                cp "${appdir}/Contents/Resources/update-settings.ini" "${appdir}/update-settings.ini"
+                appdir=$(ls -d ./*.app)
+                if [ -d "${mac_update_settings_dir_override}" ]; then
+                    cp "${mac_update_settings_dir_override}/update-settings.ini" "${appdir}/update-settings.ini"
+                else
+                    # The updater guesses the location of these files based on
+                    # its own target architecture, not the mar. If we're not
+                    # unpacking mac-on-mac, we need to copy them so it can find
+                    # them. It's important to copy (and not move), because when
+                    # we diff the installer vs updated build afterwards, the
+                    # installer version will have them in their original place.
+                    cp "${appdir}/Contents/Resources/update-settings.ini" "${appdir}/update-settings.ini"
+                fi
                 cp "${appdir}/Contents/Resources/precomplete" "${appdir}/precomplete"
             fi
             update_settings_file="${appdir}/update-settings.ini"
@@ -64,7 +70,7 @@ unpack_build () {
               cp -rp localized/*    bin/
               rm -rf nonlocalized
               rm -rf localized
-              if [ $(find optional/ | wc -l) -gt 1 ]
+              if [ "$(find optional/ | wc -l)" -gt 1 ]
               then 
                 cp -rp optional/*     bin/
                 rm -rf optional
@@ -77,45 +83,43 @@ unpack_build () {
             else
               for file in *.xpi
               do
-                unzip -o $file > /dev/null
+                unzip -o "$file" > /dev/null
               done
-              unzip -o ${locale}.xpi > /dev/null
+              unzip -o "${locale}".xpi > /dev/null
             fi
             update_settings_file='bin/update-settings.ini'
             ;;
         linux|Linux_*)
-            if `echo $pkg_file | grep -q "tar.gz"`
+            if echo "$pkg_file" | grep -q "tar.gz"
             then
                 tar xfz ../"$pkg_file" > /dev/null
-            elif `echo $pkg_file | grep -q "tar.bz2"`
+            elif echo "$pkg_file" | grep -q "tar.bz2"
             then
                 tar xfj ../"$pkg_file" > /dev/null
             else
                 echo "Unknown package type for file: $pkg_file"
                 exit 1
             fi
-            update_settings_file=`echo $product | tr '[A-Z]' '[a-z]'`'/update-settings.ini'
+            update_settings_file=$(echo "$product" | tr '[:upper:]' '[:lower:]')'/update-settings.ini'
             ;;
         *)
             echo "Unknown platform to unpack: $unpack_platform"
             exit 1
     esac
 
-    if [ ! -z $unpack_jars ]; then
-        for f in `find . -name '*.jar' -o -name '*.ja'`; do
-            unzip -o "$f" -d "$f.dir" > /dev/null
-        done
+    if [ -n "$unpack_jars" ]; then
+        find . \( -name '*.jar' -o -name '*.ja' \) -exec unzip -o {} -d {}.dir \; >/dev/null
     fi
 
-    if [ ! -z $update_settings_string ]; then
+    if [ -n "$update_settings_string" ]; then
        echo "Modifying update-settings.ini"
-       cat  "${update_settings_file}" | sed -e "s/^ACCEPTED_MAR_CHANNEL_IDS.*/ACCEPTED_MAR_CHANNEL_IDS=${update_settings_string}/" > "${update_settings_file}.new"
+       sed -e "s/^ACCEPTED_MAR_CHANNEL_IDS.*/ACCEPTED_MAR_CHANNEL_IDS=${update_settings_string}/" < "${update_settings_file}" > "${update_settings_file}.new"
        diff -u "${update_settings_file}" "${update_settings_file}.new"
        echo " "
        rm "${update_settings_file}"
        mv "${update_settings_file}.new" "${update_settings_file}"
     fi
 
-    popd > /dev/null
+    popd > /dev/null || exit
 
 }