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 close exec {_FD}>&- in aegir/tools/bin/lock.inc:41 — so never downgrade a script to #!/bin/sh semantics.
  • 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 with local and 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 $_VAR or ${_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_start in aegir/tools/bin/boa:1283 refuses to start when either pidfile exists and touches both before proceeding (boa:1297-1298); the barracuda and octopus wrappers inline the same gate (aegir/tools/bin/barracuda:1155, aegir/tools/bin/octopus:694). _clean_pid_exit removes 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 atomic mkdir fallback 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_VER guard, lock.inc:16). New tools use this — do not hand-roll pgrep -c counting.

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:

  1. bash -n on every touched .sh / .sh.inc / .sh.txt file — parse gate.
  2. 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_PHP is 5.4.5 (includes/preflight.inc:168) and enforced with version_compare() at preflight.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/php probes (php.sh.inc:3343), and 5.6 is a documented _PHP_MULTI_INSTALL value (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" == 0 is TRUE on 5.6/7.x and FALSE on 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() on NULL/scalars: quiet on 5.6, warning on 7.2, TypeError on 8.0.
  • is_numeric('0x1A') is TRUE on 5.6, FALSE on 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(), never echo — 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 in escapeshellcmd() 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 -l every touched backend file with a real 5.6 binary — on any BOA box with 5.6 in _PHP_MULTI_INSTALL, that is /opt/php56/bin/php -l.
  • A modern php -l pass 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 -n clean 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/_die outside standalone tools; fatals go through _clean_pid_exit.
  • Long-running or cron-invoked scripts hold a lock (_single_instance_lock or the boa_run.pid gate).
  • Backend PHP: php -l under 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