shithub: orca

Download patch

ref: 97b5683072987008e0cc748e3da3b59e515edfaf
parent: 692f803e0c06a669e3e2b3d21bc8dd07e135b4e6
author: cancel <cancel@cancel.fm>
date: Sat Feb 13 09:42:14 EST 2021

Change 'tool' build script from bash to POSIX sh

The 'tool' build script previously required bash. This commit changes it
to need only POSIX sh, which removes a dependency from the project. bash
is a bit easier to work with than POSIX sh, but the trade-off seems
worth it in this case. We already require POSIX, so we're guaranteed to
have access to POSIX sh. Some systems may not have bash.

As an added bonus, the startup time of the script will probably be
slightly faster by a few milliseconds, because some implementations like
dash start up faster than bash.

One downside is that POSIX sh doesn't have a built-in for 'time', and
one of my testing environments, a stripped-down Ubuntu image, didn't
have 'time' installed by default. I believe 'time' is mandatory in
POSIX, so that's a bit strange. I think this might be a common thing, so
I added a case to handle it in the 'tool' script, instead of having the
script fail.

--- a/tool
+++ b/tool
@@ -1,5 +1,5 @@
-#!/usr/bin/env bash
-set -eu -o pipefail
+#!/bin/sh
+set -euf
 
 print_usage() {
 cat <<EOF
@@ -40,8 +40,8 @@
 EOF
 }
 
-if [[ -z "${1:-}" ]]; then
-  echo "Error: Command required" >&2
+if [ -z "${1:-}" ]; then
+  printf 'Error: Command required\n' >&2
   print_usage >&2
   exit 1
 fi
@@ -58,9 +58,13 @@
   *) os=unknown;;
 esac
 
+if [ $os = unknown ]; then
+  warn "Build script not tested on this platform"
+fi
+
 cc_exe="${CC:-cc}"
 
-if [[ $os = cygwin ]]; then
+if [ $os = cygwin ]; then
   # Under cygwin, specifically ignore the mingw compilers if they're set as the
   # CC environment variable. This may be the default from the cygwin installer.
   # But we want to use 'gcc' from the cygwin gcc-core package (probably aliased
@@ -90,9 +94,9 @@
 config_mode=release
 
 while getopts c:dhsv-: opt_val; do
-  case "$opt_val" in
+  case $opt_val in
     -)
-      case "$OPTARG" in
+      case $OPTARG in
         harden) protections_enabled=1;;
         help) print_usage; exit 0;;
         static) static_enabled=1;;
@@ -102,13 +106,12 @@
         mouse) mouse_disabled=0;;
         no-mouse|nomouse) mouse_disabled=1;;
         *)
-          echo "Unknown long option --$OPTARG" >&2
-          print_usage >&2
+          printf 'Unknown option --%s\n' "$OPTARG" >&2
           exit 1
           ;;
       esac
       ;;
-    c) cc_exe="$OPTARG";;
+    c) cc_exe=$OPTARG;;
     d) config_mode=debug;;
     h) print_usage; exit 0;;
     s) stats_enabled=1;;
