Merge pull request #305 from algernon/builder-fixes

Various kaleidoscope-builder fixes
7 years ago · cd94ffe24c
parent 842294af78 14fe42f5b3
commit cd94ffe24c
2 changed files with 111 additions and 80 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -2,10 +2,15 @@ dist: trusty
 sudo: false
 os:
  - linux
+addons:
+  apt:
+    packages:
+      - shellcheck
 install:
  - git clone --depth 1 --recurse-submodules https://github.com/keyboardio/Arduino-Boards hardware/keyboardio/avr
 script:
  - make travis-test BOARD_HARDWARE_PATH=$(pwd)/hardware
+  - shellcheck bin/kaleidoscope-builder
 notifications:
  irc:
    channels:
--- a/bin/kaleidoscope-builder
+++ b/bin/kaleidoscope-builder
@ -1,4 +1,4 @@
-#! /bin/sh
+#!/usr/bin/env bash

 set -e

@ -7,10 +7,10 @@ set -e
 ######

 build_version () {
-    GIT_VERSION="$(cd $(find_sketch); git describe --abbrev=4 --dirty --always)"
-    LIB_VERSION="$(cd $(find_sketch); (grep version= ../../library.properties 2>/dev/null || echo version=0.0.0) | cut -d= -f2)-g${GIT_VERSION}"
+    GIT_VERSION="$(cd "$(find_sketch)"; git describe --abbrev=4 --dirty --always)"
+    LIB_VERSION="$(cd "$(find_sketch)"; (grep version= ../../library.properties 2>/dev/null || echo version=0.0.0) | cut -d= -f2)-g${GIT_VERSION}"

-    BUILD_PATH="${BUILD_PATH:-$(mktemp -d 2>/dev/null || mktemp -d -t 'build')}"
+    BUILD_PATH="${BUILD_PATH:-"$(mktemp -d 2>/dev/null || mktemp -d -t 'build')"}"
    OUTPUT_DIR="${OUTPUT_DIR:-output/${LIBRARY}}"
    OUTPUT_PATH="${OUTPUT_PATH:-${SOURCEDIR}/${OUTPUT_DIR}}"
 }
@ -32,15 +32,18 @@ firmware_size () {

    MAX_PROG_SIZE=28672

-    output="$($@ | grep "\\(Program\\|Data\\):" | sed -e 's,^,  - ,' && echo)"
+    output="$("$@" | grep "\\(Program\\|Data\\):" | sed -e 's,^,  - ,' && echo)"

-    PROGSIZE="$(echo "${output}" | grep Program: | cut -d: -f2 | awk '{print $1}')"
+    PROGSIZE="$(echo "${output}" | grep "Program:" | cut -d: -f2 | awk '{print $1}')"

-    PERCENT="$(echo ${PROGSIZE} ${MAX_PROG_SIZE} | awk "{ printf \"%02.01f\", \$1 / \$2 * 100 }")"
+    PERCENT="$(echo "${PROGSIZE}" "${MAX_PROG_SIZE}" | awk "{ printf \"%02.01f\", \$1 / \$2 * 100 }")"

+    # we want the sed there, doing with shell builtins would be worse.
+    # shellcheck disable=SC2001
    echo "${output}" | sed -e "s/\(Program:.*\)(\([0-9\.]*%\) Full)/\1(${PERCENT}% Full)/"
 }

