Code style & conventions
BOA is two languages with two different compatibility contracts. The bash
side (BARRACUDA/OCTOPUS chains, lib/functions/*.sh.inc, aegir/tools/)
targets the distro bash on supported Devuan/Debian — modern bash, not POSIX
sh. The PHP side (the omega8cc Provision/Hosting/Drush forks and the
hosting_* modules) has a hard PHP 5.6 floor and an 8.5 ceiling — the same
files must parse and behave identically across that whole span. Style here is
load-bearing: this code runs unattended on the whole fleet via SKYNET
self-update, so a convention violation is a fleet-wide incident, not a nit.
Bash
Layout and naming
- Shebang
#!/bin/bash. Helpers may use bash ≥ 4.1 features — e.g. the brace-expansion fd closeexec {_FD}>&-inaegir/tools/bin/lock.inc:41— so never downgrade a script to#!/bin/shsemantics. - Two-space indent, never tabs. The tree is tab-free; keep it that way.
- Functions:
_lowercase_with_underscores, leading underscore —_init_start,_clean_pid_exit,_single_instance_lock. - Variables: every BOA-defined variable carries a leading underscore.
Script/config scope is upper
_SNAKE_CASE(_AEGIR_ROOT,_DEBUG_MODE,_PHP_MULTI_INSTALL); function-locals are declared withlocaland are usually lower case (local _cmd="$*"—lib/functions/helper.sh.inc:118,local _path_found=""—BOA.sh.txt:798). The underscore is the namespace: anything without it is an environment or third-party variable. - Quote every expansion:
"${_VAR}", never$_VARor${_VAR}bare. Unquoted expansions are the single largest historical bug class in shell; ShellCheck SC2086 flags them and the few intentional splits in the tree are annotated as such (BOA.sh.txt:1091). - Conditionals: prefer
[[ ]]in new code — it gives regex matching ([[ "${_hName}" =~ ".aegir.cc"($) ]],lib/functions/helper.sh.inc:6) and no word-splitting surprises. Use(( ))for arithmetic ((( _CNT > 1 )),BARRACUDA.sh.txt:226). The tree still carries a large legacy[ ]body; when patching an existing block, match its local style — do not sweep-convert whole files. Style-only churn buries the functional diff and has no test coverage of its own. - Functions above main code; group long scripts into clearly named functions rather than nesting deep.
Output helpers — which helper is legal where
The main installer chain and lib/functions/ have exactly one emitter:
# lib/functions/helper.sh.inc:16
_msg() {
echo "BOA [$(date +%T)] ==> $*"
}
Severity and activity are message prefixes, not separate helpers. The set
in current use across lib/functions/ + the main scripts, by frequency:
INFO: (~490), PROC: (~370), WARN:, TUNE:, ERROR:, NOTE:, WAIT:,
DNLD:, EXIT:, ATTN:, OOPS:, HINT:, FATAL ERROR:, DEBUG:, PCKG:.
_msg "INFO: Checking for 'with-mysql=mysqlnd' in PHP 5.6"
_msg "OOPS: Failed to download https://${_USE_MIR}/core/$1 after ${_max_attempts} attempts"
_msg "FATAL ERROR: you must specify also _LOCAL_NETWORK_HN"
The fatal path in the chain is _msg "FATAL ERROR: …" followed by
_clean_pid_exit <reason-token> (BARRACUDA.sh.txt:216,
OCTOPUS.sh.txt:204), which appends the reason to
/var/log/boa/.barracuda.sh.exit.exceptions.log and removes the run pidfiles
before exiting.
Standalone tools in aegir/tools/bin/ define their own local helpers —
_info, _alrt, _die — because they run without the lib includes:
# aegir/tools/bin/renameaegirhost:80
_info() { echo "INFO: $*"; }
_alrt() { echo "ALRT: $*"; }
_die() { echo "ERROR: $*"; exit 1; }
The boundary is strict and verified: _alrt/_info/_die appear only in
standalone tools (renameaegirhost, clearwebbans, xmass) and never in
lib/functions/ or the main scripts. Do not introduce them there — in chain
code, emit via _msg "PREFIX: …" and exit via _clean_pid_exit. One
file-local exception to know about: BOA.sh.txt:790 shadows _msg with a
debug-gated variant ([virt-what-fix] prefix, prints only when
_DEBUG_MODE=YES) for its embedded virt-what repair block.
Error handling — no set -e
set -e appears nowhere in the main scripts or lib/functions/. That is
deliberate, not an omission: the installer chains are full of
expected-failure branches (probe commands, optional downloads, idempotent
re-runs), and set -e converts those into silent aborts halfway through a
system upgrade. Check exit codes explicitly:
if ! flock -n "${_LOCK_FD}"; then # aegir/tools/bin/lock.inc:74
echo "${_SELF_NAME}: another instance is running; exiting."
exit 0
fi
Failure that must stop the run goes through _clean_pid_exit <token> (chain)
or _die (standalone tool) so the exit is logged and the pidfiles are
released. Never let a fatal path fall off the end of a function with the
pidfiles still in place.
Re-entrancy guards
Two patterns, by context:
- The installer chain is serialised by
/run/boa_run.pid+/run/boa_wait.pid._init_startinaegir/tools/bin/boa:1283refuses to start when either pidfile exists andtouches both before proceeding (boa:1297-1298); thebarracudaandoctopuswrappers inline the same gate (aegir/tools/bin/barracuda:1155,aegir/tools/bin/octopus:694)._clean_pid_exitremoves them on every exit path. Monitor-side code treats their presence as "system busy" (_check_uptime_grace_period,aegir/tools/bin/lock.inc:99). - Standalone tools and monitors source the shared lock include and call
_single_instance_lock [lockfile] [fd](aegir/tools/bin/lock.inc:53): flock on an auto-assigned fd with an atomicmkdirfallback where flock is missing, a PID note in the lockfile, and an EXIT/INT/TERM/HUP trap that releases via the TOCTOU-safe_single_instance_unlock. The include is idempotent (_SINGLE_INSTANCE_LIB_VERguard,lock.inc:16). New tools use this — do not hand-rollpgrep -ccounting.
Comments and inline help
Terse and factual; explain why, not what. ### blocks are section
banners and file headers only — no decorative art. In standalone tools the
### header block is the help text: -h|--help prints it verbatim via
sed -n '/^###/{ s/^### \?//; p }' "$0" # aegir/tools/bin/renameaegirhost:55
so the header must stay accurate — usage line, every flag, and the
assumptions the caller must satisfy (see renameaegirhost:3-26 for the
canonical shape). A stale header is a user-facing bug, not a cosmetic one.
Untrusted values — mandatory guard helpers
Bash that passes a caller-supplied or data-derived value to chmod/chown,
a path operation, or a SQL identifier routes through the existing guards,
never raw interpolation:
| Helper | Contract | Definition |
|---|---|---|
_is_safe_ident |
[A-Za-z0-9_]+ allowlist for DB/table names before they reach a MySQL query; failures are skipped, not executed |
aegir/tools/system/mysql_cleanup.sh:113 (same helper in mysql_backup.sh, mysql_cluster_backup.sh) |
_validate_path_prefix |
resolve the caller-supplied root and require it under the allowed Aegir roots; anything else exits non-zero | aegir/tools/bin/fix-drupal-platform-permissions.sh:31 |
_chmod_safe |
per-target symlink precheck so a planted symlink can never redirect a chmod outside the validated tree |
fix-drupal-platform-permissions.sh:47 |
These scripts run via NOPASSWD sudo from the aegir and per-Octopus users —
exactly why the raw versions are exploitable. New code touching those
surfaces is held to the same standard in review.
Bash lint gate
Before submitting:
bash -non every touched.sh/.sh.inc/.sh.txtfile — parse gate.- ShellCheck clean, or annotated. The tree's convention for intentional violations is a targeted disable with the reason on the same line:
# shellcheck disable=SC2154 # _pthLog/_tRee/_xSrl are globals provided by BOA.sh.txt
# shellcheck disable=SC2086,SC2154 # intentional _crlGet flag-string split; _crlGet is a BOA.sh.txt global
(BOA.sh.txt:1071, BOA.sh.txt:1091; same pattern in
aegir/tools/system/minute.sh:12, runner.sh:100, mysql_cleanup.sh:66.)
A bare disable with no reason does not pass review.
PHP — the hard 5.6 floor
Everything the Aegir backend CLI parses — *Provision, Hosting, the Drush 8 fork, every `hosting_` module — must stay compatible with PHP 5.6**. This is not folklore; it is asserted in the code and provisioned by the installer:
drush.php:31— the deprecation-mask block is documented "Safe on PHP 5.6-8.5";DRUSH_MINIMUM_PHPis5.4.5(includes/preflight.inc:168) and enforced withversion_compare()atpreflight.inc:170.- BOA still builds and ships PHP 5.6 as a first-class runtime:
_PHP_V="5.6 7.0 7.1 7.2 7.3 7.4 8.0 8.1 8.2 8.3 8.4 8.5"(lib/functions/php.sh.inc:2846),/opt/php56/bin/phpprobes (php.sh.inc:3343), and5.6is a documented_PHP_MULTI_INSTALLvalue (lib/settings/barracuda.sh.cnf:67).
The same backend files run under a 5.6 CLI on legacy boxes and an 8.x CLI on current ones. Compatibility is therefore a span, 5.6 through 8.5 — not just a floor. Code must parse on both ends and produce the same values on both ends.
Forbidden in backend PHP (parse/fatal error on 5.6)
| Construct | First available |
|---|---|
Scalar/return type declarations (string $x, : void, : int) |
7.0 |
Null coalescing ?? |
7.0 (??= 7.4) |
Spaceship <=> |
7.0 |
| Anonymous classes | 7.0 |
| Trailing comma in call arguments | 7.3 (in parameter lists: 8.0) |
Arrow functions fn() |
7.4 |
| Typed properties | 7.4 |
match expressions |
8.0 |
Nullsafe ?-> |
8.0 |
| Named arguments | 8.0 |
| Constructor property promotion | 8.0 |
str_contains / str_starts_with / str_ends_with (stdlib) |
8.0 — polyfill or strpos() idioms only |
array_key_first / array_key_last (stdlib) |
7.3 |
Safe because 5.6 already has them: short arrays [] and traits (5.4),
generators, finally and ::class (5.5), variadics ... and argument
unpacking (5.6).
The typing ban is structural, not stylistic. The backend lives in a
deliberately de-typed Drush 8 world: the D10/D11 platform patch layer
strips : void return types from vendored core/Symfony files so the Aegir
Drush 8 fork can load them, and Provision detects patch state by searching
for the ': void' string (provision.inc:179; rationale in the
platform/install.provision.inc:145 comment block). New backend code that
reintroduces type declarations breaks the very world the patch layer
maintains.
Runtime divergence — lint proves parse, nothing more
The dangerous failure mode is not the parse error (that one is loud). It is the file that lints clean everywhere and silently computes different values on 5.6 vs 8.x:
==string↔number juggling changed in 8.0:"foo" == 0isTRUEon 5.6/7.x andFALSEon 8.0+. Code on either side of that must not care — and conversely, a blanket "modernise to===" sweep changes 5.6 behaviour and is banned. Change comparison semantics only per-site, with a stated reason.parse_url()edge cases (scheme-less inputs, odd ports/userinfo) return different components across versions — reason about credential/DSN parsing paths on both ends, don't assume.sort()-family stability is guaranteed only since 8.0; equal-key ordering may differ on 5.6.count()onNULL/scalars: quiet on 5.6, warning on 7.2,TypeErroron 8.0.is_numeric('0x1A')isTRUEon 5.6,FALSEon 7.0+.- Curly-brace string offsets
$s{0}are the reverse trap: valid 5.6–7.3, fatal on 8.0.
Rule of thumb: anything that parses config, DSNs, credentials, or URLs gets reasoned through (or tested) under both a 5.6 and an 8.x CLI before it ships, not just linted.
Drupal style, hooks, shell escaping
- Follow Drupal (D7-era) coding standards for the forks and
hosting_*modules: two-space indent, no tabs,hook_provision_*/hook_hosting_*naming,/** … */docblocks. - Log via
drush_log()/drush_set_error(), neverecho— task-queue output capture only sees the Drush log. - Shell execution contract: callers pre-escape every interpolated value
with
escapeshellarg()before the command string is built. The transit layer (Provision_Context_server::shell_exec(),Provision/Context/server.php) is defence-in-depth only: it fails closed on embedded\n/\r/\0(strpbrk,server.php:165) and wraps the whole command inescapeshellcmd()on both the local and SSH branches (server.php:170,:173) — it cannot repair an argument the caller left unescaped. New code that builds a command from external input is reviewed against this contract.
PHP lint gate
php -levery touched backend file with a real 5.6 binary — on any BOA box with5.6in_PHP_MULTI_INSTALL, that is/opt/php56/bin/php -l.- A modern
php -lpass is necessary but not sufficient: 7.x/8.x parsers accept??, arrow functions, typed properties,match,<=>— all of which fatal on 5.6. Only a 5.6 binary proves the floor. - Lint the ceiling too (8.5): the span cuts both ways —
$s{0}offsets and removed stdlib behaviours fatal on the top end while parsing fine on 5.6.
Commit style
- Subject: imperative mood, ≤ 72 characters — reads like an instruction, not a description. Match the shape of the existing log, e.g. "Fold Solr indices into the migration disk-space gate" or "Align shared tool serials across the update agents" — a verb, an object, the scope, no prefix.
- Body: explain why when the subject is not self-explanatory; omit it when it is.
- No Conventional Commits prefixes (
feat:/fix:), no emoji, no co-author or tooling trailers. - One concern per commit. A behaviour change and an unrelated cleanup are two commits.
Pre-submission checklist
-
bash -nclean on every touched shell file. - ShellCheck clean, or annotated with a reasoned
# shellcheck disable=…on the same line. - No tabs, two-space indent, all expansions quoted.
- Chain code emits via
_msg "PREFIX: …"only; no_alrt/_info/_dieoutside standalone tools; fatals go through_clean_pid_exit. - Long-running or cron-invoked scripts hold a lock
(
_single_instance_lockor theboa_run.pidgate). - Backend PHP:
php -lunder a real 5.6 binary and a current 8.x binary; no forbidden syntax; parsing paths reasoned about on both ends. - No blanket
===/modernisation sweeps in backend PHP. - Untrusted values routed through the guard helpers /
escapeshellarg()at the caller. - Commit subject imperative, ≤ 72 chars, one concern per commit.
Related
- Developing BOA — guide index — the other maintainer topics, including the release model the SKYNET serial mechanics live under.
- Reference appendix — consolidated
_VARand command tables the bash conventions feed into.