@@ -125,34 +128,34 @@
 esac
 
 warn() {
-  echo "Warning: $*" >&2
+  printf 'Warning: %s\n' "$*" >&2
 }
 fatal() {
-  echo "Error: $*" >&2
+  printf 'Error: %s\n' "$*" >&2
   exit 1
 }
 script_error() {
-  echo "Script error: $*" >&2
+  printf 'Script error: %s\n' "$*" >&2
   exit 1
 }
 
 verbose_echo() {
-  if [[ $verbose = 1 ]]; then
-    echo "$@"
+  # Don't print 'timed_stats' if it's the first part of the command
+  if [ $verbose = 1 ] && [ $# -gt 1 ]; then
+    printf '%s ' "$@" | sed -E -e 's/^timed_stats[[:space:]]+//' -e 's/ $//' | tr -d '\n'
+    printf '\n'
   fi
   "$@"
 }
 
-TIMEFORMAT='%3R'
-
-last_time=
-
 file_size() {
   wc -c < "$1" | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//'
 }
 
+last_time=
 timed_stats() {
-  if [[ $stats_enabled = 1 ]]; then
+  if [ $stats_enabled = 1 ] && command -v time >/dev/null 2>&1; then
+    TIMEFORMAT='%3R'
     { last_time=$( { time "$@" 1>&3- 2>&4-; } 2>&1 ); } 3>&1 4>&2
   else
     "$@"
@@ -160,42 +163,62 @@
 }
 
 version_string_normalized() {
-  echo "$@" | awk -F. '{ printf("%d%03d%03d%03d\n", $1,$2,$3,$4); }';
+  printf '%s\n' "$@" | awk -F. '{ printf("%d%03d%03d%03d\n", $1,$2,$3,$4); }';
 }
 
-if [[ ($os == unknown) ]]; then
-  warn "Build script not tested on this platform"
-fi
-
-# This is not perfect by any means
 cc_id=
 cc_vers=
 lld_detected=0
-if cc_vers=$(echo -e '#ifndef __clang__\n#error Not found\n#endif\n__clang_major__.__clang_minor__.__clang_patchlevel__' | "$cc_exe" -E -xc - 2>/dev/null | tail -n 1 | tr -d '\040'); then
-  cc_id=clang
-  # Mac clang/llvm doesn't say the real version of clang. Just assume it's 3.9.0
-  if [[ $os == mac ]]; then
-    cc_vers=3.9.0
-  else
-    if command -v "lld" >/dev/null 2>&1; then
-      lld_detected=1
-    fi
-  fi
-elif cc_vers=$(echo -e '#ifndef __GNUC__\n#error Not found\n#endif\n__GNUC__.__GNUC_MINOR__.__GNUC_PATCHLEVEL__' | "$cc_exe" -E -xc - 2>/dev/null | tail -n 1 | tr -d '\040'); then
-  cc_id=gcc
-elif cc_vers=$(echo -e '#ifndef __TINYC__\n#error Not found\n#endif\n__TINYC__' | "$cc_exe" -E -xc - 2>/dev/null | tail -n 1 | tr -d '\040'); then
-  cc_id=tcc
+lld_name=lld
+if preproc_result=$( ("$cc_exe" -E -xc - 2>/dev/null | tail -n 2 | tr -d '\040') <<EOF
+#if defined(__clang__)
+clang
+__clang_major__.__clang_minor__.__clang_patchlevel__
+#elif defined(__GNUC__)
+gcc
+__GNUC__.__GNUC_MINOR__.__GNUC_PATCHLEVEL__
+#elif defined(__TINYC__)
+tcc
+__TINYC__
+#else
+#error Unknown compiler
+#endif
+EOF
+); then
+  cc_id=$(printf %s "$preproc_result" | head -n 1)
+  cc_vers=$(printf %s "$preproc_result" | tail -n 1)
 fi
 
-if [[ -z $cc_id ]]; then
+if [ "$cc_id" = clang ]; then
+  case $os in
+    # Mac clang/llvm doesn't say the real version of clang. Assume it's 3.9.0.
+    mac) cc_vers=3.9.0;;
+    *)
+      # Debian names versions clang like "clang-9" and also LLD like "lld-9".
+      # To tell clang to use LLD, we have to pass an argument like
+      # '-fuse-ld=lld'. You would expect that the Debian versions of clang,
+      # like clang-9, would want '-fuse-ld=lld-9', but it seems to work both as
+      # '-fuse-ld=lld-' and also as '-fuse-ld=lld'. I'm not sure if this holds
+      # true if multiple versions of clang are installed.
+      if output=$(printf %s "$cc_exe" | awk -F- '
+          /^clang\+?\+?-/ && $NF ~ /^[0-9]+$/ { a=$NF }
+          END { if (a == "") exit -1; printf("lld-%s", a) }'); then
+        lld_name=$output
+      fi
+      if command -v "$lld_name" >/dev/null 2>&1; then lld_detected=1; fi;;
+  esac
+fi
+
+if [ -z "$cc_id" ]; then
   warn "Failed to detect compiler type"
 fi
-if [[ -z $cc_vers ]]; then
+if [ -z "$cc_vers" ]; then
   warn "Failed to detect compiler version"
 fi
 
 cc_vers_is_gte() {
-  if [[ $(version_string_normalized "$cc_vers") -ge $(version_string_normalized "$1") ]]; then
+  if [ "$(version_string_normalized "$cc_vers")" -ge \
+       "$(version_string_normalized "$1")" ]; then
     return 0
   else
     return 1
@@ -203,7 +226,7 @@
 }
 
 cc_id_and_vers_gte() {
-  if [[ $cc_id == "$1" ]] && cc_vers_is_gte "$2"; then
+  if [ "$cc_id" = "$1" ] && cc_vers_is_gte "$2"; then
     return 0
   else
     return 1
@@ -211,30 +234,24 @@
 }
 
 add() {
-  if [[ -z "${1:-}" ]]; then
+  if [ -z "${1:-}" ]; then
     script_error "At least one argument required for array add"
   fi
-  local array_name
-  array_name=${1}
+  _add_name=${1}
   shift
-  eval "$array_name+=($(printf "'%s' " "$@"))"
+  while [ -n "${1+x}" ]; do
+    # shellcheck disable=SC2034
+    _add_hidden=$1
+    eval "$_add_name"'=$(printf '"'"'%s\n%s.'"' "'"$'"$_add_name"'" "$_add_hidden")'
+    eval "$_add_name"'=${'"$_add_name"'%.}'
+    shift
+  done
 }
 
-concat() {
-  if [[ -z "${1:-}" || -z "${2:-}" ]]; then
-    script_error "Two arguments required for array concat"
-  fi
-  local lhs_name
-  local rhs_name
-  lhs_name=${1}
-  rhs_name=${2}
-  eval "$lhs_name+=(\"\${${rhs_name}[@]}\")"
-}
-
 try_make_dir() {
-  if ! [[ -e "$1" ]]; then
+  if ! [ -e "$1" ]; then
     verbose_echo mkdir "$1"
-  elif ! [[ -d "$1" ]]; then
+  elif ! [ -d "$1" ]; then
     fatal "File $1 already exists but is not a directory"
   fi
 }
@@ -242,10 +259,10 @@
 build_dir=build
 
 build_target() {
-  local cc_flags=()
-  local libraries=()
-  local source_files=()
-  local out_exe
+  cc_flags=
+  libraries=
+  source_files=
+  out_exe=
   add cc_flags -std=c99 -pipe -finput-charset=UTF-8 -Wall -Wpedantic -Wextra \
     -Wwrite-strings
   if cc_id_and_vers_gte gcc 6.0.0 || cc_id_and_vers_gte clang 3.9.0; then
@@ -253,10 +270,10 @@
       -Werror=implicit-function-declaration -Werror=implicit-int \
       -Werror=incompatible-pointer-types -Werror=int-conversion
   fi
-  if [[ $cc_id = tcc ]]; then
+  if [ "$cc_id" = tcc ]; then
     add cc_flags -Wunsupported
   fi
-  if [[ $os = mac && $cc_id = clang ]]; then
+  if [ $os = mac ] && [ "$cc_id" = clang ]; then
     # The clang that's shipped with Mac 10.12 has bad behavior for issuing
     # warnings for structs initialed with {0} in C99. We have to disable this
     # warning, or it will issue a bunch of useless warnings. It might be fixed
@@ -264,19 +281,19 @@
     # indecipherable, so we'll just always turn it off.
     add cc_flags -Wno-missing-field-initializers
   fi
-  if [[ $lld_detected = 1 ]]; then
-    add cc_flags -fuse-ld=lld
+  if [ $lld_detected = 1 ]; then
+    add cc_flags "-fuse-ld=$lld_name"
   fi
-  if [[ $protections_enabled = 1 ]]; then
+  if [ $protections_enabled = 1 ]; then
     add cc_flags -D_FORTIFY_SOURCE=2 -fstack-protector-strong
   fi
-  if [[ $pie_enabled = 1 ]]; then
+  if [ $pie_enabled = 1 ]; then
     add cc_flags -pie -fpie -Wl,-pie
   # Only explicitly specify no-pie if cc version is new enough
   elif cc_id_and_vers_gte gcc 6.0.0 || cc_id_and_vers_gte clang 6.0.0; then
     add cc_flags -no-pie -fno-pie
   fi
-  if [[ $static_enabled = 1 ]]; then
+  if [ $static_enabled = 1 ]; then
     add cc_flags -static
   fi
   case $config_mode in
@@ -283,7 +300,7 @@
     debug)
       add cc_flags -DDEBUG -ggdb
       # cygwin gcc doesn't seem to have this stuff, just elide for now
-      if [[ $os != cygwin ]]; then
+      if [ $os != cygwin ]; then
         if cc_id_and_vers_gte gcc 6.0.0 || cc_id_and_vers_gte clang 3.9.0; then
           add cc_flags -fsanitize=address -fsanitize=undefined \
             -fsanitize=float-divide-by-zero
@@ -293,14 +310,11 @@
             -fsanitize=unsigned-integer-overflow
         fi
       fi
-      if [[ $os = mac ]]; then
+      if [ $os = mac ]; then
         # Our mac clang does not have -Og
         add cc_flags -O1
       else
         add cc_flags -Og
-        # needed if address is already specified? doesn't work on mac clang, at
-        # least
-        # add cc_flags -fsanitize=leak
       fi
       case $cc_id in
         tcc) add cc_flags -g -bt10;;