+
 find_sketch () {
    SKETCH="${SKETCH:-${DEFAULT_SKETCH}}"
    LIBRARY="${LIBRARY:-${SKETCH}}"
@ -68,7 +71,10 @@ prepare_to_flash () {
    fi

    echo "Press ENTER when ready..."
-    read a
+
+    # We do not want to permit line continuations here. We just want a newline.
+    # shellcheck disable=SC2162
+    read
 }

 flash () {
@ -85,7 +91,7 @@ flash_over_usb () {
        return 1
    fi
    sleep 1s
-    ${AVRDUDE} -q -q -C ${AVRDUDE_CONF} -p${MCU} -cavr109 -D -P ${DEVICE_PORT_BOOTLOADER} -b57600 "-Uflash:w:${HEX_FILE_PATH}:i"
+    ${AVRDUDE} -q -q -C "${AVRDUDE_CONF}" -p"${MCU}" -cavr109 -D -P "${DEVICE_PORT_BOOTLOADER}" -b57600 "-Uflash:w:${HEX_FILE_PATH}:i"
 }

 flash_from_bootloader() {
@ -101,8 +107,8 @@ program() {

 flash_with_programmer() {
    ${AVRDUDE} -v \
-	       -C ${AVRDUDE_CONF} \
-	       -p${MCU} \
+	       -C "${AVRDUDE_CONF}" \
+	       -p"${MCU}" \
 	       -cusbtiny \
 	       -D \
 	       -B 1 \
@ -114,12 +120,12 @@ hex_with_bootloader () {
        compile
    fi

-    cat ${HEX_FILE_PATH} | awk '/^:00000001FF/ == 0' > ${HEX_FILE_WITH_BOOTLOADER_PATH}
+    awk '/^:00000001FF/ == 0' "${HEX_FILE_PATH}" > "${HEX_FILE_WITH_BOOTLOADER_PATH}"
    echo "Using ${BOOTLOADER_PATH}"
-    ${MD5} ${BOOTLOADER_PATH}
-    cat ${BOOTLOADER_PATH} >> ${HEX_FILE_WITH_BOOTLOADER_PATH}
-    ln -sf "${HEX_FILE_WITH_BOOTLOADER_PATH}" "${OUTPUT_PATH}/${SKETCH}-latest-with-bootloader.hex"
-    cat <<EOF
+    ${MD5} "${BOOTLOADER_PATH}"
+    cat "${BOOTLOADER_PATH}" >> "${HEX_FILE_WITH_BOOTLOADER_PATH}"
+    ln -sf -- "${HEX_FILE_WITH_BOOTLOADER_PATH}" "${OUTPUT_PATH}/${SKETCH}-latest-with-bootloader.hex"
+    cat <<- EOF

 		Combined firmware and bootloader are now at ${HEX_FILE_WITH_BOOTLOADER_PATH}
 		Make sure you have the bootloader version you expect.
@ -130,8 +136,8 @@ EOF
 }

 build () {
-    compile $@
-    size $@
+    compile "$@"
+    size "$@"
 }

 compile () {
@ -140,17 +146,27 @@ compile () {
    install -d "${OUTPUT_PATH}"

    echo "Building ${OUTPUT_DIR}/${SKETCH} (${LIB_VERSION}) ..."
+
+    # This is defined in the (optional) user config.
+    # shellcheck disable=SC2154
    ${compile_HOOKS}

    if [ -d "${ARDUINO_LOCAL_LIB_PATH}/libraries" ]; then
+        # shellcheck disable=SC2089
+        # We want literal backslashes here, not arrays.
        local_LIBS="-libraries \"${ARDUINO_LOCAL_LIB_PATH}/libraries\""
    fi

    ARDUINO_PACKAGES=""
-    if [ -d ${ARDUINO_PACKAGE_PATH} ]; then
+    if [ -d "${ARDUINO_PACKAGE_PATH}" ]; then
+        # shellcheck disable=SC2089
+        # We want literal backslashes here, not arrays.
 	      ARDUINO_PACKAGES="-hardware \"${ARDUINO_PACKAGE_PATH}\""
    fi

+    # SC2091: We do not care if quotes or backslashes are not respected.
+    # SC2086: We want word splitting.
+    # shellcheck disable=SC2086,SC2090
    ${ARDUINO_BUILDER} \
        -compile \
 	      ${ARDUINO_PACKAGES} \
@ -188,7 +204,7 @@ _find_all () {
    for plugin in ./*.ino \
 	    	          examples/* \
                  src/*.ino; do
-        if [ -d "$(dirname ${plugin})" ] || [ -f "${plugin}" ]; then
+        if [ -d "$(dirname "${plugin}")" ] || [ -f "${plugin}" ]; then
            p="$(basename "${plugin}" .ino)"
            if [ "${p}" != '*' ]; then
                echo "${p}"
@ -203,7 +219,7 @@ build_all () {
    for plugin in ${plugins}; do
        export SKETCH="${plugin}"
        export LIBRARY="${plugin}"
-        $0 ${plugin} build
+        $0 "${plugin}" build
    done
 }

@ -239,7 +255,7 @@ decompile () {
 }

 clean () {
-    rm -rf "${OUTPUT_PATH}"
+    rm -rf -- "${OUTPUT_PATH}"
 }

 reset_device() {
@ -277,11 +293,11 @@ EOF

 $DEVICE_PORT is not writable:

-  `ls -l $DEVICE_PORT`
+  $(ls -l "$DEVICE_PORT")

 You are currently in the following groups:

-  `id -Gn`
+  $(id -Gn)

 Please ensure you have followed the instructions on setting up your
 account to be in the right group:
@ -294,7 +310,7 @@ EOF
 }

 usage () {
-    cat <<EOF
+    cat <<- EOF
 		Usage: $0 SKETCH commands...

 		Runs all of the commands in the context of the Sketch.
@ -349,23 +365,28 @@ fi
 ##
 ##  - if there is only one argument, that's a command

-ROOT="$(cd $(dirname $0)/..; pwd)"
+ROOT="$(cd "$(dirname "$0")"/..; pwd)"
 export ROOT
+# shellcheck disable=SC2155
 export SOURCEDIR="$(pwd)"

 if [ -e "${HOME}/.kaleidoscope-builder.conf" ]; then
+    # shellcheck disable=SC1090
    . "${HOME}/.kaleidoscope-builder.conf"
 fi

 if [ -e "${SOURCEDIR}/.kaleidoscope-builder.conf" ]; then
+    # shellcheck disable=SC1090
    . "${SOURCEDIR}/.kaleidoscope-builder.conf"
 fi

 if [ -e "${SOURCEDIR}/kaleidoscope-builder.conf" ]; then
+    # shellcheck disable=SC1090
    . "${SOURCEDIR}/kaleidoscope-builder.conf"
 fi

-. ${ROOT}/etc/kaleidoscope-builder.conf
+# shellcheck disable=SC1090
+. "${ROOT}/etc/kaleidoscope-builder.conf"

 if [ ! -z "${VERBOSE}" ] && [ "${VERBOSE}" -gt 0 ]; then
    ARDUINO_VERBOSE="-verbose"
@ -382,6 +403,8 @@ for i in $(seq 1 $#); do

    case "${v}" in
        *=*)
+            # Exporting an expansion is *precisely* what we want here.
+            # shellcheck disable=SC2086,SC2163
            export ${v}
            ;;
        *)
@ -390,10 +413,12 @@ for i in $(seq 1 $#); do
    esac
 done

+# Word splitting is desired here.
+# shellcheck disable=SC2086
 set -- ${cmds}

 if [ $# -eq 1 ]; then
-    cmd="$(echo $1 | tr '-' '_')"
+    cmd="$(echo "$1" | tr '-' '_')"
    ${cmd}
    exit $?
 fi
@ -407,8 +432,9 @@ fi

 cmds=""

+# shellcheck disable=2034
 for i in $(seq 1 $#); do
-    cmds="${cmds} $(echo $1 | tr '-' '_')"
+    cmds="${cmds} $(echo "$1" | tr '-' '_')"
    shift
 done