From 6ac7b25a4c1aacfde41331bba651ab9bad9eb88e Mon Sep 17 00:00:00 2001 From: Vratko Polak Date: Tue, 19 Feb 2019 15:01:34 +0100 Subject: Bash style: Add rules for handling working dir + Also added details related to "set -e" and exit codes. Change-Id: I731151efd7facc1def3c8cbba4e466e793c93689 Signed-off-by: Vratko Polak --- docs/bash_code_style.rst | 94 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/docs/bash_code_style.rst b/docs/bash_code_style.rst index 5a73d7b736..e5dbc9c06e 100644 --- a/docs/bash_code_style.rst +++ b/docs/bash_code_style.rst @@ -120,6 +120,10 @@ Safety When was the last time you checked error code of "echo" command, for example? + + Another example: "set -e" in your function has no effect + if any ancestor call is done with logical or, + for example in "func || code=$?" construct. + + As there is no reliable method of error detection, and there are two largely independent unreliable methods, the best what we can do is to apply both. So, code SHOULD explicitly @@ -162,9 +166,27 @@ Bash options + Code MUST to store "$?" and call "set -e" immediatelly afterwards. + + Code MUST NOT use this approach when calling functions. + + + That is because functions are instructed to apply "set -e" on their own + which (when triggered) will exit the whole entry script. + + + Unless overriden by ERR trap. + But code SHOULD NOT set any ERR trap. + + + If code needs exit code of a function, it is RECOMMENDED to use + pattern 'code="0"; called_function || code="${?}"'. + + + In this case, contributor MUST make sure nothing in the + called_function sub-graph relies on "set -e" behavior, + because the call being part of "or construct" disables it. + + Code MAY append "|| true" for benign commands, when it is clear non-zero exit codes make no difference. + + Also in this case, the contributor MUST make sure nothing within + the called sub-graph depends on "set -e", as it is disabled. + + Code MUST apply "-u" as unset variable is generally a typo, thus an error. + Code MAY temporarily apply "+u" if a command needs that to pass. @@ -377,6 +399,78 @@ Arguments or variables + For most Linux distros, "getopt" is RECOMMENDED. +Working directory handling +~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++ Functions SHOULD act correctly without neither assuming + what the currect working directory is, nor changing it. + + + That is why global variables and arguments SHOULD contain + (normalized) full paths. + + + Motivation: Different call sites MAY rely on different working directories. + ++ A function MAY return (also with nonzero exit code) when working directory + is changed. + + + In this case the function documentation MUST clearly state where (and when) + is the working directory changed. + + + Exception: Functions with undocumented exit code. + + + Those functions MUST return nonzero code only on "set -e" or "die". + + + Note that both "set -e" and "die" by default result in exit of the whole + entry script, but the caller MAY have altered that behavior + (by registering ERR trap, or redefining die function). + + + Any callers which use "set +e" or "|| true" MUST make sure + their (and their caller ancestors') assumption on working directory + are not affected. + + + Such callers SHOULD do that by restoring the original working directory + either in their code, + + + or contributors SHOULD do such restoration in the function code, + (see below) if that is more convenient. + + + Motivation: Callers MAY rely on this side effect to simplify their logic. + ++ A function MAY assume a particular directory is already set + as the working directory (to save space). + + + In this case function documentation MUST clearly state what the assumed + working directory is. + + + Motivation: Callers MAY call several functions with common + directory of interest. + + + Example: Several dowload actions to execute in sequence, + implemented as functions assuming ${DOWNLOAD_DIR} + is the working directory. + ++ A function MAY change the working directory transiently, + before restoring it back before return. + + + Such functions SHOULD use command "pushd" to change the working directory. + + + Such functions SHOULD use "trap 'trap - RETURN; popd' RETURN" + imediately after the pushd. + + + In that case, the "trap - RETURN" part MUST be included, + to restore any trap set by ancestor. + + + Functions MAY call "trap - RETURN; popd" exlicitly. + + + Such functions MUST NOT call another pushd (before an explicit popd), + as traps do not stack within a function. + ++ If entry scripts also use traps to restore working directory (or other state), + they SHOULD use EXIT traps instead. + + + That is because "exit" command, as well as the default behavior + of "die" or "set -e" cause direct exit (without skipping function returns). + Function size ~~~~~~~~~~~~~ -- cgit 1.2.3-korg