@@ -308,25 +322,25 @@
       ;;
     release)
       add cc_flags -DNDEBUG -O2 -g0
-      if [[ $protections_enabled != 1 ]]; then
+      if [ $protections_enabled != 1 ]; then
         add cc_flags -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
         case $cc_id in
           gcc|clang) add cc_flags -fno-stack-protector;;
         esac
       fi
-      if [[ $os = mac ]]; then
-        # todo some stripping option
-        true
-      else
-        # -flto is good on both clang and gcc on Linux
-        case $cc_id in
-          gcc|clang)
-            if [[ $os != bsd ]]; then
-              add cc_flags -flto
-            fi
-        esac
-        add cc_flags -s
-      fi
+      case $os in
+        mac) ;; # todo some stripping option
+        *)
+          # -flto is good on both clang and gcc on Linux
+          case $cc_id in
+            gcc|clang)
+              if [ $os != bsd ]; then
+                add cc_flags -flto
+              fi
+          esac
+          add cc_flags -s
+          ;;
+      esac
       ;;
     *) fatal "Unknown build config \"$config_mode\"";;
   esac
@@ -369,14 +383,14 @@
       out_exe=orca
       case $os in
         mac)
-          local brew_prefix=
           if ! brew_prefix=$(printenv HOMEBREW_PREFIX); then
