savannah-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Savannah-cvs] [696] add fsf bash style guide


From: iank
Subject: [Savannah-cvs] [696] add fsf bash style guide
Date: Tue, 12 Dec 2023 14:39:26 -0500 (EST)

Revision: 696
          
http://svn.savannah.gnu.org/viewvc/?view=rev&root=administration&revision=696
Author:   iank
Date:     2023-12-12 14:39:24 -0500 (Tue, 12 Dec 2023)
Log Message:
-----------
add fsf bash style guide

Added Paths:
-----------
    trunk/sviki/fsf/bash-style-guide.mdwn

Added: trunk/sviki/fsf/bash-style-guide.mdwn
===================================================================
--- trunk/sviki/fsf/bash-style-guide.mdwn                               (rev 0)
+++ trunk/sviki/fsf/bash-style-guide.mdwn       2023-12-12 19:39:24 UTC (rev 
696)
@@ -0,0 +1,457 @@
+# FSF Tech Team Bash style guide
+
+This is a work in progress. Contributions are welcome. This is a general
+style guide, but focused on FSF tech team and the volunteers we work
+with administering about 130 GNU/Linux systems.
+
+Please donate at https://www.fsf.org/ to help advance computer user
+freedom.
+
+# About other documentation
+
+Useful:
+
+* Man bash(1)
+
+* https://mywiki.wooledge.org/EnglishFrontPage
+
+* Stackoverflow is good, but often has answers that are not robust, and
+posting to the site may require running javascript.
+For a common example, see https://mywiki.wooledge.org/ParsingLs
+
+
+About "portability": If you read something online that says, you should
+do it this way because is "more portable", you often want the exact
+opposite. It often means that bash or gnu coreutils can do it in a more
+readable or otherwise better way. We all deserve to use systems where we
+can install and use GNU software, don't hamstring yourself for the sake
+of catering to less capable systems unless you really have some good
+reason to do so. It is usually easier to wait and port your software if
+you need it.
+
+
+# Things mostly relevant to sharing
+
+Always use bash, never /bin/sh, etc.
+
+Use common sense and be consistent. If you change script style, and it
+is in a git repo, try to make a commit which only changes the style.
+
+Scripts must be executable, preferably with no extension, but .sh is
+okay. lowercase-dash-separated. Scripts meant to be sourced (libraries)
+are more okay to have .sh extention. This is common practice scripts in
+/usr/bin, don't have .sh, it's extraneous overhead to the person using
+it.
+
+If a script is getting published, follow
+https://www.gnu.org/licenses/license-recommendations.en.html .
+
+# Shellcheck
+
+The shebang should not have arguments so bash script-name works the
+same. Use the set command instead.
+
+
+Before using any script, it should have no output from
+
+```
+shellcheck -x -e 2046,2068,2086,2206,2029,2033,2054,2164,2254 SCRIPT_FILE
+```
+
+The numbers are exceptions from the rules. The reasons for the exceptions:
+
+The following exceptions are for quoting variables. Shellcheck complains
+that unquoted variables will do splitting and globbing. You should
+simply know this and account for this it in your code. Most variables
+are things we know don't have spaces and it would be too verbose to
+quote them all the time. If the variable could have characters like
+space, *, ?, [, then definitely quote it unless you want globing. If it
+the variable is the program's args, always quote it, because the person
+running it can easily make errors. For scripts which are expected to
+take untrusted input, or process files or variables which could have
+spaces, run shellcheck with no exceptions to verify quoting.
+
+  # 2046 = unquoted $(cmd)
+  # 2068 = Double quote array expansions to avoid re-splitting elements.
+  # 2086 = unquoted $var
+  # 2206 = unquoted $var in array=($var)
+  # 2254 = unquoted $var in case x in $var)
+
+Miscellaneous exceptions:
+
+  # 2029 = Using unescaped variable in ssh call. Reason: This never
+  works, warning is generally a false positive.
+  # 2032 = Warning about calling shell function over ssh. There are ways
+  for this to be a false positive, and we generally don't need a warning
+  that shellcheck doesn't know if a command exists.
+  # 2033 = shell function as argument to another shell. Reason: This never
+  works, warning is generally a false positive.
+  # 2054 = Array declaration contains a comma. Reason: This is warning of a
+  mistake based on how other languages work, but this would never work
+  as expected in bash and is a legitimate thing to do.
+  # 2164 = cd without condition for failure. Reason: We use automatic error
+  handling.
+
+This is based on shellcheck version: 0.8.0 from trisquel 11. A newer
+shellcheck probably has new rules worth ignoring.
+
+If there is a need for something shellcheck warns about, use a directive
+to override the warning, eg:
+
+```
+# shellcheck disable=SC2035 # justification
+```
+
+If the shellcheck you are using is too old for -x, get a new one using
+package pinning or haskell stack.
+
+
+
+For shellcheck SC2207/2206, how to avoid x=( $(cmd) ) and y=( $var ) since
+it unnecessarily expands globs:
+
+For splitting a variable on spaces without glob expansion:
+
+```
+tmpstr=$(cmd)
+IFS=" " read -r -a array <<<"$tmpstr"
+```
+
+And for splitting on lines:
+
+```
+var=$(cmd)
+mapfile -t array <<<"$var"
+```
+
+Add comments describing functions that are not short and
+obvious. Document any arguments, global variables used, return value
+that is different from the default of the exit code of the last command
+run. See the err script in this repo for example.
+
+Comment any tricky or important parts of code.
+
+Use TODO comments
+```
+# TODO iank: add x functionality
+```
+
+Indent 2 spaces.
+
+Try to wrap lines longer than ~80 chars. Use \ followed by newline where 
appropriate.
+
+Use here documents to print multiple lines instead multiple echo lines.
+
+cat <<END
+I have lots
+of stuff to say.
+END
+
+
+Put then and do on the same line as the condition:
+```
+if x; then
+  :
+fi
+for x; do
+  :
+done
+```
+
+To work from the same directory of the script
+```
+readonly this_file="$(readlink -f -- "${BASH_SOURCE[0]}")"; cd ${this_file%/*}
+```
+
+Variables:
+
+Environment variables are all caps. Other vars are lowercase. Underscore
+separates words. Declare constants and environment variables at the top
+of the file when possible.
+
+```
+# Constant
+readonly conf_dir=/some/path
+
+# Constant and environment
+declare -xr conf_dir=/some/path
+
+# A constant set based on condition needs to be set readonly after it's set.
+verbose=false
+if [[ $1 ]]; then
+  verbose=true
+fi
+readonly verbose
+```
+
+Use local to make function local variables enforced. This adds
+readability and avoids error from using variable in an unintended
+scope. eg: `local y=5`
+
+
+Never quote literal integers.
+
+Prefer single quoting strings that have no subsitution (things that
+start with $).
+
+Non command options or path names can be optionally quoted. eg `x='oo'`
+
+
+
+
+Functions:
+
+Put all functions together near the top of the file after includes,
+constants, and set statements.
+
+
+Always use `return 0`, never `return`, because you can return nonzero
+by accident otherwise.
+
+
+# Error handling
+
+You should pick one of the following options (each subheading is an
+option) and use it consistently throughout your script.
+
+## Automatically exit on failed command:
+
+For scripts with no functions add:
+set -eE -o pipefail
+trap 'echo "$0:$LINENO:error: \"$BASH_COMMAND\" returned $?" >&2' ERR
+
+For scripts with functions, copy/paste or source the bash-bear script
+from , which will exit on error and print stack traces.
+
+```
+set -e; . /path/to/bash-bear; set +e
+test-func() {
+  result=true
+  echo do stuff
+  result=false
+}
+test-funcc
+$result || echo handling false result
+
+echo this prints too
+```
+
+To allow a specific command to to fail, make it part of a conditional. eg:
+iptables -D rule || [[ $? == 1 ]] # allow exit code 1 when rule doesn't exist
+
+### Gotchas in automatic error handling:
+
+### Functions in conditionals
+
+Don't put functions in conditionals generally because the trap is
+ignored in that function call. Instead, use a result variable. However,
+it ok for very short functions when only the last line can fail. eg:
+
+```
+afunc() { echo ok; "$@"; }
+```
+### Process Substitution
+
+Don't use process substitution when the command inside could fail,
+because the trap won't catch it. Eg, do NOT do this:
+
+```
+while read -r l; do something; done < <(command-that-could-fail)
+```
+
+Instead, do this
+tmp=$(mktemp)
+command-that-could-fail >$tmp
+while read -r l; do something; done <$tmp
+
+### Command substitution
+
+This won't be caught by error trap:
+
+```
+echo hello $(command-that-could-fail)
+```
+
+Instead, do this
+
+```
+var=$(command-that-could-fail)
+echo hello "$var"
+```
+
+### Arithmetic expressions
+
+If there is a syntax error in the expression in a condional `((
+expression ))`, it returns 1it does not cause the program to exit. That
+is bad and a source of bugs.
+
+```
+# Avoid this
+x=/s
+if (( x )); then
+  do something
+fi
+
+# Instead, pick one of the following:
+
+# option a:
+x=/s
+# This will fail due to syntax error.
+tmp=$(( x ))
+if (( tmp )); then
+  do something
+fi
+
+# option b:
+x=/s
+if (( x )); then
+  do something
+else
+  echo some error happened >&2
+  exit 1
+fi
+
+``
+
+
+## Manual error checking
+
+Explicit handling should
+be done for almost all commands not builtin to bash.
+
+```
+set -E -o pipefail
+trap 'echo "$0:$LINENO:error: \"$BASH_COMMAND\" returned $?" >&2' ERR
+```
+
+Or for scripts with functions, as above,
+
+```
+source err
+err-print
+```
+
+Manual error handling example:
+
+```
+iptables -D $rule
+if [[ $? != [01] ]]; then
+  err-exit exiting due to failed iptables
+fi
+```
+
+3. If doing a best effort and not exiting on errors, exit code should be
+the higest error.
+
+```
+error=0; trap 'error=$(($?>$error?$?:$error))' ERR
+# intentionally ignoring errors to do best effort
+command1
+command2
+command3
+exit $error
+```
+
+Additional error handling notes:
+
+In some circumstances pipefail may be too blunt. In that case, check
+`${PIPESTATUS[@]}`.
+
+
+Misc:
+
+Prefer the use of builtins such as the Parameter Expansion functions in
+bash(1) as they are more robust.
+
+```
+# Prefer this:
+addition=$((x + y))
+substitution="${string/#foo/bar}"
+
+# Instead of this:
+addition="$(expr $x + $y)"
+substitution="$(echo "${string}" | sed -e 's/^foo/bar/')"
+```
+
+
+Avoid eval.
+
+Avoid $_. It is easy to alter the environment so it is broken. So far
+seen this happen in interactive shell only.
+
+When a var might have spaces, double quote it.
+
+When input is generally untrusted, protect against input that starts
+with dash.
+
+```
+rm -v -- "$var"
+rm -v ./* # a file could be named -rf
+```
+
+Avoid set -u. empty vars are useful, check for them with [[ $var ]]. set
+-u just leads to explicitly setting variables to empty and other hackery
+to avoid unset errors, making it complication that is not worth
+it. shellcheck will find completely unset variables.
+
+Prefer $var except when ${var} is necessary or adds clarity, like 
"comb${x}ining strings"
+
+Never use [. [[ is a special syntax which doesn't do variable expansion
+and avoids edge cases.
+
+```
+[[ $filename == *.png ]] # righthand globs over lefthand. filename has png 
extension
+# Quote only the righthand side when globbing is not desired.
+[[ $filename == "*.png" ]] # filename is literal string "*.png"
+```
+Use double equal for string equality [[ $x == 2 ]]
+
+
+
+Use gnu getopts whenever there is more than one option, or an option and
+an argument. Reference info at:
+/usr/share/doc/util-linux/examples/getopt-parse.bash
+```
+usage() {
+    cat <<EOF
+Usage: ${0##*/} [OPTIONS] ARG1
+One line description
+
+
+-h|--help  Print help and exit.
+
+Note: Uses GNU getopt options parsing style
+EOF
+    exit $1
+}
+
+##### begin command line parsing ########
+
+bool_opt=false # default
+long_opt=foo # default
+temp=$(getopt -l help,long-opt: hso: "$@") || usage 1
+eval set -- "$temp"
+while true; do
+    case $1 in
+        -s) bool_opt=true; shift ;;
+        -o|--long-opt) long_opt="$2"; shift 2 ;;
+        -h|--help) usage ;;
+        --) shift; break ;;
+        *) echo "$0: Internal error! unexpected args: $*" ; exit 1 ;;
+    esac
+done
+read arg1 arg2 <<<"$@"
+
+##### end command line parsing ########
+```
+
+# License
+
+Copying and distribution of this file, with or without modification,
+are permitted in any medium without royalty provided the copyright
+notice and this notice are preserved.  This file is offered as-is,
+without any warranty.
+
+This GNU all-permissive license applies to all the files in this
+directory. Note:
+https://www.gnu.org/licenses/license-recommendations.en.html




reply via email to

[Prev in Thread] Current Thread [Next in Thread]