-             brew_prefix=/usr/local/
+             brew_prefix=/usr/local
           fi
-          local ncurses_dir="$brew_prefix/opt/ncurses"
-          if ! [[ -d "$ncurses_dir" ]]; then
-            echo "Error: ncurses directory not found at $ncurses_dir" >&2
-            echo "Install with: brew install ncurses" >&2
+          ncurses_dir="$brew_prefix/opt/ncurses"
+          if ! [ -d "$ncurses_dir" ]; then
+            printf 'Error: ncurses directory not found at %s\n' \
+              "$ncurses_dir" >&2
+            printf 'Install with: brew install ncurses\n' >&2
             exit 1
           fi
           # prefer homebrew version of ncurses if installed. Will give us
@@ -384,11 +398,12 @@
           add libraries "-L$ncurses_dir/lib"
           add cc_flags "-I$ncurses_dir/include"
           # todo mach time stuff for mac?
-          if [[ $portmidi_enabled = 1 ]]; then
-            local portmidi_dir="$brew_prefix/opt/portmidi"
-            if ! [[ -d "$portmidi_dir" ]]; then
-              echo "Error: PortMidi directory not found at $portmidi_dir" >&2
-              echo "Install with: brew install portmidi" >&2
+          if [ $portmidi_enabled = 1 ]; then
+            portmidi_dir="$brew_prefix/opt/portmidi"
+            if ! [ -d "$portmidi_dir" ]; then
+              printf 'Error: PortMidi directory not found at %s\n' \
+                "$portmidi_dir" >&2
+              printf 'Install with: brew install portmidi\n' >&2
               exit 1
             fi
             add libraries "-L$portmidi_dir/lib"
@@ -398,7 +413,7 @@
           add cc_flags -DORCA_OS_MAC
         ;;
         bsd)
-          if [[ $portmidi_enabled = 1 ]]; then
+          if [ $portmidi_enabled = 1 ]; then
             add libraries "-L/usr/local/lib"
             add cc_flags "-I/usr/local/include"
           fi
@@ -413,11 +428,12 @@
       # as a separate library that explicitly needs to be linked, or it might
       # not. And if it does, it might need to be either -ltinfo or -ltinfow.
       # Yikes. If this is Linux, let's try asking pkg-config what it thinks.
-      local curses_flags=0
-      if [[ $os == linux ]]; then
+      curses_flags=0
+      if [ $os = linux ]; then
         if curses_flags=$(pkg-config --libs ncursesw formw 2>/dev/null); then
-          # split by spaces into separate args, then append to array
-          IFS=" " read -r -a libraries <<< "$curses_flags"
+          # Split by spaces intentionall
+          # shellcheck disable=SC2086
+          IFS=' ' add libraries $curses_flags
           curses_flags=1
         else
           curses_flags=0
@@ -425,44 +441,60 @@
       fi
       # If we didn't get the flags by pkg-config, just guess. (This will work
       # most of the time, including on Mac with Homebrew, and cygwin.)
-      if [[ $curses_flags = 0 ]]; then
+      if [ $curses_flags = 0 ]; then
         add libraries -lncursesw -lformw
       fi
-      if [[ $portmidi_enabled = 1 ]]; then
+      if [ $portmidi_enabled = 1 ]; then
         add libraries -lportmidi
         add cc_flags -DFEAT_PORTMIDI
-        if [[ $config_mode = debug ]]; then
-          echo -e "Warning: The PortMidi library contains code that may trigger address sanitizer in debug builds.\\nThese are not bugs in orca." >&2
+        if [ $config_mode = debug ]; then
+          cat >&2 <<EOF
+Warning: The PortMidi library contains code that may trigger address sanitizer
+in debug builds. These are probably not bugs in orca.
+EOF
         fi
       fi
-      if [[ $mouse_disabled = 1 ]]; then
+      if [ $mouse_disabled = 1 ]; then
         add cc_flags -DFEAT_NOMOUSE
       fi
       ;;
     *)
-      echo -e "Unknown build target '$1'\\nValid targets: orca, cli" >&2
+      printf 'Unknown build target %s\nValid build targets: %s\n' \
+        "$1" 'orca, cli' >&2
       exit 1
       ;;
   esac
   try_make_dir "$build_dir"
-  if [[ $config_mode = debug ]]; then
+  if [ $config_mode = debug ]; then
     build_dir=$build_dir/debug
     try_make_dir "$build_dir"
   fi
-  local out_path=$build_dir/$out_exe
-  # bash versions quirk: empty arrays might give error on expansion, use +
-  # trick to avoid expanding second operand
-  verbose_echo timed_stats "$cc_exe" "${cc_flags[@]}" -o "$out_path" "${source_files[@]}" ${libraries[@]+"${libraries[@]}"}
-  if [[ $stats_enabled = 1 ]]; then
-    echo "time: $last_time"
-    echo "size: $(file_size "$out_path")"
+  out_path=$build_dir/$out_exe
+  IFS='
+'
+  # shellcheck disable=SC2086
+  verbose_echo timed_stats "$cc_exe" $cc_flags -o "$out_path" $source_files $libraries
+  compile_ok=$?
+  if [ $stats_enabled = 1 ]; then
+    if [ -n "$last_time" ]; then
+      printf '%s\n' "time: $last_time"
+    else
+      printf '%s\n' "time: unavailable (missing 'time' command)"
+    fi
+    if [ $compile_ok = 0 ]; then
+      printf '%s\n' "size: $(file_size "$out_path")"
+    fi
   fi
 }
 
 print_info() {
-  local linker_name
-  if [[ $lld_detected = 1 ]]; then
+  if [ $lld_detected = 1 ]; then
     linker_name=LLD
+    # Not sure if we should always print the specific LLD name or not. Or never
+    # print it.
+    if [ "$lld_name" != lld ]; then
+      linker_name="$linker_name ($lld_name)"
+    fi
   else
     linker_name=default
   fi
@@ -480,36 +512,38 @@
 
 case $cmd in
   info)
-    if [[ "$#" -gt 1 ]]; then
+    if [ "$#" -gt 1 ]; then
       fatal "Too many arguments for 'info'"
     fi
     print_info; exit 0;;
   build)
-    if [[ "$#" -lt 1 ]]; then
+    if [ "$#" -lt 1 ]; then
       fatal "Too few arguments for 'build'"
     fi
-    if [[ "$#" -gt 1 ]]; then
-      echo "Too many arguments for 'build'" >&2
-      echo "The syntax has changed. Updated usage examples:" >&2
-      echo "./tool build --portmidi orca   (release)" >&2
-      echo "./tool build -d orca           (debug)" >&2
+    if [ "$#" -gt 1 ]; then
+      cat >&2 <<EOF
+Too many arguments for 'build'
+The syntax has changed. Updated usage examples:
+./tool build --portmidi orca   (release)
+./tool build -d orca           (debug)
+EOF
       exit 1
     fi
     build_target "$1"
     ;;
   clean)
-    if [[ -d "$build_dir" ]]; then
+    if [ -d "$build_dir" ]; then
       verbose_echo rm -rf "$build_dir"
     fi
     ;;
   help) print_usage; exit 0;;
-  -*)
-    echo "The syntax has changed for the 'tool' build script." >&2
-    echo "The options now need to come after the command name." >&2
-    echo "Do it like this instead:" >&2
-    echo "./tool build --portmidi orca" >&2
-    exit 1
-    ;;
+  -*) cat >&2 <<EOF
+The syntax has changed for the 'tool' build script.
+The options now need to come after the command name.
+Do it like this instead:
+./tool build --portmidi orca
+EOF
+    exit 1 ;;
   *) fatal "Unrecognized command $cmd";;
 esac
 
--