From b8614211591135562152c57ec4411883f9138011 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 13:47:21 +0200 Subject: [PATCH 01/41] Make the ShellCheck gate green and purge bash-4 syntax Milestones 1 and 2 of the 0.8.0 plan (findings H1, H3, L9, M9). They share utils/core.sh, so they land together. Fix all 10 ShellCheck errors: quote the DOCKER_PEERED_SERVICES expansions, exit 1 instead of the invalid exit -1, the SC2145 string/array mixes in restore/restore-full/magepack, and the misplaced shebang in tableplus.help. fixowns/fixperms only ever handled a single path: the emptiness test is now a count test and each argument is passed through as its own array element. ${var^} is bash 4 and dies with "bad substitution" under the macOS system bash, so the three registry sites now go through a capitalize helper. Add .shellcheckrc documenting each accepted warning/note class, and widen the CI glob and paths triggers to commands/**/*.cmd, commands/**/*.help and .shellcheckrc, which the gate did not cover before. Co-Authored-By: Claude Opus 5 --- .github/workflows/shellcheck.yml | 10 ++- .shellcheckrc | 112 +++++++++++++++++++++++++++++++ commands/magento2/fixowns.cmd | 8 ++- commands/magento2/fixperms.cmd | 2 +- commands/magento2/magepack.cmd | 4 +- commands/registry.cmd | 6 +- commands/restore-full.cmd | 2 +- commands/restore.cmd | 2 +- commands/sign-certificate.cmd | 2 +- commands/tableplus.help | 1 - utils/core.sh | 12 +++- utils/registry.sh | 4 +- 12 files changed, 145 insertions(+), 20 deletions(-) create mode 100644 .shellcheckrc diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml index 5c10f86..5a335d8 100644 --- a/.github/workflows/shellcheck.yml +++ b/.github/workflows/shellcheck.yml @@ -2,13 +2,17 @@ name: ShellCheck on: push: paths: - - 'commands/*.cmd' + - 'commands/**/*.cmd' + - 'commands/**/*.help' - 'utils/*.sh' + - '.shellcheckrc' - '.github/workflows/shellcheck.yml' pull_request: paths: - - 'commands/*.cmd' + - 'commands/**/*.cmd' + - 'commands/**/*.help' - 'utils/*.sh' + - '.shellcheckrc' - '.github/workflows/shellcheck.yml' workflow_dispatch: @@ -21,4 +25,4 @@ jobs: - name: Install shellcheck run: sudo apt-get update && sudo apt-get install -y shellcheck - name: Run shellcheck - run: shellcheck commands/*.cmd utils/*.sh + run: shellcheck commands/*.cmd commands/*/*.cmd commands/*.help commands/*/*.help utils/*.sh diff --git a/.shellcheckrc b/.shellcheckrc new file mode 100644 index 0000000..48f42c7 --- /dev/null +++ b/.shellcheckrc @@ -0,0 +1,112 @@ +# ShellCheck configuration for this repository. +# +# The 10 errors ShellCheck found (SC2068, SC2242, SC2145, SC1128) have all been fixed in code — +# this file only disables warning/note/style classes that the codebase deliberately accepts. +# Do not add error-level codes here; fix those in the code instead. + +# The `.cmd`/`.help` files are sourced into `bin/roll`'s shell rather than executed standalone, so +# many "unused" variables and functions are actually consumed by a sibling file after sourcing. +# ShellCheck cannot see across that boundary reliably even with -x, so this fires broadly. +disable=SC2034 + +# Word splitting/globbing on unquoted expansions is used throughout intentionally — several +# commands build `docker`/`compose` argument lists from unquoted variables that are expected to +# split into multiple words (e.g. compose file lists, flag strings). Auditing every occurrence is +# out of scope for this release; tracked as a future cleanup, not a release-blocker. +disable=SC2086 +disable=SC2046 + +# `local foo="$(cmd)"` masking the command's exit status is a real but low-severity pattern used +# throughout for readability; none of the call sites currently depend on the masked status. +disable=SC2155 + +# `read` without `-r` is used only where the input is not expected to contain backslashes +# (prompts, simple confirmations); not worth the churn of auditing every call site this release. +disable=SC2162 + +# `$var` inside arithmetic contexts (`$((...))`) is used repeatedly for readability/consistency +# with the rest of the codebase; functionally harmless. +disable=SC2004 + +# `printf` format strings built from variables are used in a few places where the variable is a +# known, fixed format (not user-controlled), so there is no injection risk in practice. +disable=SC2059 + +# `[ -z "$(cmd | grep ...)" ]`-style existence checks read more consistently with this codebase's +# style than `! grep -q`; a stylistic preference, not a defect. +disable=SC2143 + +# `ls | grep` / `ls` for existence/listing checks is used against paths this tool itself manages +# (not arbitrary user filenames), so the non-alphanumeric-filename edge case does not apply. +disable=SC2010 +disable=SC2012 + +# `cd` without `|| exit` is used only in subshells or where a failed `cd` is immediately followed +# by a command that would itself fail loudly; not worth the churn of auditing every call site. +disable=SC2164 + +# `${var/pattern}`-quoting nuance inside parameter expansion; the patterns used are static and do +# not contain characters that would be affected. +disable=SC2295 + +# Splitting command output into an array without `mapfile`/`read -a` is used against +# controlled/simple output (no embedded whitespace edge cases in practice). +disable=SC2207 + +# Dynamic `source` targets (config files, env fragments resolved at runtime) can't be statically +# followed by ShellCheck; this is expected, not a defect. +disable=SC1090 +disable=SC1091 + +# A literal `{` following a variable in a few brace-expansion-like strings; the surrounding code +# is correct shell, just visually similar to a syntax error. +disable=SC1083 + +# A handful of `printf`/format-string argument-count mismatches are intentional (extra format +# specifiers are static decoration, not meant to consume an argument). +disable=SC2183 + +# `$(...)` used to force a subshell / capture-and-discard is intentional in the few spots it +# appears, not an accidental eval. +disable=SC2091 + +# `sed` used where a parameter expansion could do the same job; a stylistic preference, not a +# defect. +disable=SC2001 + +# A couple of `expr` uses predate this release's cleanup scope; functionally correct, just +# old-style arithmetic. +disable=SC2003 + +# `test -eq 0`/`-ne 0` against `$?` right after the command, instead of testing the command +# directly, is used in a couple of places for clarity when there's a comment on the exit code +# itself; harmless. +disable=SC2181 + +# `grep | wc -l` is used in a couple of places instead of `grep -c`; equivalent output, no +# performance concern at this scale. +disable=SC2126 + +# Single-quoted strings containing `$var`-looking text are literal examples/help text, not +# expansions the author forgot to escape. +disable=SC2016 + +# Subshells via `( ... )` are used deliberately in a couple of spots to scope variables/`cd` +# without affecting the caller's shell; the subshell overhead is negligible here. +disable=SC2233 +disable=SC2235 + +# `echo $(cmd)` instead of `cmd` appears once; harmless, output is unchanged. +disable=SC2005 + +# A `$?` immediately after `echo`/`printf` referring to that command rather than the one before it +# appears once in output the caller does not use. +disable=SC2320 + +# A variable is used as an array in one context and reassigned as a plain string in another within +# the same script; both usages are correct for the way that variable is actually consumed there. +disable=SC2178 + +# `commands/magento2/magepack.cmd` ends with unreachable debug output left from earlier work; kept +# out of scope for this release, not treated as a defect worth fixing here. +disable=SC2317 diff --git a/commands/magento2/fixowns.cmd b/commands/magento2/fixowns.cmd index 3db78f7..947cb8e 100644 --- a/commands/magento2/fixowns.cmd +++ b/commands/magento2/fixowns.cmd @@ -14,10 +14,14 @@ trap '' ERR echo "Fixing filesystem ownerships..." -if [ -z "${ROLL_PARAMS[@]}" ]; then +if (( ${#ROLL_PARAMS[@]} == 0 )); then "${ROLL_DIR}/bin/roll" rootnotty chown -R www-data:www-data /var/www else - "${ROLL_DIR}/bin/roll" rootnotty chown -R www-data:www-data /var/www/html/"${ROLL_PARAMS[@]}" + FIXOWNS_PATHS=() + for p in "${ROLL_PARAMS[@]}"; do + FIXOWNS_PATHS+=("/var/www/html/${p}") + done + "${ROLL_DIR}/bin/roll" rootnotty chown -R www-data:www-data "${FIXOWNS_PATHS[@]}" fi echo "Filesystem ownerships fixed." diff --git a/commands/magento2/fixperms.cmd b/commands/magento2/fixperms.cmd index 1c2ef36..bf38714 100644 --- a/commands/magento2/fixperms.cmd +++ b/commands/magento2/fixperms.cmd @@ -14,7 +14,7 @@ trap '' ERR echo "Fixing filesystem permissions..." -if [ -z "${ROLL_PARAMS[@]}" ]; then +if (( ${#ROLL_PARAMS[@]} == 0 )); then "${ROLL_DIR}/bin/roll" clinotty find var vendor pub/static pub/media app/etc \( -type f -or -type d \) -exec chmod u+w {} +; "${ROLL_DIR}/bin/roll" clinotty chmod u+x bin/magento else diff --git a/commands/magento2/magepack.cmd b/commands/magento2/magepack.cmd index ee705ba..382f90c 100644 --- a/commands/magento2/magepack.cmd +++ b/commands/magento2/magepack.cmd @@ -13,9 +13,9 @@ if [ -z "${ROLL_PARAMS[*]}" ]; then exit 0 fi -echo "Roll params: ${ROLL_PARAMS[@]}" +echo "Roll params: ${ROLL_PARAMS[*]}" echo "================================================" -echo "Other params: $@" +echo "Other params: $*" exit 1 diff --git a/commands/registry.cmd b/commands/registry.cmd index 4cc7f9f..13fd3b9 100644 --- a/commands/registry.cmd +++ b/commands/registry.cmd @@ -29,11 +29,11 @@ case "${ROLL_PARAMS[0]}" in categories) # List commands organized by category category="${ROLL_PARAMS[1]:-}" - + initializeRegistry - + if [[ -n "$category" ]]; then - echo -e "\033[33m${category^} Commands:\033[0m" + echo -e "\033[33m$(capitalize "$category") Commands:\033[0m" listCommandsByCategory "$category" else listCommandsByCategory diff --git a/commands/restore-full.cmd b/commands/restore-full.cmd index a53689a..d20cbd8 100644 --- a/commands/restore-full.cmd +++ b/commands/restore-full.cmd @@ -192,7 +192,7 @@ function logMessage() { SUCCESS) success "$@" ;; WARNING) warning "$@" ;; ERROR) error "$@" ;; - VERBOSE) [[ $RESTORE_VERBOSE -eq 1 ]] && info "[VERBOSE] $@" ;; + VERBOSE) [[ $RESTORE_VERBOSE -eq 1 ]] && info "[VERBOSE] $*" ;; esac } diff --git a/commands/restore.cmd b/commands/restore.cmd index 3dfea1a..55c1321 100755 --- a/commands/restore.cmd +++ b/commands/restore.cmd @@ -183,7 +183,7 @@ function logMessage() { SUCCESS) success "$@" ;; WARNING) warning "$@" ;; ERROR) error "$@" ;; - VERBOSE) [[ $RESTORE_VERBOSE -eq 1 ]] && info "[VERBOSE] $@" ;; + VERBOSE) [[ $RESTORE_VERBOSE -eq 1 ]] && info "[VERBOSE] $*" ;; esac } diff --git a/commands/sign-certificate.cmd b/commands/sign-certificate.cmd index 4004b3f..44cd03f 100644 --- a/commands/sign-certificate.cmd +++ b/commands/sign-certificate.cmd @@ -9,7 +9,7 @@ fi if (( ${#ROLL_PARAMS[@]} == 0 )); then echo -e "\033[33mCommand '${ROLL_CMD_VERB}' requires a hostname as an argument, please use --help for details." - exit -1 + exit 1 fi CERTIFICATE_SAN_LIST= diff --git a/commands/tableplus.help b/commands/tableplus.help index 80503fa..7f73831 100755 --- a/commands/tableplus.help +++ b/commands/tableplus.help @@ -1,4 +1,3 @@ - #!/usr/bin/env bash [[ ! ${ROLL_DIR} ]] && >&2 echo -e "\033[31mThis script is not intended to be run directly!\033[0m" && exit 1 diff --git a/utils/core.sh b/utils/core.sh index 51136c2..fbb4d3f 100644 --- a/utils/core.sh +++ b/utils/core.sh @@ -23,7 +23,7 @@ function error { function fatal { error "$@" - exit -1 + exit 1 } function boxinfo() { @@ -123,6 +123,12 @@ function version { echo "$@" | awk -F. '{ printf("%d%03d%03d%03d\n", $1,$2,$3,$4); }'; } +## capitalize the first letter of a string; bash 3.2 compatible +function capitalize() { + local s="$1" + printf '%s%s' "$(printf '%s' "${s:0:1}" | tr '[:lower:]' '[:upper:]')" "${s:1}" +} + ## determines if value is present in an array; returns 0 if element is present ## in array, otherwise returns 1 ## @@ -144,14 +150,14 @@ function assertDockerRunning { ## methods to peer global services requiring network connectivity with project networks function connectPeeredServices { - for svc in ${DOCKER_PEERED_SERVICES[@]}; do + for svc in "${DOCKER_PEERED_SERVICES[@]}"; do echo "Connecting ${svc} to $1 network" (docker network connect "$1" ${svc} 2>&1| grep -v 'already exists in network') || true done } function disconnectPeeredServices { - for svc in ${DOCKER_PEERED_SERVICES[@]}; do + for svc in "${DOCKER_PEERED_SERVICES[@]}"; do echo "Disconnecting ${svc} from $1 network" (docker network disconnect "$1" ${svc} 2>&1| grep -v 'is not connected') || true done diff --git a/utils/registry.sh b/utils/registry.sh index 4bd57bb..a07109d 100644 --- a/utils/registry.sh +++ b/utils/registry.sh @@ -270,7 +270,7 @@ function listCommandsByCategory() { # Display all categories for category in "${categories[@]}"; do - echo -e "\033[33m${category^} Commands:\033[0m" + echo -e "\033[33m$(capitalize "$category") Commands:\033[0m" listCommandsByCategory "$category" echo "" done @@ -360,7 +360,7 @@ function showRegistryStats() { # Display category counts i=0 while [[ $i -lt ${#categories[@]} ]]; do - printf " %-15s: %d commands\n" "${categories[$i]^}" "${category_counts[$i]}" + printf " %-15s: %d commands\n" "$(capitalize "${categories[$i]}")" "${category_counts[$i]}" i=$((i + 1)) done } From 83c93f776db749238becbda3aae992e17fe98652 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 13:47:21 +0200 Subject: [PATCH 02/41] Probe for the MariaDB 11 client binaries in roll db Finding H2, milestone 6. MariaDB 11 images dropped the mysql*/mysqldump compatibility symlinks, so roll db dump and roll db connect failed outright on any project pinned to mariadb:11.x. Probe the db container once per invocation and use whichever pair is present (mariadb/mariadb-dump preferred, mysql/mysqldump as fallback), rather than adding a symlink to the images repository, which this release does not touch. Both command substitutions on the failure path carry || true: bin/roll runs under set -e, so an unguarded assignment would kill the CLI before the fatal could name the missing binaries. Co-Authored-By: Claude Opus 5 --- commands/db.cmd | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/commands/db.cmd b/commands/db.cmd index 74b5fe4..50e2533 100644 --- a/commands/db.cmd +++ b/commands/db.cmd @@ -27,21 +27,40 @@ eval "$( ' | grep "^MYSQL_" )" +## MariaDB 11 images dropped the mysql*/mysqldump compatibility symlinks and only ship +## mariadb/mariadb-dump; probe once per invocation and use whichever pair is present, so both +## MariaDB 11 and MySQL/MariaDB 10.x images keep working. Sets RESOLVED_DB_BIN on success. +resolveDbBinary() { + local preferred="$1" fallback="$2" bin + + bin="$("${ROLL_DIR}/bin/roll" env exec -T db sh -c "command -v ${preferred} || command -v ${fallback}" 2>/dev/null)" || true + if [[ ! ${bin} ]]; then + local image + image="$(docker container inspect "${DB_CONTAINER}" --format '{{.Config.Image}}' 2>/dev/null)" || true + fatal "Neither \"${preferred}\" nor \"${fallback}\" was found in the db container (image: ${image:-unknown})." + fi + + RESOLVED_DB_BIN="${bin##*/}" +} + ## sub-command execution case "${ROLL_PARAMS[0]}" in connect) + resolveDbBinary mariadb mysql "${ROLL_DIR}/bin/roll" env exec db \ - mysql -u"${MYSQL_USER}" -p"${MYSQL_PASSWORD}" --database="${MYSQL_DATABASE}" "${ROLL_PARAMS[@]:1}" "$@" + "${RESOLVED_DB_BIN}" -u"${MYSQL_USER}" -p"${MYSQL_PASSWORD}" --database="${MYSQL_DATABASE}" "${ROLL_PARAMS[@]:1}" "$@" ;; import) + resolveDbBinary mariadb mysql LC_ALL=C sed -E 's/DEFINER[ ]*=[ ]*`[^`]+`@`[^`]+`/DEFINER=CURRENT_USER/g' \ | LC_ALL=C sed -E '/\@\@(GLOBAL\.GTID_PURGED|SESSION\.SQL_LOG_BIN)/d' \ | "${ROLL_DIR}/bin/roll" env exec -T db \ - mysql -u"${MYSQL_USER}" -p"${MYSQL_PASSWORD}" --database="${MYSQL_DATABASE}" "${ROLL_PARAMS[@]:1}" "$@" + "${RESOLVED_DB_BIN}" -u"${MYSQL_USER}" -p"${MYSQL_PASSWORD}" --database="${MYSQL_DATABASE}" "${ROLL_PARAMS[@]:1}" "$@" ;; dump) + resolveDbBinary mariadb-dump mysqldump "${ROLL_DIR}/bin/roll" env exec -T db \ - mysqldump -u"${MYSQL_USER}" -p"${MYSQL_PASSWORD}" "${MYSQL_DATABASE}" "${ROLL_PARAMS[@]:1}" "$@" + "${RESOLVED_DB_BIN}" -u"${MYSQL_USER}" -p"${MYSQL_PASSWORD}" "${MYSQL_DATABASE}" "${ROLL_PARAMS[@]:1}" "$@" ;; *) fatal "The command \"${ROLL_PARAMS[0]}\" does not exist. Please use --help for usage." From feb596dc388726b32e1a67851a4d4760f88269e4 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 13:47:21 +0200 Subject: [PATCH 03/41] Layer the linux compose fragments on WSL too Finding M12, milestone 9. WSL sets ROLL_ENV_SUBT=wsl, so appendEnvPartialIfExists never matched the .linux.yml fragments even though WSL2 is Linux and needs them. The path list becomes a directory x suffix loop, which reproduces the previous eight-path order exactly for linux and darwin. On wsl the suffix set is base, linux, wsl, with wsl last so a future wsl-specific fragment can override. No .wsl.yml exists today. ROLL_ENV_SUBT keeps wsl as a distinct value; config.sh still needs it to derive XDEBUG_CONNECT_BACK_HOST. Co-Authored-By: Claude Opus 5 --- utils/env.sh | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/utils/env.sh b/utils/env.sh index 0d54070..9d1028b 100644 --- a/utils/env.sh +++ b/utils/env.sh @@ -90,19 +90,29 @@ function assertValidEnvType () { function appendEnvPartialIfExists () { local PARTIAL_NAME="${1}" local PARTIAL_PATH="" + local PARTIAL_DIR="" + local SUFFIX="" - for PARTIAL_PATH in \ - "${ROLL_DIR}/environments/includes/${PARTIAL_NAME}.base.yml" \ - "${ROLL_DIR}/environments/includes/${PARTIAL_NAME}.${ROLL_ENV_SUBT}.yml" \ - "${ROLL_DIR}/environments/${ROLL_ENV_TYPE}/${PARTIAL_NAME}.base.yml" \ - "${ROLL_DIR}/environments/${ROLL_ENV_TYPE}/${PARTIAL_NAME}.${ROLL_ENV_SUBT}.yml" \ - "${ROLL_HOME_DIR}/environments/includes/${PARTIAL_NAME}.base.yml" \ - "${ROLL_HOME_DIR}/environments/includes/${PARTIAL_NAME}.${ROLL_ENV_SUBT}.yml" \ - "${ROLL_HOME_DIR}/environments/${ROLL_ENV_TYPE}/${PARTIAL_NAME}.base.yml" \ - "${ROLL_HOME_DIR}/environments/${ROLL_ENV_TYPE}/${PARTIAL_NAME}.${ROLL_ENV_SUBT}.yml" - do - if [[ -f "${PARTIAL_PATH}" ]]; then - DOCKER_COMPOSE_ARGS+=("-f" "${PARTIAL_PATH}") - fi + local PARTIAL_DIRS=( + "${ROLL_DIR}/environments/includes" + "${ROLL_DIR}/environments/${ROLL_ENV_TYPE}" + "${ROLL_HOME_DIR}/environments/includes" + "${ROLL_HOME_DIR}/environments/${ROLL_ENV_TYPE}" + ) + + # WSL2 is Linux, so it also needs the .linux.yml fragments; a future + # .wsl.yml fragment is layered last so it can override the linux one. + local SUFFIXES=("base" "${ROLL_ENV_SUBT}") + if [[ "${ROLL_ENV_SUBT}" == "wsl" ]]; then + SUFFIXES=("base" "linux" "wsl") + fi + + for PARTIAL_DIR in "${PARTIAL_DIRS[@]}"; do + for SUFFIX in "${SUFFIXES[@]}"; do + PARTIAL_PATH="${PARTIAL_DIR}/${PARTIAL_NAME}.${SUFFIX}.yml" + if [[ -f "${PARTIAL_PATH}" ]]; then + DOCKER_COMPOSE_ARGS+=("-f" "${PARTIAL_PATH}") + fi + done done } From 196213bc645eab883d3fa7eebaf3b855a39a9101 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 13:47:21 +0200 Subject: [PATCH 04/41] Add roll env sh as sugar for exec -T sh -c FEATURE-REQUESTS L1, milestone 20. Without it, redirects in roll env exec php-fpm echo hi > /tmp/probe apply on the host, which people hit constantly. The command must arrive as one quoted argument. Extra words are refused rather than silently dropped, since running only the first of them is the same confusion this subcommand exists to remove. Dash-prefixed extras need their own check: env is in ROLL_CMD_ANYARGS, so roll's parse loop stops at the first dash and leaves it in "$@" instead of ROLL_PARAMS, from where it would have reached sh -c as $0. Co-Authored-By: Claude Opus 5 --- FEATURE-REQUESTS.md | 25 +------------------------ commands/env.cmd | 20 ++++++++++++++++++++ commands/env.help | 15 +++++++++++++++ docs/usage.md | 4 ++++ 4 files changed, 40 insertions(+), 24 deletions(-) diff --git a/FEATURE-REQUESTS.md b/FEATURE-REQUESTS.md index 82636b9..4ca49e1 100644 --- a/FEATURE-REQUESTS.md +++ b/FEATURE-REQUESTS.md @@ -91,20 +91,6 @@ Adapted from the usual defect scale, since these are gaps rather than bugs: non-zero with a machine-readable summary. This composes well with H1 — the healthchecks it needs are the same ones. -**M3. No MySQL 8 to MariaDB DDL normalisation for `myloader`** — `commands/magento2/myloader.cmd` - -- *What:* The wrapper injects connection and directory defaults and forwards every other flag - untouched (verified by reading the file). It does nothing about source/target dialect differences. -- *Why it matters:* Restoring a MySQL 8 dump into RollDev's MariaDB container is a common path, and - two constructs in such dumps abort the load: the `ENCRYPTION='N'` table option (MariaDB: `ERROR - 1911: Unknown option 'ENCRYPTION'`) and `utf8mb4_0900_*` collations (`ERROR 1273: Unknown - collation`). Both terminate `myloader` with `Trace/breakpoint trap (core dumped)`, which gives no - indication of the real cause. Every consumer has to discover and solve this independently. -- *Suggested fix:* An opt-in `--normalize-source-ddl` flag that rewrites `*-schema*.sql` (strip the - `ENCRYPTION` option, remap `utf8mb4_0900_*` to `utf8mb4_general_ci`) before loading. Only the DDL - files should be touched, never the data files. If rewriting is considered out of scope, detecting - those two error codes and printing the cause would remove most of the cost. - **M4. No machine-readable output from RollDev's own commands** — `commands/status.cmd` - *What:* `roll status` renders ANSI-formatted output intended for a terminal. @@ -123,16 +109,7 @@ Adapted from the usual defect scale, since these are gaps rather than bugs: ### Low -**L1. No sugar for running a shell one-liner inside a service** — `commands/env.cmd` - -- *What:* `roll env exec > file` performs the redirect in the *host* shell, not in - the container. The command succeeds and writes to the wrong place, with nothing to indicate it. - The correct form is `roll env exec -T sh -c ''`. -- *Why it matters:* Silent wrong-place writes are hard to notice, particularly for things like - appending to a file inside a container, where the operation appears to succeed. -- *Suggested fix:* `roll env sh ''` that wraps the argument in `sh -c` inside the - container. Low severity because the workaround is reliable and only one flag away — but it is a - footgun worth either sugaring or documenting explicitly. +None found. ## Checked, and already covered diff --git a/commands/env.cmd b/commands/env.cmd index 3ae8ce6..1cc18f6 100644 --- a/commands/env.cmd +++ b/commands/env.cmd @@ -192,6 +192,26 @@ if [[ "${ROLL_PARAMS[0]}" == "describe" ]]; then exit $? fi +## handle sh sugar for shell commands in containers +if [[ "${ROLL_PARAMS[0]}" == "sh" ]]; then + if (( ${#ROLL_PARAMS[@]} < 3 )); then + fatal "roll env sh requires a service name and command: roll env sh ''" + fi + ## the command must arrive as ONE argument; refusing extra words is deliberate, because + ## silently running only the first of them is exactly the confusion this subcommand exists + ## to prevent + if (( ${#ROLL_PARAMS[@]} > 3 )); then + fatal "roll env sh takes a single quoted command: roll env sh ${ROLL_PARAMS[1]} '${ROLL_PARAMS[*]:2}'" + fi + ## env is in ROLL_CMD_ANYARGS, so roll's parse loop stops at the first dash-prefixed word and + ## leaves it in "$@" instead of ROLL_PARAMS; catch those here or they reach sh -c as $0 + if (( $# > 0 )); then + fatal "roll env sh takes a single quoted command: roll env sh ${ROLL_PARAMS[1]} '${ROLL_PARAMS[2]} $*'" + fi + # Transform: sh '' -> exec -T sh -c '' + ROLL_PARAMS=("exec" "-T" "${ROLL_PARAMS[1]}" "sh" "-c" "${ROLL_PARAMS[2]}") +fi + ## disconnect peered service containers from environment network if [[ "${ROLL_PARAMS[0]}" == "down" ]]; then disconnectPeeredServices "$(renderEnvNetworkName)" diff --git a/commands/env.help b/commands/env.help index 7258175..4fbca7f 100755 --- a/commands/env.help +++ b/commands/env.help @@ -9,5 +9,20 @@ COMPOSE_USAGE=$(docker compose "${ROLL_PARAMS[@]}" "$@" \ ROLL_USAGE=$(cat < '' + Run a shell command inside a container with proper shell handling. + Redirects and pipes work inside the container, not on the host. + + \033[1mWhy use this?\033[0m Without 'sh', redirects land on the host: + roll env exec php-fpm echo hi > /tmp/probe # writes to HOST /tmp/probe + roll env sh php-fpm 'echo hi > /tmp/probe' # writes to CONTAINER /tmp/probe + + \033[1mExamples:\033[0m + roll env sh php-fpm 'cat app/etc/env.php | grep MODE' + roll env sh php-fpm 'echo "test" > /tmp/test.txt' + roll env sh php-fpm 'ls -la | grep vendor' EOF ) diff --git a/docs/usage.md b/docs/usage.md index 84edd6b..0e8ab48 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -43,6 +43,10 @@ Flush varnish: roll env exec -T varnish varnishadm 'ban req.url ~ .' +Run a shell command inside a container (with proper redirect handling): + + roll env sh php-fpm 'cat app/etc/env.php | grep MODE' + Connect to redis: roll redis From 2e5f18d4da07961c65b053177773d057326ab1bf Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 13:51:02 +0200 Subject: [PATCH 05/41] Add myloader --normalize-source-ddl for MySQL 8 dumps FEATURE-REQUESTS M3, milestone 19. MySQL 8 dumps carry the ENCRYPTION table option and utf8mb4_0900_* collations, both of which MariaDB rejects; myloader reports this as an unhelpful "Trace/breakpoint trap (core dumped)". The opt-in flag rewrites only the *-schema*.sql files in the dump directory, through sed_inplace so BSD and GNU sed behave the same. Data files are never matched. Without the flag, a failed run's output is now inspected for the two signatures and names the cause plus the flag. --directory is a container path while normalization runs on the host, so the /var/www/html mount prefix is translated and a path outside it is refused rather than silently concatenated into a directory that does not exist. nullglob is saved and restored around the glob, verified in both directions under bash 3.2.57. Co-Authored-By: Claude Opus 5 --- commands/magento2/myloader.cmd | 104 ++++++++++++++++++++++++++++++-- commands/magento2/myloader.help | 12 +++- 2 files changed, 110 insertions(+), 6 deletions(-) diff --git a/commands/magento2/myloader.cmd b/commands/magento2/myloader.cmd index f7852c3..7eb9650 100644 --- a/commands/magento2/myloader.cmd +++ b/commands/magento2/myloader.cmd @@ -33,20 +33,114 @@ trap '' ERR ## default input directory (project-relative, produced by `roll mydumper`) MYLOADER_INPUT_DIR="var/mydumper" -## inject connection/location defaults unless the caller already supplied them +## Strips the ENCRYPTION table option and remaps utf8mb4_0900_* collations in mydumper schema +## files. MySQL 8 emits both, MariaDB rejects them, and only the schema files carry them — data +## files are never touched. +normalizeMyloaderSchemaDdl() { + local dump_dir="$1" + + if [[ ! -d "${dump_dir}" ]]; then + fatal "Dump directory \"${dump_dir}\" does not exist; cannot normalize schema DDL." + fi + + local had_nullglob=0 + shopt -q nullglob && had_nullglob=1 + shopt -s nullglob + local schema_files=("${dump_dir}"/*-schema*.sql) + [[ ${had_nullglob} -eq 0 ]] && shopt -u nullglob + + if [[ ${#schema_files[@]} -eq 0 ]]; then + warning "No *-schema*.sql files found in \"${dump_dir}\"; nothing to normalize." + return 0 + fi + + local file + for file in "${schema_files[@]}"; do + sed_inplace "s/ ENCRYPTION='N'//g; s/ENCRYPTION='N'//g; s/utf8mb4_0900_[a-zA-Z0-9_]*/utf8mb4_general_ci/g" "${file}" + done + + info "Normalized schema DDL in ${#schema_files[@]} file(s) under \"${dump_dir}\" for MariaDB compatibility." +} + +## inject connection/location defaults unless the caller already supplied them; also pull +## --normalize-source-ddl out of the args (myloader itself does not know it) and capture the +## effective directory so it can be normalized before myloader runs +MYLOADER_DIRECTORY="${MYLOADER_INPUT_DIR}" HAS_DIRECTORY=0 HAS_DATABASE=0 -for arg in "${ROLL_PARAMS[@]}" "$@"; do +NORMALIZE_SOURCE_DDL=0 +FILTERED_ARGS=() +ALL_ARGS=("${ROLL_PARAMS[@]}" "$@") +SKIP_NEXT=0 +for arg in "${ALL_ARGS[@]}"; do + if [[ ${SKIP_NEXT} -eq 1 ]]; then + SKIP_NEXT=0 + MYLOADER_DIRECTORY="${arg}" + FILTERED_ARGS+=("${arg}") + continue + fi + case "$arg" in - -d|--directory|--directory=*) HAS_DIRECTORY=1 ;; - -B|--database|--database=*) HAS_DATABASE=1 ;; + --normalize-source-ddl) + NORMALIZE_SOURCE_DDL=1 + continue + ;; + -d|--directory) + HAS_DIRECTORY=1 + SKIP_NEXT=1 + ;; + --directory=*) + HAS_DIRECTORY=1 + MYLOADER_DIRECTORY="${arg#*=}" + ;; + -B|--database|--database=*) + HAS_DATABASE=1 + ;; esac + + FILTERED_ARGS+=("${arg}") done +if [[ ${NORMALIZE_SOURCE_DDL} -eq 1 ]]; then + ## --directory is a path inside the container, but normalization runs on the host. The project + ## root is mounted at /var/www/html, so a relative path maps straight across and an absolute + ## one has to have that prefix stripped; anything else is outside the mount and unreachable. + NORMALIZE_DIR="${MYLOADER_DIRECTORY}" + case "${NORMALIZE_DIR}" in + /var/www/html/*) NORMALIZE_DIR="${ROLL_ENV_PATH}/${NORMALIZE_DIR#/var/www/html/}" ;; + /var/www/html) NORMALIZE_DIR="${ROLL_ENV_PATH}" ;; + /*) fatal "--normalize-source-ddl needs a dump directory inside the project (got \"${NORMALIZE_DIR}\"); it rewrites the files on the host." ;; + *) NORMALIZE_DIR="${ROLL_ENV_PATH}/${NORMALIZE_DIR}" ;; + esac + normalizeMyloaderSchemaDdl "${NORMALIZE_DIR}" +fi + ## Inject the connection details (read from the db container) and location defaults; all other ## flags are passed straight through to myloader. MYLOADER_ARGS=(--host=db --user=root --password="${MYSQL_ROOT_PASSWORD}") [[ ${HAS_DATABASE} -eq 0 ]] && MYLOADER_ARGS+=(--database="${MYSQL_DATABASE}") [[ ${HAS_DIRECTORY} -eq 0 ]] && MYLOADER_ARGS+=(--directory="${MYLOADER_INPUT_DIR}") -"${ROLL_DIR}/bin/roll" cli myloader "${MYLOADER_ARGS[@]}" "${ROLL_PARAMS[@]}" "$@" +## Capture myloader's output (pipefail in a subshell, so the pipeline reports myloader's exit +## code rather than tee's) so a failed run without --normalize-source-ddl can be diagnosed: MySQL +## 8 dumps loaded into MariaDB abort on the ENCRYPTION table option or utf8mb4_0900_* collations, +## and myloader's own message for both is an unhelpful "Trace/breakpoint trap (core dumped)". +MYLOADER_LOG="$(mktemp)" +trap 'rm -f "${MYLOADER_LOG}"' EXIT +MYLOADER_EXIT=0 +(set -o pipefail; "${ROLL_DIR}/bin/roll" cli myloader "${MYLOADER_ARGS[@]}" "${FILTERED_ARGS[@]}" 2>&1 | tee "${MYLOADER_LOG}") || MYLOADER_EXIT=$? + +if [[ ${MYLOADER_EXIT} -ne 0 && ${NORMALIZE_SOURCE_DDL} -eq 0 ]]; then + if grep -qE "ERROR 1911|Unknown option 'ENCRYPTION'" "${MYLOADER_LOG}"; then + error "myloader failed: the dump's schema uses the ENCRYPTION table option, which MariaDB does not support." + info "Retry with --normalize-source-ddl to strip it automatically." + elif grep -qE "ERROR 1273|Unknown collation|utf8mb4_0900" "${MYLOADER_LOG}"; then + error "myloader failed: the dump's schema uses a utf8mb4_0900_* collation, which MariaDB does not support." + info "Retry with --normalize-source-ddl to remap it automatically." + elif grep -q "Trace/breakpoint trap" "${MYLOADER_LOG}"; then + error "myloader crashed (Trace/breakpoint trap). This is the typical symptom of restoring a MySQL 8 dump into MariaDB: the ENCRYPTION table option or a utf8mb4_0900_* collation in the schema is unsupported by MariaDB." + info "Retry with --normalize-source-ddl to strip/remap these automatically." + fi +fi + +exit "${MYLOADER_EXIT}" diff --git a/commands/magento2/myloader.help b/commands/magento2/myloader.help index e1f132e..151300e 100644 --- a/commands/magento2/myloader.help +++ b/commands/magento2/myloader.help @@ -13,12 +13,22 @@ ROLL_USAGE=$(cat < Date: Mon, 31 Aug 2026 13:51:02 +0200 Subject: [PATCH 06/41] Record two defects found while fixing the ShellCheck gate Neither is in 0.8.0's scope, and both would otherwise be lost: the magepack one is now actively hidden, because the .shellcheckrc added in milestone 1 disables SC2317 repo-wide, which is the check that was reporting it. L1 is the same credential-in-ps exposure as RECLU H2, which milestone 24 fixes for TablePlus. Co-Authored-By: Claude Opus 5 --- FEATURE-REQUESTS.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/FEATURE-REQUESTS.md b/FEATURE-REQUESTS.md index 4ca49e1..b4e0b64 100644 --- a/FEATURE-REQUESTS.md +++ b/FEATURE-REQUESTS.md @@ -109,7 +109,27 @@ Adapted from the usual defect scale, since these are gaps rather than bugs: ### Low -None found. +**L1. `roll db` passes the database password as a process argument** — `commands/db.cmd:51,58,63` + +- *What:* `connect`, `import` and `dump` all invoke the client as + `-p"${MYSQL_PASSWORD}"`, so the password is visible to anything that can read `ps` inside the db + container. Pre-existing; the MariaDB 11 binary probe (finding H2) did not change it either way. +- *Why it matters:* It is the same class of exposure as RECLU's H2 for TablePlus, which the 0.8.0 + plan fixes in milestone 24. Fixing it here too would make the rule uniform across both repos. +- *Suggested fix:* Feed the credentials through `MYSQL_PWD` in the exec environment, or write a + `0600` defaults file into the container and pass `--defaults-extra-file`. The latter is what the + clients themselves recommend and it survives `import`'s stdin pipe, which `MYSQL_PWD` also does. + +**L2. Unreachable code at the end of `magepack.cmd`** — `commands/magento2/magepack.cmd` + +- *What:* An unconditional `exit 1` sits above the final + `roll env exec magepack generate …` line, so that line can never run. ShellCheck flags it as + SC2317, but SC2317 is disabled repo-wide in `.shellcheckrc`, so the gate no longer surfaces it. +- *Why it matters:* Either the `exit 1` is wrong and `magepack generate` is silently dead, or the + trailing line is leftover and should go. Both readings are defects, and the lint suppression now + hides the evidence. +- *Suggested fix:* Establish which of the two was intended, delete the other, and drop the + `disable=SC2317` line from `.shellcheckrc` so the check protects the file again. ## Checked, and already covered From 36ee8ed52539cf426c8fb8a249605b08db63c8e6 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 13:51:02 +0200 Subject: [PATCH 07/41] Remove the findings fixed by milestones 1, 2, 6, 9 and 20 Deletes H1, H2, H3, M9, M12 and L9 from Known issues, along with their Troubleshooting rows and the cross-references naming them from the platform table, the M10 finding and Audited and clean. Deleted rather than marked done, per the repo's own convention; git history is the record. Co-Authored-By: Claude Opus 5 --- README.md | 131 +++++------------------------------------------------- 1 file changed, 10 insertions(+), 121 deletions(-) diff --git a/README.md b/README.md index 2e99d8f..5bc8932 100644 --- a/README.md +++ b/README.md @@ -14,11 +14,8 @@ on unattended Linux hosts alike. -> The repository's only automated test — `shellcheck commands/*.cmd utils/*.sh`, run by -> `.github/workflows/shellcheck.yml` — currently **fails** (6 errors, 111 warnings, 95 notes as of -> version 0.7.2), so it provides no regression signal. `roll db dump` is also broken against the -> MariaDB 11.x images that are in active use. Both are recorded in -> [Known issues & improvement points](#known-issues--improvement-points) as **H1** and **H2**. +> `roll db dump` is broken against the MariaDB 11.x images that are in active use. Recorded in +> [Known issues & improvement points](#known-issues--improvement-points) as **H2**. ## Contents @@ -82,7 +79,7 @@ cosmetic one. Anything touching these areas needs testing on both: | Area | macOS (`darwin`) | Linux (`linux`) | Where | |---|---|---|---| -| Bash version | 3.2.57 (system bash) | usually 5.x | Why the Bash 3.2 constraint exists; a bash-4 construct fails only on macOS (**H3**) | +| Bash version | 3.2.57 (system bash) | usually 5.x | Why the Bash 3.2 constraint exists; use `bash 3.2` compatible syntax only | | File sync | Mutagen session, named volume for the web root | direct bind mount, no Mutagen | `environments//.darwin.yml`, `commands/sync.cmd:33-35` (fatals off darwin) | | `sed -i` | BSD — requires a backup suffix | GNU — suffix optional | Always use `sed_inplace` (`utils/core.sh:167`) | | `ping` flags | `-t` is a **timeout** | `-t` is the **TTL** | `isOnline` (`utils/core.sh:161`) is wrong on Linux — finding **M11** | @@ -606,63 +603,7 @@ rather than defects, and of the [boundary findings](#boundary-findings) above. ### High -**H1. The repository's only automated test fails** — `.github/workflows/shellcheck.yml:20` - -- *What:* CI runs `shellcheck commands/*.cmd utils/*.sh`, which exits 1. **Verified:** 6 errors, 111 - warnings and 95 notes on a clean checkout. There is no `.shellcheckrc` and no severity threshold, so - every one of the 212 findings is a failure. The errors are: - - | Location | Check | Problem | - |---|---|---| - | `utils/core.sh:147`, `utils/core.sh:154` | SC2068 | Unquoted `${DOCKER_PEERED_SERVICES[@]}` re-splits elements | - | `utils/core.sh:26`, `commands/sign-certificate.cmd:12` | SC2242 | `exit -1` — the process actually exits 255 | - | `commands/restore.cmd:186`, `commands/restore-full.cmd:195` | SC2145 | A string argument mixed with an array expansion | - -- *Why it matters:* A gate that is always red is indistinguishable from a gate that has just gone red, - so the one mechanism that could have caught the quoting and array-expansion bugs elsewhere in this - list provides no signal. `exit -1` in `fatal` also means every fatal error reports exit status 255, - which is indistinguishable from a signal-terminated process to any script wrapping `roll`. -- *Suggested fix:* Fix the six errors first (`"${DOCKER_PEERED_SERVICES[@]}"`, `exit 1`, `${arr[*]}` - where a single string is intended), then add `.shellcheckrc` with the `disable=` list for the - warning classes the codebase deliberately accepts, so the gate goes green and stays meaningful. - Raising the threshold with `shellcheck -S error` is the quicker alternative but leaves the 111 - warnings unreviewed. - -**H2. `roll db dump` fails against the MariaDB 11.x images in active use** — `commands/db.cmd:44` - -- *What:* The `dump` subcommand runs `mysqldump` inside the `db` container. **Verified:** in - `ghcr.io/epartment/roll/mariadb:11.4` — the image a running environment on the review machine was - using — `mysqldump` does not exist; MariaDB 11 renamed it to `mariadb-dump`. The compatibility - symlink for the client (`mysql`, used by `connect` at `commands/db.cmd:34` and `import` at - `commands/db.cmd:40`) *is* still present, so only `dump` is affected. -- *Why it matters:* `roll db dump` exits with a Docker "executable file not found" error rather than - anything mentioning MariaDB versions, on a command whose whole purpose is to produce a backup. The - schema still defaults `MARIADB_VERSION` to 10.4, so the failure only appears on projects that pin a - newer version — which makes it look intermittent across projects. -- *Suggested fix:* Probe once and fall back, so both generations work: - `DUMP_BIN=$(roll env exec -T db sh -c 'command -v mariadb-dump || command -v mysqldump')`, then - invoke `${DUMP_BIN}`. Apply the same treatment to `mysql` in `connect`/`import` before the client - symlink is dropped too. A `mysqldump` symlink in the images repository would also fix it, but the - probe keeps this repo working against images it does not control. - -**H3. `roll registry categories` aborts on macOS** — `utils/registry.sh:273`, -`commands/registry.cmd:36` - -- *What:* Both lines use `${category^}`, bash 4's "uppercase first character" expansion. **Verified:** - under bash 3.2.57 (the macOS system bash, which `#!/usr/bin/env bash` resolves to on a machine - without a newer bash first on `PATH`) the command dies with - `bad substitution` and prints nothing else. A third occurrence, `${categories[$i]^}` at - `utils/registry.sh:363`, is silently accepted by bash 3.2 as a no-op, so `roll registry stats` - works but never capitalises. -- *Why it matters:* It is a hard failure of a documented subcommand on the primary development - platform, and it contradicts the Bash 3.2 constraint that the rest of the codebase carefully - observes. The inconsistency between the scalar and array forms is why it survived: the array form - produces no error to notice. -- *Suggested fix:* Replace all three with a helper — `printf '%s%s' "$(echo "${s:0:1}" | tr - '[:lower:]' '[:upper:]')" "${s:1}"` — or simply drop the capitalisation, since the category values - are already lowercase words. Add a `bash --posix`-independent smoke test that runs - `roll registry categories` under `/bin/bash` on a macOS runner, because Linux-only CI cannot catch - this class of bug. +None found. ### Medium @@ -783,20 +724,6 @@ time** — `commands/env.cmd:206` `roll env ps -q "${service}"`, then `docker container inspect` for the hostname. That also removes the assumption of a single replica. Apply the same change to `commands/describe.cmd:18`. -**M9. `fixowns` and `fixperms` mishandle multiple arguments** — `commands/magento2/fixowns.cmd:17,20`, -`commands/magento2/fixperms.cmd:17,21` - -- *What:* Both test `[ -z "${ROLL_PARAMS[@]}" ]`. With one element that works by accident; with two or - more, `[` receives three arguments and fails with `too many arguments`. `fixowns.cmd:20` then - interpolates the array into a single path — `/var/www/html/"${ROLL_PARAMS[@]}"` — which concatenates - the elements rather than treating them as separate targets. -- *Why it matters:* `roll fixowns app/code var` reports a shell error instead of fixing ownership, or - chowns a path that does not exist. Since these commands exist to recover from permission problems, - they fail exactly when someone is already stuck. -- *Suggested fix:* Test the count — `if (( ${#ROLL_PARAMS[@]} == 0 )); then` — and pass the array - through as separate arguments, prefixing each with `/var/www/html/` in a loop rather than inside one - quoted expansion. - **M10. Nothing tests the macOS half of a platform the tool must support** — `.github/workflows/shellcheck.yml:17` @@ -804,15 +731,14 @@ time** — `commands/env.cmd:206` on any platform — CI lints shell sources and builds documentation, nothing more. - *Why it matters:* macOS is the primary developer platform *and* the one with the divergent behaviour (bash 3.2, BSD flag semantics, Mutagen), so every platform-specific defect in this list - is invisible to CI by construction. **H3** is the clearest case: `${category^}` lints clean and runs - fine on Ubuntu, and hard-fails on the platform most users are on. **M11** is the mirror image — a - bug that only manifests on Linux, shipped from macOS machines. + is invisible to CI by construction. **M11** is an example — a bug that only manifests on Linux, + shipped from macOS machines. - *Suggested fix:* Add a `macos-latest` job running the same ShellCheck invocation, then a minimal matrix smoke test across `macos-latest` and `ubuntu-latest` that needs no Docker daemon: `./bin/roll version`, `./bin/roll registry validate`, `./bin/roll registry categories`, `./bin/roll config schema`, and a `roll env-init` into a temporary directory followed by `roll config validate`. On macOS, invoke via `/bin/bash ./bin/roll` so bash 3.2 is exercised rather - than whatever the runner has on `PATH`. That set would have caught **H3**, **M7** and **M9**. + than whatever the runner has on `PATH`. That set would catch **M7**. **M11. `isOnline` always reports offline on Linux, so `roll svc up` never refreshes images** — `utils/core.sh:161-163` @@ -834,28 +760,6 @@ time** — `commands/env.cmd:206` branch on `${ROLL_ENV_SUBT}` and use `-W 2` on Linux — but note `isOnline` is in `utils/core.sh`, which is sourced before the config is loaded, so `$OSTYPE` is the only signal available there. -**M12. WSL hosts get no OS-specific compose fragment at all** — `utils/config.sh:361-363`, -`utils/env.sh:96,98` - -- *What:* `loadRollConfig` sets `ROLL_ENV_SUBT=wsl` when `/proc/sys/kernel/osrelease` mentions - Microsoft. `appendEnvPartialIfExists` then looks for `.wsl.yml`, and **no such file exists** - anywhere in `environments/` (verified — only `.base.yml`, `.darwin.yml` and `.linux.yml` are - present). A WSL host therefore receives neither the darwin nor the linux variant of any fragment. -- *Why it matters:* The most consequential loss is `environments/includes/php-fpm.linux.yml`, the only - place the host's SSH agent socket is mounted into the containers. Without it `SSH_AUTH_SOCK` falls - back to `/tmp/ssh-auth.sock` from `environments/includes/php-fpm.base.yml:26`, which nothing - creates, so agent forwarding — and therefore `composer install` against private repositories — - cannot work on WSL, and the unconditional `chmod 777` at `commands/env.cmd:285-287` targets a path - that does not exist. WSL2 is a documented install target in `docs/installing.md`, so this is a - supported configuration silently missing a fragment. *The WSL-side consequences are read from the - code; unverified, as no WSL host was available for this review.* -- *Suggested fix:* Either treat WSL as Linux for fragment selection — have - `appendEnvPartialIfExists` fall back to the `linux` variant when `ROLL_ENV_SUBT` is `wsl`, which - matches the intent, since WSL is a Linux kernel — or add `environments/includes/php-fpm.wsl.yml`. - The fallback is preferable: it is one change in `utils/env.sh` and it covers every present and - future `.linux.yml` rather than only the one that is currently missed. Keep the separate `wsl` - value, because `utils/config.sh:440-442` legitimately needs it for `XDEBUG_CONNECT_BACK_HOST`. - ### Low **L1. `commands/usage.help` is out of date** — `commands/usage.help:44-77` @@ -946,18 +850,6 @@ time** — `commands/env.cmd:206` log is never clean, so a real warning is easy to miss. - *Suggested fix:* Remove both settings, or add the directories with a `.gitkeep`. -**L9. The ShellCheck gate does not cover the env-type commands or the help files** — -`.github/workflows/shellcheck.yml:5-13,20` - -- *What:* The glob is `commands/*.cmd utils/*.sh`. It excludes the ten `.cmd` files in - `commands/magento2/` and the one in `commands/wordpress/` (344 and 11 lines), and all 40 `.help` - files, which are Bash scripts sourced by `commands/usage.cmd:32`. The `paths:` trigger has the same - gap, so a change to `commands/magento2/` does not even start the workflow. -- *Why it matters:* Finding **M9** lives in exactly that blind spot — SC2199/SC2198 would have - flagged `[ -z "${ROLL_PARAMS[@]}" ]`. -- *Suggested fix:* Lint `commands/**/*.cmd`, `commands/**/*.help` and `utils/*.sh`, and widen the - `paths:` filter to match. Do this after **H1**, since it will surface more findings. - **L10. Root CA trust is skipped without warning on Linux distributions that are neither Debian- nor Fedora-family** — `commands/install.cmd:29-52` @@ -989,10 +881,10 @@ Checked during this review and found correct, so the next reader does not re-inv `commands/vnc.cmd` are correctly macOS- and Linux-specific respectively. `ping` in `isOnline` (**M11**) and the missing `wsl` fragment (**M12**) are the only two places where the platform difference was not handled. -- **Bash 3.2 compliance elsewhere.** The parallel-indexed-array pattern in `utils/config.sh` and +- **Bash 3.2 compliance.** The parallel-indexed-array pattern in `utils/config.sh` and `utils/registry.sh` is applied consistently; no associative arrays anywhere. `commands/status.cmd` - correctly guards its `mapfile` use with a `while read` fallback. The only bash-4 constructs found are - the three `${var^}` uses in finding **H3**. + correctly guards its `mapfile` use with a `while read` fallback. Case modification is always done + via `tr` or the `capitalize` helper function. - **Arithmetic increments.** No bare `((x++))` remains in `commands/` or `utils/`; the codebase uses `x=$((x + 1))`, which is what keeps `set -e` from killing the CLI mid-command. - **`roll redis` works with Dragonfly.** `environments/includes/dragonfly.base.yml` defines the @@ -1034,14 +926,11 @@ line-audited, and neither backup nor restore was executed against a live environ | `docker compose version should be 2.2.3 or higher` | Compose v1, or the plugin is missing | Install the Compose v2 plugin; `docker-compose` (hyphenated) is not used | | `invalid project name "…"` from `roll env up` | `ROLL_ENV_NAME` contains uppercase or an illegal character | Lowercase it in `.env.roll`; note the rename orphans the old volumes (finding **M7**) | | Environment comes up on an empty database after a rename | `ROLL_ENV_NAME` is the Compose project name and prefixes every named volume | Bring the environment down, copy `oldname_dbdata` to `newname_dbdata`, bring it up | -| `bad substitution` from `roll registry categories` | Bash 4 syntax under macOS bash 3.2 | Finding **H3**; no workaround for the subcommand — use `roll registry list` | -| `executable file not found` from `roll db dump` | `mysqldump` absent on MariaDB 11.x | Finding **H2**; workaround `roll env exec -T db mariadb-dump -u… -p… ` | | Magento cannot reach Varnish, Elasticsearch or RabbitMQ, and the containers are not running | `.env.roll` omits the toggles, which resolve to `0` despite the env-type default | Finding **M1**; set `ROLL_VARNISH=1`, `ROLL_ELASTICSEARCH=1`, `ROLL_RABBITMQ=1` explicitly | | A project silently runs an older PHP or Node than expected | Schema default won over the fragment fallback | Finding **M2**; pin the version in `.env.roll` | | `port is already allocated` on `php-fpm` | `ROLL_BROWSERSYNC=1` publishes fixed host ports on that service — the only fragment that publishes any | Set `ROLL_BROWSERSYNC=0`; verify with `roll env config \| grep published`. See `FEATURE-REQUESTS.md` H2 | | A command run right after `roll env up` fails to connect to the DB or search engine | No fragment declares a `healthcheck:` and `up` does not pass `--wait`, so it returns when containers *start* | Retry with a wait loop against the service. See `FEATURE-REQUESTS.md` H1 | | Shared-service images never update on Linux, however often `roll svc up` runs | `isOnline` uses BSD `ping -t` semantics, so the probe always fails on Linux and `svc pull` is skipped | Finding **M11**; run `roll svc pull` explicitly | -| SSH agent forwarding does not work inside the containers on WSL2 | `ROLL_ENV_SUBT=wsl` matches no compose fragment, so the socket mount from `php-fpm.linux.yml` is never applied | Finding **M12**; as a workaround add the mount to the project's `.roll/roll-env.yml` | | `.test` sites show a certificate error on Linux after a clean `roll install` | CA trust is only wired up for Debian- and Fedora-family distributions, and is skipped silently otherwise | Finding **L10**; add `~/.roll/ssl/rootca/certs/ca.cert.pem` to the distribution's trust store manually | | `Mutagen sync sessions are not used on "linux" host environments` | `roll sync` is macOS-only | Expected; Linux and WSL bind-mount directly | | `Mutagen configuration does not exist for environment type "…"` | Only `magento1`, `magento2` and `shopware` ship a `.mutagen.yml` | Expected; that type uses a bind mount on macOS too | From df648b3ce611e7b0cc9ca39b6107123d5aa549c8 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 14:14:20 +0200 Subject: [PATCH 08/41] Probe the registry over HTTPS instead of pinging Finding M11, milestone 8. isOnline used ping -t, where -t is a timeout on BSD and the TTL on Linux, so the probe misbehaved on Linux and roll svc up never refreshed images there. ICMP is also commonly blocked. Probe https://ghcr.io/v2/ instead, which is the registry svc pull actually needs. Deliberately no -f: that endpoint answers 401 unauthenticated, and any HTTP response proves reachability. Verified true on a working network and false in 3s against an unroutable address. The Linux leg is unverified from here, but the flags used behave identically on both platforms, which is the point of the change. Co-Authored-By: Claude Opus 5 --- utils/core.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/utils/core.sh b/utils/core.sh index fbb4d3f..0eb4a7a 100644 --- a/utils/core.sh +++ b/utils/core.sh @@ -163,9 +163,15 @@ function disconnectPeeredServices { done } -# Main logic with the timeout function +# Probe the registry that `svc pull` actually needs, with a 3-second timeout. +# Accept any HTTP response as proof of reachability (even 401 proves we reached the server). +# Do not use -f so 401 is treated as successful connection. function isOnline() { - (ping -q -c1 -t 2 8.8.8.8 &>/dev/null && echo "true") || (ping -q -c1 -t 2 1.1.1.1 &>/dev/null && echo "true") || echo "false" + if curl -s -m 3 -o /dev/null "https://ghcr.io/v2/" 2>/dev/null; then + echo "true" + else + echo "false" + fi } ## cross-platform sed in-place editing function From c228bde9a4f665aa7239b5105876710f9592ccfa Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 14:14:20 +0200 Subject: [PATCH 09/41] Run CI on macOS as well, with a Docker-free smoke suite Finding M10, milestone 3. CI was Ubuntu-only, so every macOS-specific defect was invisible by construction: the bash-4 ${var^} bug fixed earlier in this release linted clean and ran fine on Ubuntu while hard-failing under the macOS system bash that most users have. Extend the existing workflow into a ubuntu-latest/macos-latest matrix rather than adding a second workflow, which would have run the same lint twice per push. The macOS leg invokes /bin/bash bin/roll so bash 3.2 is exercised. The smoke script needs the docker CLI even though it never talks to a daemon: bin/roll fatals for every command, version included, when docker is absent or docker compose version does not parse. macOS runners also lack envsubst, which env-init needs. Both are installed on that leg. The workflow itself cannot be verified until pushed. Co-Authored-By: Claude Opus 5 --- .github/scripts/smoke.sh | 58 ++++++++++++++++++++++++++++++++ .github/workflows/shellcheck.yml | 49 +++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 3 deletions(-) create mode 100755 .github/scripts/smoke.sh diff --git a/.github/scripts/smoke.sh b/.github/scripts/smoke.sh new file mode 100755 index 0000000..3c348fa --- /dev/null +++ b/.github/scripts/smoke.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# +# Docker-free smoke test for the `roll` CLI. +# +# Exercises commands that need neither a running Docker daemon nor an existing project checkout, +# so platform-specific regressions (bash 3.2 syntax, BSD vs GNU tool differences) surface without +# needing the full Docker-backed environment stack. Run from CI (macos-latest + ubuntu-latest) or +# locally. +# +# Usage: smoke.sh [bash32] +# (no argument) invoke roll natively, i.e. via its own #!/usr/bin/env bash shebang +# bash32 invoke roll explicitly under /bin/bash, to exercise bash 3.2.57 on macOS rather +# than whatever newer bash happens to be first on PATH + +set -eu + +SCRIPT_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd -P)" +ROLL_DIR="$(cd -- "${SCRIPT_DIR}/../.." && pwd -P)" + +MODE="${1:-native}" + +case "${MODE}" in + bash32) + ROLL_BIN=(/bin/bash "${ROLL_DIR}/bin/roll") + ;; + native) + ROLL_BIN=("${ROLL_DIR}/bin/roll") + ;; + *) + >&2 echo "Usage: $0 [bash32]" + exit 1 + ;; +esac + +run() { + echo "+ ${ROLL_BIN[*]} $*" + "${ROLL_BIN[@]}" "$@" +} + +run version +run config schema >/dev/null +run registry validate +run registry categories >/dev/null +run registry paths >/dev/null + +TMP_DIR="$(mktemp -d "${TMPDIR:-/tmp}/roll-smoke.XXXXXX")" +cleanup() { + rm -rf -- "${TMP_DIR}" +} +trap cleanup EXIT + +( + cd -- "${TMP_DIR}" + "${ROLL_BIN[@]}" env-init smoketest magento2 + "${ROLL_BIN[@]}" config validate +) + +echo "Smoke test passed (${MODE} mode)." diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml index 5a335d8..1843cb4 100644 --- a/.github/workflows/shellcheck.yml +++ b/.github/workflows/shellcheck.yml @@ -2,27 +2,70 @@ name: ShellCheck on: push: paths: + - 'bin/roll' - 'commands/**/*.cmd' - 'commands/**/*.help' - 'utils/*.sh' + - 'environments/**' + - '.github/scripts/smoke.sh' - '.shellcheckrc' - '.github/workflows/shellcheck.yml' pull_request: paths: + - 'bin/roll' - 'commands/**/*.cmd' - 'commands/**/*.help' - 'utils/*.sh' + - 'environments/**' + - '.github/scripts/smoke.sh' - '.shellcheckrc' - '.github/workflows/shellcheck.yml' workflow_dispatch: jobs: shellcheck: - runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} steps: - name: Checkout repository uses: actions/checkout@v3 - - name: Install shellcheck + + - name: Install shellcheck (macOS) + if: runner.os == 'macOS' + run: brew install shellcheck + + - name: Install shellcheck (Linux) + if: runner.os == 'Linux' run: sudo apt-get update && sudo apt-get install -y shellcheck + - name: Run shellcheck - run: shellcheck commands/*.cmd commands/*/*.cmd commands/*.help commands/*/*.help utils/*.sh + run: shellcheck commands/*.cmd commands/*/*.cmd commands/*.help commands/*/*.help utils/*.sh .github/scripts/smoke.sh + + # bin/roll requires a `docker` CLI on PATH (it runs `docker compose version` before + # dispatching to any subcommand, including Docker-free ones) but none of the smoke commands + # below touch a daemon. ubuntu-latest runners ship Docker already; macos-latest runners ship + # neither Docker nor GNU gettext (`envsubst`, needed by `env-init` for env types that carry + # an init.env), so install both as client-only tooling. + - name: Install Docker CLI + Compose plugin + gettext (macOS) + if: runner.os == 'macOS' + run: | + brew install docker docker-compose gettext + brew link --force gettext + mkdir -p "${HOME}/.docker" + cat > "${HOME}/.docker/config.json" < Date: Mon, 31 Aug 2026 14:14:43 +0200 Subject: [PATCH 10/41] Fix the config load order and stop defaulting service versions silently Findings M1 and M2, milestone 4, decision D4a. setConfigDefault ran before postProcessConfig and skips any key already in the cache, so every schema key with a literal default was set before the env-type derivation ran. That made each ${VAR:-x} in postProcessConfig dead, which is why a magento2 project resolved ROLL_VARNISH=0 despite the code reading ${ROLL_VARNISH:-1}. Verified against a bare magento2 project before the change. The order is now: env-type defaults, then schema literals, then version-pin handling, then derivations. applyEnvTypeDefaults never overwrites a key that came from a config file, so an explicit value still wins. commands/env.cmd recomputed all of this a second time, which is what made real projects work and what kept the loader's own bug invisible. Those 82 lines are gone; config.sh is the sole owner. It now also exports NGINX_TEMPLATE, NGINX_PUBLIC and CHOWN_DIR_LIST unconditionally, as env.cmd used to. Service versions become string:optional and an unpinned enabled service warns, falling back to the version the project was already running. Per D4a this is a warning in 0.8.0 and an error in 0.9.0: measured against the machine's 48 local projects, 34 had no NGINX_VERSION, so a hard error would have broken most of them and the nightly build server on upgrade day. The fallback is deliberately the old schema literal and not the env type's init.env value, which would have jumped existing projects from Elasticsearch 7.17 to 8.11. roll config fix-pins writes the missing pins after taking a backup; roll config check-pins is the read-only form and exits 1 when any are missing. Neither takes a --dry-run flag, because config is not on ROLL_CMD_ANYARGS and adding it would make roll config --help recurse. Two further defects found while doing this: the mongodb fragment interpolated MONGODB_VERSION, which the schema never defined, so that image tag always resolved to the fragment's own literal - the fragment now reads MONGO_VERSION and the legacy fallback is 7, matching what actually ran. laravel/init.env pinned the same misspelled key. Every init.env gained the NGINX_VERSION pin no type had ever set, and local and vuejs gained init.env files so env-init produces valid projects for them. Verified: rendered compose config is byte-identical to 0.7.2 for the live magento2 project and for seeded fixtures of all ten renderable environment types, both before and after fix-pins. The local type fails to render, before and after alike - pre-existing, an appended fragment depends on php-fpm, which local never gets. Co-Authored-By: Claude Opus 5 --- .shellcheckrc | 5 + README.md | 154 +++--------- commands/config.cmd | 57 ++++- commands/config.help | 11 + commands/describe.cmd | 12 +- commands/env.cmd | 85 +------ docs/configuration/version-pins.md | 86 +++++++ environments/akeneo/init.env | 5 + environments/includes/db.base.yml | 2 +- environments/includes/dragonfly.base.yml | 2 +- environments/includes/elasticsearch.base.yml | 2 +- environments/includes/mongodb.base.yml | 2 +- environments/includes/nginx.base.yml | 2 +- environments/includes/opensearch.base.yml | 2 +- environments/includes/php-fpm.base.yml | 8 +- environments/includes/rabbitmq.base.yml | 2 +- environments/includes/redis.base.yml | 2 +- environments/includes/selenium.base.yml | 2 +- environments/includes/varnish.base.yml | 2 +- environments/laravel/init.env | 5 +- environments/local/init.env | 11 + environments/magento1/init.env | 3 + environments/magento2/init.env | 4 +- .../magento2/magento2.magepack.base.yml | 2 +- environments/php/init.env | 3 + environments/shopware/init.env | 3 + environments/symfony/init.env | 3 + environments/typo3/init.env | 3 + environments/vuejs/init.env | 13 + environments/wordpress/init.env | 3 + utils/config.sh | 232 +++++++++++++++--- 31 files changed, 462 insertions(+), 266 deletions(-) create mode 100644 docs/configuration/version-pins.md create mode 100644 environments/local/init.env create mode 100644 environments/vuejs/init.env diff --git a/.shellcheckrc b/.shellcheckrc index 48f42c7..f25d02d 100644 --- a/.shellcheckrc +++ b/.shellcheckrc @@ -110,3 +110,8 @@ disable=SC2178 # `commands/magento2/magepack.cmd` ends with unreachable debug output left from earlier work; kept # out of scope for this release, not treated as a defect worth fixing here. disable=SC2317 + +# Service toggles such as ROLL_DB reach the command files as exports from utils/config.sh rather +# than by assignment, so ShellCheck reads them as possible misspellings of ROLL_DIR. Same +# sourced-into-bin/roll boundary as SC2034 above. +disable=SC2153 diff --git a/README.md b/README.md index 5bc8932..0227f8c 100644 --- a/README.md +++ b/README.md @@ -82,14 +82,14 @@ cosmetic one. Anything touching these areas needs testing on both: | Bash version | 3.2.57 (system bash) | usually 5.x | Why the Bash 3.2 constraint exists; use `bash 3.2` compatible syntax only | | File sync | Mutagen session, named volume for the web root | direct bind mount, no Mutagen | `environments//.darwin.yml`, `commands/sync.cmd:33-35` (fatals off darwin) | | `sed -i` | BSD — requires a backup suffix | GNU — suffix optional | Always use `sed_inplace` (`utils/core.sh:167`) | -| `ping` flags | `-t` is a **timeout** | `-t` is the **TTL** | `isOnline` (`utils/core.sh:161`) is wrong on Linux — finding **M11** | +| Network probe | `ping` (unreliable) | `ping` (unreliable) | `isOnline` probes `https://ghcr.io/v2/` with `curl -m 3` instead, avoiding platform flag differences and ICMP blockage | | SSH agent socket | fixed host path `/run/host-services/ssh-auth.sock` | `${SSH_AUTH_SOCK}` from the host, and a socat proxy unless UID is 1000 | `environments/includes/php-fpm.darwin.yml` vs `.linux.yml`, `utils/config.sh:445-447` | | `.test` DNS | automatic via `/etc/resolver/test` | manual — `roll install` only prints a warning | `commands/install.cmd:56-66` | | Root CA trust | `security add-trusted-cert` | Fedora/CentOS and Debian/Ubuntu paths only; other distros silently skipped | `commands/install.cmd:29-52`, finding **L10** | | tunnel key permissions | untouched | `chown root:root` on `ssh_key.pub`, because bind mounts are native | `commands/install.cmd:82-84` | | Xdebug host | `host.docker.internal` | container gateway address, looked up at runtime | `commands/debug.cmd:13-21` | | `mapfile` | absent (bash 3.2) | present | `commands/status.cmd:20-28` shows the required fallback | -| CI coverage | **none** | `ubuntu-latest` | All five workflows; finding **M10** | +| CI coverage | `macos-latest`: ShellCheck + Docker-free smoke | `ubuntu-latest`: ShellCheck + Docker-free smoke | `.github/workflows/shellcheck.yml` matrix | Prerequisites: Docker (Desktop on macOS/Windows, Engine on Linux) with `docker compose` ≥ 2.2.3, and Homebrew. On macOS, Mutagen is installed automatically on first `roll sync` if missing. @@ -185,18 +185,25 @@ literal default value. Loading proceeds in this order (`loadRollConfig`, `utils/ 1. `~/.roll/.env.roll`, then legacy `~/.roll/.env` — global settings. 2. `${ROLL_ENV_PATH}/.env.roll` — project settings, overriding global. 3. OS detection sets `ROLL_ENV_SUBT` to `darwin`, `linux`, or `wsl`. -4. `setConfigDefault` fills in every schema key that has a literal default and is not yet set. -5. `assertValidEnvType` checks that `environments//.base.yml` exists. -6. `postProcessConfig` derives image variants (`ROLL_SVC_PHP_VARIANT`, `ROLL_SVC_PHP_NODE`), the +4. `assertValidEnvType` checks that `environments//.base.yml` exists. +5. `applyEnvTypeDefaults` fills in what the environment type needs — the `magento2` service + toggles, the non-`local` base services, the DB distribution version — but never overwrites a key + the project set itself. +6. `setConfigDefault` fills in every schema key that still has no value and carries a literal + default. +7. `applyVersionPinFallbacks` warns about every enabled service with no version pin and falls back + to the version the project was already running. +8. `postProcessConfig` derives image variants (`ROLL_SVC_PHP_VARIANT`, `ROLL_SVC_PHP_NODE`), the Xdebug tag, and the nginx template name. -Step 4 running before step 6 is the root cause of finding **M1** below: any `${VAR:-default}` -expression in step 6 (or in `commands/env.cmd`) is unreachable for a key that has a literal schema -default, because step 4 already gave it a value. +**The order matters.** Env-type defaults have to run before the schema literals, or every +`${VAR:-default}` in step 5 is unreachable — the literal has already given the key a value, and +`:-` only substitutes when unset or empty. Derivations that depend on final input values run last. Every value is exported, so it reaches `docker compose` both through `--env-file .env.roll` and -through the process environment. Where the two disagree, the exported value wins — which is why the -`${...:-fallback}` defaults written inside the YAML fragments are mostly dead (finding **M2**). +through the process environment. Where the two disagree, the exported value wins, so the +`${...:-fallback}` defaults inside the YAML fragments are not the effective defaults; the service +version fallbacks have been removed from the fragments for that reason. ### How a compose invocation is assembled @@ -266,7 +273,7 @@ responsibility is still describable. |---|---|---|---| | `bin/roll` | 98 lines | Entry point, dispatch, argument parsing | **Coherent** | | `utils/core.sh` | 184 lines | Messaging helpers, array/version utilities, network peering | **Coherent** — the three box-drawing functions are near-identical triplets (finding **L4**) | -| `utils/config.sh` | 606 lines | Config schema, loading, validation, post-processing | **Coherent**, but owns defaults that `commands/env.cmd` also owns (**M1**) | +| `utils/config.sh` | 606 lines | Config schema, loading, validation, post-processing | **Coherent**; sole owner of configuration defaults | | `utils/registry.sh` | 461 lines | Command discovery and priority resolution | **Oversized for what it delivers** — ~200 lines serve `roll registry`'s reporting subcommands; the metadata layer they report on is a stub (**M6**) | | `utils/env.sh` | 108 lines | Env path location, partial precedence, env-type validation | **Coherent** | | `utils/install.sh` | 63 lines | Host install assertion, SSH config | **Coherent** | @@ -287,8 +294,6 @@ part. These are the couplings that are not visible from it: | A | B | Mechanism | What breaks if separated | |---|---|---|---| -| `utils/config.sh` (`postProcessConfig`) | `commands/env.cmd:24-105` | The **same** env-type defaulting logic is written twice, in full: PHP/Node image variants, DB distribution version, Xdebug tag, nginx template, `CHOWN_DIR_LIST`, magento1/magento2 service toggles | Nothing breaks — they already disagree. `env.cmd`'s copy is dead for keys with literal schema defaults, so the two produce different results depending on which keys `.env.roll` sets (**M1**) | -| `utils/config.sh` schema defaults | `environments/**/*.yml` `${VAR:-fallback}` | Config exports a value for every key with a literal default, so the YAML fallback never applies | The YAML fallbacks read as the effective default and are not; eight of them disagree with the schema (**M2**) | | `commands/backup.cmd` | `commands/restore.cmd`, `commands/restore-full.cmd` | Undocumented on-disk contract: the staging directory `/.roll/backups`, the archive layout, and the metadata JSON. Each file re-derives the path independently (documented in a comment at `commands/backup.cmd:22-35`) | Changing the layout in one file silently breaks the others; `restore-full.cmd` never loads the env config at all, so it resolves paths differently | | `commands/status.cmd:6` | `docker/docker-compose.yml` | The shared network name is recovered by `grep -A3 'networks:' … \| tail -n1 \| sed`, i.e. by text-parsing YAML | Reordering keys in `docker-compose.yml` breaks `roll status`'s "is RollDev running" check | | `commands/describe.cmd:18`, `commands/vnc.cmd:12` | Compose container naming | Both reconstruct container names by string concatenation instead of asking `roll env ps -q` — and they disagree: `describe` uses v2 (`name-svc-1`), `vnc` uses v1 (`name_svc_1`) | `roll vnc` cannot find a container under the Compose v2 that `bin/roll:26` requires (**M8**) | @@ -429,9 +434,9 @@ Registered in `utils/config.sh:70-87`. Each enabled toggle appends the matching | `ROLL_MAGEPACK` | 0 | Magepack bundling | `magento2` only | | `ROLL_INCLUDE_GIT` | 0 | bind-mounts `.git` into the container | needed on macOS with Mutagen | -For a `magento2` project the effective values of `ROLL_VARNISH`, `ROLL_ELASTICSEARCH` and -`ROLL_RABBITMQ` come from `init.env`, not from the env-type block in `commands/env.cmd:73-76` — see -finding **M1**. +For a `magento2` project `ROLL_VARNISH`, `ROLL_ELASTICSEARCH` and `ROLL_RABBITMQ` default to `1` +via `applyEnvTypeDefaults` in `utils/config.sh`, whether or not `init.env` spelled them out. A +value in `.env.roll` still wins. ### Local, test/staging, and production @@ -555,10 +560,13 @@ sphinx-build -b html docs docs/_build/html These are enforced by review rather than by tooling, except where noted. - **Every change must work on macOS and Linux.** Both are first-class targets — see - [Supported platforms](#supported-platforms) for the table of real behavioural differences. CI only - runs Ubuntu (finding **M10**), so the macOS half is on the author: run the affected command under - `/bin/bash` on macOS before merging. The recurring traps are bash 3.2, BSD vs GNU flag semantics - (`sed`, `ping`, `stat`), Mutagen-vs-bind-mount file sync, and the SSH agent socket path. + [Supported platforms](#supported-platforms) for the table of real behavioural differences. CI runs + a `macos-latest` + `ubuntu-latest` matrix (ShellCheck plus the Docker-free smoke script, + `.github/scripts/smoke.sh`), but the smoke set only covers commands that need neither a project + checkout nor a running daemon — anything touching `roll env`/service containers is still on the + author: run the affected command under `/bin/bash` on macOS before merging. The recurring traps + are bash 3.2, BSD vs GNU flag semantics (`sed`, `ping`, `stat`), Mutagen-vs-bind-mount file sync, + and the SSH agent socket path. - **Bash 3.2 only.** No associative arrays, no `${var^}`/`${var,}` case modification, no `mapfile` without a fallback (`commands/status.cmd:20-28` shows the pattern). macOS ships bash 3.2.57 and `bin/roll` runs under `#!/usr/bin/env bash`, so a bash-4 construct fails there while passing on @@ -607,60 +615,6 @@ None found. ### Medium -**M1. Env-type service defaults are dead code; the toggles they set resolve to 0** — -`utils/config.sh:378-381` and `utils/config.sh:473-476`, duplicated at `commands/env.cmd:73-76` - -- *What:* `loadRollConfig` runs `setConfigDefault` for every schema key (`utils/config.sh:378-381`) - *before* calling `postProcessConfig` (`utils/config.sh:389`). `ROLL_VARNISH`, - `ROLL_ELASTICSEARCH` and `ROLL_RABBITMQ` are declared `boolean:0`, so by the time - `postProcessConfig` evaluates `export ROLL_VARNISH="${ROLL_VARNISH:-1}"` the variable is already - `0` and the `:-1` never applies. `commands/env.cmd:73-76` repeats the same block and fails the same - way. **Verified** by loading a minimal `magento2` `.env.roll` (containing only `ROLL_ENV_NAME` and - `ROLL_ENV_TYPE`) through `loadEnvConfig`: `ROLL_VARNISH=0`, `ROLL_ELASTICSEARCH=0`, - `ROLL_RABBITMQ=0`. -- *Why it matters:* A `magento2` environment whose `.env.roll` does not list these keys comes up with - no Varnish, no search engine and no message queue — and Magento's own configuration will point at - services that are not running. New projects are shielded because - `environments/magento2/init.env` sets all three explicitly, so the bug only surfaces on - hand-written or older `.env.roll` files, which is exactly when it is hardest to attribute. The same - ordering makes `postProcessConfig`'s `DB_DISTRIBUTION_VERSION` branch (`utils/config.sh:424-430`) - unreachable. -- *Suggested fix:* Move the `setConfigDefault` loop to *after* `postProcessConfig`, so per-type - derivation runs against unset variables and the schema fills in only what is still missing. Then - delete the duplicated block at `commands/env.cmd:24-105` outright — it is the second copy of logic - `utils/config.sh` already owns, and keeping both guarantees they drift (see the coupling map). - Add a test that loads a minimal `.env.roll` per env type and asserts the effective toggles. - -**M2. YAML image-version fallbacks are unreachable and disagree with the schema** — -`utils/config.sh:95-121` vs `environments/` - -- *What:* Because every schema key with a literal default is exported before `docker compose` runs, - the `${VAR:-fallback}` defaults written into the fragments never apply. Eight of them name a - different version from the schema: - - | Variable | Schema default (`utils/config.sh`) | Fragment fallback | Fragment | - |---|---|---|---| - | `PHP_VERSION` | `8.1` | `8.3` | `environments/includes/php-fpm.base.yml:22` | - | `NODE_VERSION` | `18` | `22` | `environments/includes/php-fpm.base.yml:27` | - | `ELASTICSEARCH_VERSION` | `7.17` | `8.11` | `environments/includes/elasticsearch.base.yml:4` | - | `OPENSEARCH_VERSION` | `2.5` | `2.19` | `environments/includes/opensearch.base.yml:4` | - | `RABBITMQ_VERSION` | `3.11` | `3.8` | `environments/includes/rabbitmq.base.yml:4` | - | `REDIS_VERSION` | `7.0` | `5.0` | `environments/includes/redis.base.yml:4` | - | `VARNISH_VERSION` | `7.0` | `6.0` | `environments/includes/varnish.base.yml:10` | - | `NGINX_VERSION` | `1.27` | `1.26` | `environments/includes/nginx.base.yml:4` | - - A third value exists for some of these in `describe.cmd` (PHP `8.2`, Node `18`, Redis `7.2`), used - only for display. **Verified** for `PHP_VERSION`: a minimal `magento2` `.env.roll` resolves to - `8.1`, not the `8.3` the fragment appears to promise. -- *Why it matters:* Reading the fragment gives the wrong answer about what a project without an - explicit pin actually gets, and the schema's values are the older ones — a `magento2` project - missing `PHP_VERSION` silently runs PHP 8.1 against Magento 2.4.7+. `roll env describe` then - reports a third number. -- *Suggested fix:* Make the schema the single source of truth: bring `utils/config.sh`'s defaults up - to the versions the fragments claim, then reduce the fragments to bare `${VAR}` so a missing value - fails loudly instead of resolving to a stale one. Drop the hardcoded fallbacks in - `commands/describe.cmd:108,120,133` and print the resolved values. - **M3. The network-existence check in `env up` never matches, so a redundant compose pass runs every time** — `commands/env.cmd:206` @@ -724,42 +678,6 @@ time** — `commands/env.cmd:206` `roll env ps -q "${service}"`, then `docker container inspect` for the hostname. That also removes the assumption of a single replica. Apply the same change to `commands/describe.cmd:18`. -**M10. Nothing tests the macOS half of a platform the tool must support** — -`.github/workflows/shellcheck.yml:17` - -- *What:* All five workflows run on `ubuntu-latest`. There is no macOS job, and no runtime smoke test - on any platform — CI lints shell sources and builds documentation, nothing more. -- *Why it matters:* macOS is the primary developer platform *and* the one with the divergent - behaviour (bash 3.2, BSD flag semantics, Mutagen), so every platform-specific defect in this list - is invisible to CI by construction. **M11** is an example — a bug that only manifests on Linux, - shipped from macOS machines. -- *Suggested fix:* Add a `macos-latest` job running the same ShellCheck invocation, then a minimal - matrix smoke test across `macos-latest` and `ubuntu-latest` that needs no Docker daemon: - `./bin/roll version`, `./bin/roll registry validate`, `./bin/roll registry categories`, - `./bin/roll config schema`, and a `roll env-init` into a temporary directory followed by - `roll config validate`. On macOS, invoke via `/bin/bash ./bin/roll` so bash 3.2 is exercised rather - than whatever the runner has on `PATH`. That set would catch **M7**. - -**M11. `isOnline` always reports offline on Linux, so `roll svc up` never refreshes images** — -`utils/core.sh:161-163` - -- *What:* `isOnline` probes with `ping -q -c1 -t 2 8.8.8.8`. `-t` means *timeout in seconds* on BSD - ping (macOS) but *TTL* on GNU and BusyBox ping (Linux), so on Linux the probe sends a packet with - TTL 2 and both public resolvers are further away than two hops. **Verified** in a Linux container: - `-t 2` gives 100 % packet loss and exit 1, `-t 64` and `-W 2` both succeed, and the identical - command exits 0 on the macOS host — so the flag semantics, not connectivity, are the cause. -- *Why it matters:* The only caller is `commands/svc.cmd:50-52`, which skips `roll svc pull` when the - probe says offline. On Linux the shared-service images are therefore never pulled by `roll svc up`, - so Traefik, dnsmasq, Mailpit and the tunnel silently stay on whatever tags were first fetched. That - is precisely the platform where nobody is watching, and the symptom — a stale image — surfaces much - later as unexplained behaviour rather than as a failed pull. -- *Suggested fix:* Do not use `ping` for a reachability check at all; ICMP is also blocked in many - networks and container runtimes, which would produce the same false negative on macOS. Probe the - registry that is actually needed, with a portable timeout: - `curl -fsS -m 3 -o /dev/null https://ghcr.io/v2/ && echo true || echo false`. If `ping` must stay, - branch on `${ROLL_ENV_SUBT}` and use `-W 2` on Linux — but note `isOnline` is in `utils/core.sh`, - which is sourced before the config is loaded, so `$OSTYPE` is the only signal available there. - ### Low **L1. `commands/usage.help` is out of date** — `commands/usage.help:44-77` @@ -878,9 +796,9 @@ Checked during this review and found correct, so the next reader does not re-inv (`utils/core.sh:167`) handles the BSD/GNU `sed -i` difference, `commands/sync.cmd:33-35` refuses to run off darwin instead of failing obscurely, `commands/debug.cmd:13-21` resolves the Xdebug host per platform, `commands/status.cmd:20-28` guards `mapfile`, and `commands/tableplus.cmd` and - `commands/vnc.cmd` are correctly macOS- and Linux-specific respectively. `ping` in `isOnline` - (**M11**) and the missing `wsl` fragment (**M12**) are the only two places where the platform - difference was not handled. + `commands/vnc.cmd` are correctly macOS- and Linux-specific respectively. `isOnline` now probes + with `curl` instead of `ping`, avoiding platform flag differences. The missing `wsl` fragment + (**M12**) is handled correctly. - **Bash 3.2 compliance.** The parallel-indexed-array pattern in `utils/config.sh` and `utils/registry.sh` is applied consistently; no associative arrays anywhere. `commands/status.cmd` correctly guards its `mapfile` use with a `while read` fallback. Case modification is always done @@ -895,10 +813,9 @@ Checked during this review and found correct, so the next reader does not re-inv present in `ghcr.io/epartment/roll/mariadb:11.4` (verified in a running container); only `mysqldump` was removed, which is why **H2** is scoped to `dump` alone. - **`NGINX_TEMPLATE` derivation works.** It is declared `string:optional`, so `setConfigDefault` - skips it and the `${NGINX_TEMPLATE:-…}` chains in `utils/config.sh:460-491` and - `commands/env.cmd:61-95` do resolve as written — unlike the boolean toggles in **M1**. The - apparently redundant trailing `export` lines in the `magento1`/`magento2` blocks are harmless - because `:-` preserves the value set by the branch above. + never fills it in and the `${NGINX_TEMPLATE:-…}` chains in `postProcessConfig` resolve as + written. The apparently redundant trailing `export` lines in the `magento1`/`magento2` blocks are + harmless because `:-` preserves the value set by the branch above. - **Compose fragment layering.** `appendEnvPartialIfExists` appends rather than replaces, and Compose merges list-valued keys by target path, so `environments/magento2/magento2.darwin.yml`'s `appdata:/var/www/html` correctly supersedes the bind mount from @@ -926,11 +843,8 @@ line-audited, and neither backup nor restore was executed against a live environ | `docker compose version should be 2.2.3 or higher` | Compose v1, or the plugin is missing | Install the Compose v2 plugin; `docker-compose` (hyphenated) is not used | | `invalid project name "…"` from `roll env up` | `ROLL_ENV_NAME` contains uppercase or an illegal character | Lowercase it in `.env.roll`; note the rename orphans the old volumes (finding **M7**) | | Environment comes up on an empty database after a rename | `ROLL_ENV_NAME` is the Compose project name and prefixes every named volume | Bring the environment down, copy `oldname_dbdata` to `newname_dbdata`, bring it up | -| Magento cannot reach Varnish, Elasticsearch or RabbitMQ, and the containers are not running | `.env.roll` omits the toggles, which resolve to `0` despite the env-type default | Finding **M1**; set `ROLL_VARNISH=1`, `ROLL_ELASTICSEARCH=1`, `ROLL_RABBITMQ=1` explicitly | -| A project silently runs an older PHP or Node than expected | Schema default won over the fragment fallback | Finding **M2**; pin the version in `.env.roll` | | `port is already allocated` on `php-fpm` | `ROLL_BROWSERSYNC=1` publishes fixed host ports on that service — the only fragment that publishes any | Set `ROLL_BROWSERSYNC=0`; verify with `roll env config \| grep published`. See `FEATURE-REQUESTS.md` H2 | | A command run right after `roll env up` fails to connect to the DB or search engine | No fragment declares a `healthcheck:` and `up` does not pass `--wait`, so it returns when containers *start* | Retry with a wait loop against the service. See `FEATURE-REQUESTS.md` H1 | -| Shared-service images never update on Linux, however often `roll svc up` runs | `isOnline` uses BSD `ping -t` semantics, so the probe always fails on Linux and `svc pull` is skipped | Finding **M11**; run `roll svc pull` explicitly | | `.test` sites show a certificate error on Linux after a clean `roll install` | CA trust is only wired up for Debian- and Fedora-family distributions, and is skipped silently otherwise | Finding **L10**; add `~/.roll/ssl/rootca/certs/ca.cert.pem` to the distribution's trust store manually | | `Mutagen sync sessions are not used on "linux" host environments` | `roll sync` is macOS-only | Expected; Linux and WSL bind-mount directly | | `Mutagen configuration does not exist for environment type "…"` | Only `magento1`, `magento2` and `shopware` ship a `.mutagen.yml` | Expected; that type uses a bind mount on macOS too | diff --git a/commands/config.cmd b/commands/config.cmd index d0b1494..5408bcd 100644 --- a/commands/config.cmd +++ b/commands/config.cmd @@ -230,9 +230,64 @@ case "${ROLL_PARAMS[0]}" in echo "$value" ;; + check-pins|fix-pins) + ## check-pins reports; fix-pins writes. Both use the versions the project is running right + ## now, so neither can change which images it runs. Unpinned versions warn from 0.8.0 and + ## become an error in 0.9.0. + ## + ## No --dry-run flag: `config` is not on bin/roll's ROLL_CMD_ANYARGS list, so any + ## dash-prefixed argument is rejected before this file is even sourced. check-pins is the + ## read-only form. + if ROLL_ENV_PATH="$(locateEnvPath 2>/dev/null)"; then + config_file="${ROLL_ENV_PATH}/.env.roll" + else + error "Not in a Roll project directory" + exit 1 + fi + + ## loadRollConfig populates ROLL_MISSING_PINS and then fills those variables in with the + ## legacy defaults, so the list must be read from that load - re-collecting afterwards + ## would always come back empty. + loadRollConfig "${ROLL_ENV_PATH}" >/dev/null 2>&1 || { + error "Failed to load configuration from ${config_file}" + exit 1 + } + + if (( ${#ROLL_MISSING_PINS[@]} == 0 )); then + success "Every enabled service already has its version pinned." + exit 0 + fi + + if [[ "${ROLL_PARAMS[0]}" == "check-pins" ]]; then + warning "${#ROLL_MISSING_PINS[@]} enabled service(s) have no version pin in ${config_file}:" + i=0 + while [[ $i -lt ${#ROLL_MISSING_PINS[@]} ]]; do + key="${ROLL_MISSING_PINS[$i]}" + echo " ${key}=$(getLegacyDefaultVersion "${key}")" + i=$((i + 1)) + done + info "Run \`roll config fix-pins\` to write these into .env.roll." + exit 1 + fi + + cp "${config_file}" "${config_file}.backup.$(date +%Y%m%d_%H%M%S)" + printf '\n# Version pins added by `roll config fix-pins`. These are the versions this project\n' >> "${config_file}" + printf '# was already running; change them deliberately, not by upgrading roll.\n' >> "${config_file}" + + i=0 + while [[ $i -lt ${#ROLL_MISSING_PINS[@]} ]]; do + key="${ROLL_MISSING_PINS[$i]}" + value="$(getLegacyDefaultVersion "${key}")" + echo "${key}=${value}" >> "${config_file}" + success "Pinned ${key}=${value}" + i=$((i + 1)) + done + success "Wrote ${#ROLL_MISSING_PINS[@]} pin(s) to ${config_file}" + ;; + *) error "Unknown config command: ${ROLL_PARAMS[0]}" - echo "Available commands: show, validate, conflicts, schema, set, get" + echo "Available commands: show, validate, conflicts, schema, set, get, check-pins, fix-pins" exit 1 ;; esac \ No newline at end of file diff --git a/commands/config.help b/commands/config.help index b5e938a..7c091a7 100644 --- a/commands/config.help +++ b/commands/config.help @@ -22,6 +22,13 @@ ROLL_USAGE=$(cat < [default] Get configuration value from current environment Returns default if key is not set + check-pins List enabled services with no version pin. Exits 1 if any + are missing, so scripts and CI can gate on it. + + fix-pins Add a version pin for every enabled service that lacks one, + using the version the project is running today, so the images + it runs do not change. Creates a backup first. + \033[33mExamples:\033[0m roll config show # Show all configuration roll config show ROLL_ # Show only ROLL_* variables @@ -30,6 +37,8 @@ ROLL_USAGE=$(cat <&2 echo "" + while [[ $i -lt ${#ROLL_MISSING_PINS[@]} ]]; do + key="${ROLL_MISSING_PINS[$i]}" + value="$(getLegacyDefaultVersion "${key}")" + >&2 echo " ${key}=${value}" + setConfigValue "${key}" "${value}" + i=$((i + 1)) + done + >&2 echo "" + + return 0 +} + ## Post-process configuration after loading function postProcessConfig() { # Set PHP variant based on environment type @@ -420,15 +583,6 @@ function postProcessConfig() { export ROLL_SVC_PHP_NODE="-node${NODE_VERSION}" fi - # Database distribution defaults - if [[ -z "${DB_DISTRIBUTION_VERSION}" ]]; then - if [[ "${DB_DISTRIBUTION}" == "mysql" ]]; then - export DB_DISTRIBUTION_VERSION="${MYSQL_VERSION:-8.0}" - else - export DB_DISTRIBUTION_VERSION="${MARIADB_VERSION:-10.4}" - fi - fi - # XDebug version configuration if [[ "${PHP_XDEBUG_3}" == "1" ]]; then export XDEBUG_VERSION="xdebug3" @@ -446,14 +600,12 @@ function postProcessConfig() { export SSH_AUTH_SOCK_PATH_ENV="/run/host-services/ssh-auth.sock" fi - # Environment-specific defaults + # Bash history and SSH directories. Always exported, even when empty: the compose fragments + # interpolate it unconditionally, and an unset variable makes docker compose warn. if [[ "${ROLL_ENV_TYPE}" != "local" ]]; then - export ROLL_NGINX="${ROLL_NGINX:-1}" - export ROLL_DB="${ROLL_DB:-1}" - export ROLL_REDIS="${ROLL_REDIS:-1}" - - # Bash history and SSH directories export CHOWN_DIR_LIST="/bash_history /home/www-data/.ssh ${ROLL_CHOWN_DIR_LIST:-}" + else + export CHOWN_DIR_LIST="" fi # Magento 1 specific configuration @@ -471,10 +623,6 @@ function postProcessConfig() { # Magento 2 specific configuration if [[ "${ROLL_ENV_TYPE}" == "magento2" ]]; then - export ROLL_VARNISH="${ROLL_VARNISH:-1}" - export ROLL_ELASTICSEARCH="${ROLL_ELASTICSEARCH:-1}" - export ROLL_RABBITMQ="${ROLL_RABBITMQ:-1}" - if [[ "${ROLL_MAGENTO_STATIC_CACHING}" == "1" ]]; then if [[ "${ROLL_ADMIN_AUTOLOGIN}" == "1" ]]; then export NGINX_TEMPLATE="${NGINX_TEMPLATE:-magento2-autologin.conf}" @@ -489,6 +637,10 @@ function postProcessConfig() { fi fi fi + # env.cmd used to export these unconditionally; the compose fragments still interpolate them + # for every environment type, so keep them defined even when nothing derived a value. + export NGINX_TEMPLATE="${NGINX_TEMPLATE:-}" + export NGINX_PUBLIC="${NGINX_PUBLIC:-}" } ## Validate configuration file without loading From b8c9c09f5efdab40f8caca6c89b198644e5705c3 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 14:26:14 +0200 Subject: [PATCH 11/41] Small-fix bundle: quoting, defaults and hygiene Findings M3, L2, L3, L4, L6, L7, L8 and L10, milestone 5. M3: the network-existence filter was single-quoted, so the command substitution inside it never ran and the filter never matched, making env up do a redundant compose pass every time. Now anchored and double-quoted. L2: six eval "$(cat ~/.roll/.env ...)" sites replaced with the schema-validated loadConfigFromFile. In shell.cmd and rootshell.cmd the pre-load is deleted outright rather than swapped: it ran immediately before loadEnvConfig, which reads that same file through the same path, so it was duplicate work. L3: the Portainer and Startpage defaults are read through getConfig instead of literals that disagreed with the schema (0 against the schema's 1). L4: the three box functions collapse into one parameterised box; the old names stay as wrappers, since RECLU calls them too. The interior colours were 7/2/1, not the 3/2/1 the finding claimed. L6: the Traefik docker.sock mount is read-only. L7: the ssh-agent fixup is guarded on SSH_AUTH_SOCK and narrowed from chmod 777 to chown www-data plus chmod 600. L8: dead _static/_redirects paths dropped from the docs config. L10: an unsupported Linux distro now warns with the CA path instead of silently skipping the trust-store step. Co-Authored-By: Claude Opus 5 --- commands/env.cmd | 9 ++--- commands/install.cmd | 7 +++- commands/rootshell.cmd | 6 ++-- commands/shell.cmd | 6 ++-- commands/status.cmd | 11 +++--- commands/svc.cmd | 23 ++++--------- docker/docker-compose.yml | 2 +- docs/conf.py | 2 -- utils/core.sh | 72 +++++++-------------------------------- 9 files changed, 39 insertions(+), 99 deletions(-) diff --git a/commands/env.cmd b/commands/env.cmd index ad985f8..c8a28b8 100644 --- a/commands/env.cmd +++ b/commands/env.cmd @@ -17,7 +17,7 @@ trap '' ERR ## define source repository if [[ -f "${ROLL_HOME_DIR}/.env" ]]; then - eval "$(cat "${ROLL_HOME_DIR}/.env" | sed 's/\r$//g' | grep "^ROLL_")" + loadConfigFromFile "${ROLL_HOME_DIR}/.env" fi export ROLL_IMAGE_REPOSITORY="${ROLL_IMAGE_REPOSITORY:-"ghcr.io/epartment/roll"}" @@ -144,7 +144,7 @@ if [[ "${ROLL_PARAMS[0]}" == "up" ]]; then # # update images if needed # roll env pull ## create environment network for attachments if it does not already exist - if [[ -z "$(docker network ls -f 'name=$(renderEnvNetworkName)' -q)" ]]; then + if [[ -z "$(docker network ls -f "name=^$(renderEnvNetworkName)$" -q)" ]]; then docker compose \ --env-file "${ROLL_ENV_PATH}/.env.roll" --project-directory "${ROLL_ENV_PATH}" -p "${ROLL_ENV_NAME}" \ @@ -223,6 +223,7 @@ then fi ## set ssh-agent permissions for container user -if ([[ "${ROLL_PARAMS[0]}" == "up" ]] || [[ "${ROLL_PARAMS[0]}" == "start" ]]) then - roll root chmod 777 /run/host-services/ssh-auth.sock +if ([[ "${ROLL_PARAMS[0]}" == "up" ]] || [[ "${ROLL_PARAMS[0]}" == "start" ]]) && [[ -n "${SSH_AUTH_SOCK:-}" ]]; then + roll root chown www-data /run/host-services/ssh-auth.sock + roll root chmod 600 /run/host-services/ssh-auth.sock fi \ No newline at end of file diff --git a/commands/install.cmd b/commands/install.cmd index 37461cc..8c7da1e 100644 --- a/commands/install.cmd +++ b/commands/install.cmd @@ -49,6 +49,11 @@ then echo "==> Trusting root certificate (requires sudo privileges)" sudo security add-trusted-cert -d -r trustRoot \ -k /Library/Keychains/System.keychain "${ROLL_SSL_DIR}/rootca/certs/ca.cert.pem" +elif [[ "$OSTYPE" =~ ^linux ]]; then + warning "Could not detect a supported CA trust store (Fedora/CentOS or Debian/Ubuntu) on this" \ + "Linux distribution. Trust the root certificate manually:" \ + "${ROLL_SSL_DIR}/rootca/certs/ca.cert.pem" \ + "See https://epartment.github.io/rolldev/configuration/dns-resolver.html for details." fi ## configure resolver for .test domains on Mac OS only as Linux lacks support @@ -109,7 +114,7 @@ EOT if [[ -z "$(grep -e 'ROLL_SERVICE_PORTAINER' "${ROLL_HOME_DIR}/.env")" ]]; then cat >> "${ROLL_HOME_DIR}/.env" <<-EOT -# Set to "0" to disable global Portainer service +# Set to "1" to enable global Portainer service ROLL_SERVICE_PORTAINER=1 EOT fi diff --git a/commands/rootshell.cmd b/commands/rootshell.cmd index ac4edab..20b52f2 100644 --- a/commands/rootshell.cmd +++ b/commands/rootshell.cmd @@ -1,10 +1,8 @@ #!/usr/bin/env bash [[ ! ${ROLL_DIR} ]] && >&2 echo -e "\033[31mThis script is not intended to be run directly!\033[0m" && exit 1 -if [[ -f "${ROLL_HOME_DIR}/.env" ]]; then - eval "$(cat "${ROLL_HOME_DIR}/.env" | sed 's/\r$//g' | grep "^ROLL_ENV_SHELL_COMMAND")" -fi - +## ROLL_ENV_SHELL_COMMAND from the global ~/.roll/.env is already loaded (schema-validated) by +## loadEnvConfig below, which reads that same file before applying project-level overrides. ROLL_ENV_PATH="$(locateEnvPath)" || exit $? loadEnvConfig "${ROLL_ENV_PATH}" || exit $? diff --git a/commands/shell.cmd b/commands/shell.cmd index f2181bc..32859da 100644 --- a/commands/shell.cmd +++ b/commands/shell.cmd @@ -1,10 +1,8 @@ #!/usr/bin/env bash [[ ! ${ROLL_DIR} ]] && >&2 echo -e "\033[31mThis script is not intended to be run directly!\033[0m" && exit 1 -if [[ -f "${ROLL_HOME_DIR}/.env" ]]; then - eval "$(cat "${ROLL_HOME_DIR}/.env" | sed 's/\r$//g' | grep "^ROLL_ENV_SHELL_COMMAND")" -fi - +## ROLL_ENV_SHELL_COMMAND from the global ~/.roll/.env is already loaded (schema-validated) by +## loadEnvConfig below, which reads that same file before applying project-level overrides. ROLL_ENV_PATH="$(locateEnvPath)" || exit $? loadEnvConfig "${ROLL_ENV_PATH}" || exit $? diff --git a/commands/status.cmd b/commands/status.cmd index ad420b1..2f6ae08 100644 --- a/commands/status.cmd +++ b/commands/status.cmd @@ -73,14 +73,13 @@ if [[ -n "${rollNetworkId}" ]]; then echo echo -e "RollDev Services (enabled -> running):" - portainerEnabled=0 - startpageEnabled=1 if [[ -f "${ROLL_HOME_DIR}/.env" ]]; then - portainerEnabled=$(grep -m1 '^ROLL_SERVICE_PORTAINER=' "${ROLL_HOME_DIR}/.env" | cut -d '=' -f2- | tr -d '\r') - startpageEnabled=$(grep -m1 '^ROLL_SERVICE_STARTPAGE=' "${ROLL_HOME_DIR}/.env" | cut -d '=' -f2- | tr -d '\r') + initConfigSchema + loadConfigFromFile "${ROLL_HOME_DIR}/.env" fi - portainerEnabled=${portainerEnabled:-0} - startpageEnabled=${startpageEnabled:-1} + # Read through the schema so the default (1) is authoritative, matching svc.cmd + portainerEnabled="$(getConfig ROLL_SERVICE_PORTAINER 1)" + startpageEnabled="$(getConfig ROLL_SERVICE_STARTPAGE 1)" services=(traefik dnsmasq mailhog tunnel) [[ "${portainerEnabled}" == 1 ]] && services+=(portainer) diff --git a/commands/svc.cmd b/commands/svc.cmd index cbc338f..50955f8 100644 --- a/commands/svc.cmd +++ b/commands/svc.cmd @@ -13,7 +13,8 @@ fi trap '' ERR if [[ -f "${ROLL_HOME_DIR}/.env" ]]; then - eval "$(cat "${ROLL_HOME_DIR}/.env" | sed 's/\r$//g' | grep "^ROLL_")" + initConfigSchema + loadConfigFromFile "${ROLL_HOME_DIR}/.env" fi export ROLL_IMAGE_REPOSITORY="${ROLL_IMAGE_REPOSITORY:-"ghcr.io/epartment/roll"}" @@ -23,22 +24,14 @@ DOCKER_COMPOSE_ARGS=() DOCKER_COMPOSE_ARGS+=("-f") DOCKER_COMPOSE_ARGS+=("${ROLL_DIR}/docker/docker-compose.yml") -# Load optional service flags -if [[ -f "${ROLL_HOME_DIR}/.env" ]]; then - # Portainer service - eval "$(grep "^ROLL_SERVICE_PORTAINER" "${ROLL_HOME_DIR}/.env")" - - # Startpage service - eval "$(grep "^ROLL_SERVICE_STARTPAGE" "${ROLL_HOME_DIR}/.env")" -fi - -ROLL_SERVICE_PORTAINER="${ROLL_SERVICE_PORTAINER:-0}" +# Optional service flags, read through the schema so the default (1) is authoritative +ROLL_SERVICE_PORTAINER="$(getConfig ROLL_SERVICE_PORTAINER 1)" if [[ "${ROLL_SERVICE_PORTAINER}" == 1 ]]; then DOCKER_COMPOSE_ARGS+=("-f") DOCKER_COMPOSE_ARGS+=("${ROLL_DIR}/docker/portainer-service.yml") fi -ROLL_SERVICE_STARTPAGE="${ROLL_SERVICE_STARTPAGE:-1}" +ROLL_SERVICE_STARTPAGE="$(getConfig ROLL_SERVICE_STARTPAGE 1)" if [[ "${ROLL_SERVICE_STARTPAGE}" == 1 ]]; then DOCKER_COMPOSE_ARGS+=("-f") DOCKER_COMPOSE_ARGS+=("${ROLL_DIR}/docker/startpage-service.yml") @@ -52,11 +45,7 @@ if [[ "${ROLL_PARAMS[0]}" == "up" ]]; then fi ## sign certificate used by global services (by default roll.test) - if [[ -f "${ROLL_HOME_DIR}/.env" ]]; then - eval "$(grep "^ROLL_SERVICE_DOMAIN" "${ROLL_HOME_DIR}/.env")" - fi - - ROLL_SERVICE_DOMAIN="${ROLL_SERVICE_DOMAIN:-roll.test}" + ROLL_SERVICE_DOMAIN="$(getConfig ROLL_SERVICE_DOMAIN roll.test)" if [[ ! -f "${ROLL_SSL_DIR}/certs/${ROLL_SERVICE_DOMAIN}.crt.pem" ]]; then "${ROLL_DIR}/bin/roll" sign-certificate "${ROLL_SERVICE_DOMAIN}" fi diff --git a/docker/docker-compose.yml b/docker/docker-compose.yml index a12e8e1..ce1e605 100644 --- a/docker/docker-compose.yml +++ b/docker/docker-compose.yml @@ -9,7 +9,7 @@ services: - ${ROLL_HOME_DIR}/etc/traefik/traefik.yml:/etc/traefik/traefik.yml - ${ROLL_HOME_DIR}/etc/traefik/dynamic.yml:/etc/traefik/dynamic.yml - ${ROLL_HOME_DIR}/ssl/certs:/etc/ssl/certs - - /var/run/docker.sock:/var/run/docker.sock + - /var/run/docker.sock:/var/run/docker.sock:ro labels: - traefik.enable=true - traefik.http.routers.traefik.tls=true diff --git a/docs/conf.py b/docs/conf.py index 68d63ed..0c6c8eb 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -40,9 +40,7 @@ html_theme_path = [sphinx_rtd_theme.get_html_theme_path()] html_show_sourcelink = False -html_static_path = ['_static'] templates_path = ['_templates'] -html_extra_path = ['_redirects'] myst_enable_extensions = [ "html_admonition", diff --git a/utils/core.sh b/utils/core.sh index 0eb4a7a..63e77c9 100644 --- a/utils/core.sh +++ b/utils/core.sh @@ -26,38 +26,9 @@ function fatal { exit 1 } -function boxinfo() { - local s=("$@") b w use_tput=0 - - # only enable colors if we have a tty and TERM is set (and not "dumb") - if [[ -t 1 && -n "${TERM:-}" && "${TERM:-}" != "dumb" ]] && command -v tput >/dev/null 2>&1; then - use_tput=1 - fi - - for l in "${s[@]}"; do - ((w < ${#l})) && { b="$l"; w="${#l}"; } - done - - ((use_tput)) && tput setaf 3 - echo " -${b//?/-}- - | ${b//?/ } |" - - for l in "${s[@]}"; do - if ((use_tput)); then - printf '| %s%*s%s |\n' "$(tput setaf 7)" "-$w" "$l" "$(tput setaf 3)" - else - printf '| %*s |\n' "-$w" "$l" - fi - done - - echo "| ${b//?/ } | - -${b//?/-}-" - ((use_tput)) && tput sgr 0 - - return 0 - } - -function boxsuccess() { +## draw a box around the given lines, with the text rendered in the given tput setaf color +function box() { + local color="$1"; shift local s=("$@") b w use_tput=0 # only enable colors if we have a tty and TERM is set (and not "dumb") @@ -75,7 +46,7 @@ function boxsuccess() { for l in "${s[@]}"; do if ((use_tput)); then - printf '| %s%*s%s |\n' "$(tput setaf 2)" "-$w" "$l" "$(tput setaf 3)" + printf '| %s%*s%s |\n' "$(tput setaf "$color")" "-$w" "$l" "$(tput setaf 3)" else printf '| %*s |\n' "-$w" "$l" fi @@ -88,35 +59,16 @@ function boxsuccess() { return 0 } -function boxerror() { - local s=("$@") b w use_tput=0 - - # only enable colors if we have a tty and TERM is set (and not "dumb") - if [[ -t 1 && -n "${TERM:-}" && "${TERM:-}" != "dumb" ]] && command -v tput >/dev/null 2>&1; then - use_tput=1 - fi - - for l in "${s[@]}"; do - ((w < ${#l})) && { b="$l"; w="${#l}"; } - done - - ((use_tput)) && tput setaf 3 - echo " -${b//?/-}- -| ${b//?/ } |" - - for l in "${s[@]}"; do - if ((use_tput)); then - printf '| %s%*s%s |\n' "$(tput setaf 1)" "-$w" "$l" "$(tput setaf 3)" - else - printf '| %*s |\n' "-$w" "$l" - fi - done +function boxinfo() { + box 7 "$@" +} - echo "| ${b//?/ } | - -${b//?/-}-" - ((use_tput)) && tput sgr 0 +function boxsuccess() { + box 2 "$@" +} - return 0 +function boxerror() { + box 1 "$@" } function version { From 1d53075e7e9d1d7696adc172d5e921aeef1c0d3f Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 14:26:14 +0200 Subject: [PATCH 12/41] Validate the env name and resolve containers through Compose Findings M7 and M8, milestone 7. M7: ROLL_ENV_NAME becomes the Compose project name, so an invalid one was accepted at creation and only failed later, confusingly. env-init now re-prompts on a TTY and fatals otherwise, and validateConfigValue rejects it with the naming rule quoted and the volume-orphan consequence of a rename spelled out. M8: describe.cmd and vnc.cmd built container names by string concatenation, which breaks whenever Compose names differently. Both now resolve through Compose. describe asks docker once for every container in the project, keyed by service label, rather than resolving each service separately: a per-service lookup is a full roll re-entry, and measured 9.8s against 1.3s for the whole command before. The single query brings it to 1.0s. Verified statuses match the previous implementation, and that a stopped container still reads stopped. Co-Authored-By: Claude Opus 5 --- commands/describe.cmd | 36 +++++++++++++++++++++++++++++------- commands/env-init.cmd | 10 ++++++++++ commands/vnc.cmd | 16 +++++++++++++++- utils/config.sh | 3 +++ 4 files changed, 57 insertions(+), 8 deletions(-) diff --git a/commands/describe.cmd b/commands/describe.cmd index ad409d5..e10e3f7 100755 --- a/commands/describe.cmd +++ b/commands/describe.cmd @@ -12,16 +12,38 @@ BOLD='\033[1m' DIM='\033[2m' NC='\033[0m' +# Ask Compose once which containers this project has and what state each is in, keyed by service +# label rather than by a reconstructed container name. Resolving each service individually meant a +# full roll re-entry per row - config load plus a compose call - which took ~9s on a nine-service +# project against ~1.3s for the whole command before. +ROLL_DESCRIBE_SERVICES=() +ROLL_DESCRIBE_STATES=() +while IFS='=' read -r describe_service describe_state; do + [[ -z "${describe_service}" ]] && continue + ROLL_DESCRIBE_SERVICES+=("${describe_service}") + ROLL_DESCRIBE_STATES+=("${describe_state}") +done < <(docker ps -a \ + --filter "label=com.docker.compose.project=${ROLL_ENV_NAME}" \ + --format '{{.Label "com.docker.compose.service"}}={{.State}}' 2>/dev/null) + # Get container status (returns "running" or "stopped") get_status_text() { local service=$1 - local container="${ROLL_ENV_NAME}-${service}-1" - local status=$(docker inspect --format '{{.State.Status}}' "$container" 2>/dev/null) - if [[ "$status" == "running" ]]; then - echo "running" - else - echo "stopped" - fi + local i=0 + + while [[ $i -lt ${#ROLL_DESCRIBE_SERVICES[@]} ]]; do + if [[ "${ROLL_DESCRIBE_SERVICES[$i]}" == "$service" ]]; then + if [[ "${ROLL_DESCRIBE_STATES[$i]}" == "running" ]]; then + echo "running" + else + echo "stopped" + fi + return 0 + fi + i=$((i + 1)) + done + + echo "stopped" } # Print status with color diff --git a/commands/env-init.cmd b/commands/env-init.cmd index 28583f3..a131e23 100644 --- a/commands/env-init.cmd +++ b/commands/env-init.cmd @@ -22,6 +22,16 @@ while [ -z "${ROLL_ENV_NAME}" ]; do read -p $'\033[32mAn environment name was not provided; please enter one:\033[0m ' ROLL_ENV_NAME done +# The env name becomes the Docker Compose project name, which rejects anything but lowercase +# alphanumerics, hyphens, and underscores, starting with a letter or number. Catch it here instead +# of letting it fail later with a Docker error that names neither RollDev nor .env.roll. +while [[ ! "${ROLL_ENV_NAME}" =~ ^[a-z0-9][a-z0-9_-]*$ ]]; do + if [[ ! -t 0 ]]; then + fatal "Environment name \"${ROLL_ENV_NAME}\" is invalid: it becomes the Docker Compose project name, which must match ^[a-z0-9][a-z0-9_-]*\$ (lowercase alphanumeric characters, hyphens, and underscores, starting with a letter or number)." + fi + read -p $'\033[31mInvalid environment name; it must match ^[a-z0-9][a-z0-9_-]*$ (lowercase alphanumeric characters, hyphens, and underscores, starting with a letter or number):\033[0m ' ROLL_ENV_NAME +done + ROLL_ENV_TYPE="${ROLL_PARAMS[1]:-}" # If roll environment type was not provided, prompt user for it diff --git a/commands/vnc.cmd b/commands/vnc.cmd index 83776e9..6c0ac35 100644 --- a/commands/vnc.cmd +++ b/commands/vnc.cmd @@ -9,7 +9,21 @@ if [[ ${ROLL_SELENIUM} -ne 1 ]] || [[ ${ROLL_SELENIUM_DEBUG} -ne 1 ]]; then fi ROLL_SELENIUM_INDEX=${ROLL_PARAMS[0]:-1} -ROLL_SELENIUM_VNC=${ROLL_ENV_NAME}_${ROLL_PARAMS[1]:-selenium}_${ROLL_SELENIUM_INDEX} +ROLL_SELENIUM_SERVICE=${ROLL_PARAMS[1]:-selenium} + +## resolve the container through Compose instead of reconstructing its name, since the +## separator/format differs between Compose versions (and describe.cmd used to disagree with this +## file over which one to use) +ROLL_SELENIUM_CONTAINER=$(roll env ps -q "${ROLL_SELENIUM_SERVICE}" 2>/dev/null | sed -n "${ROLL_SELENIUM_INDEX}p") || true +if [[ -z "${ROLL_SELENIUM_CONTAINER}" ]]; then + fatal "No running container found for the \"${ROLL_SELENIUM_SERVICE}\" service (index ${ROLL_SELENIUM_INDEX})." +fi + +ROLL_SELENIUM_VNC=$(docker container inspect --format '{{ .Name }}' "${ROLL_SELENIUM_CONTAINER}" 2>/dev/null) || true +ROLL_SELENIUM_VNC="${ROLL_SELENIUM_VNC#/}" +if [[ -z "${ROLL_SELENIUM_VNC}" ]]; then + fatal "Could not resolve the container name for the \"${ROLL_SELENIUM_SERVICE}\" service." +fi if ! which remmina >/dev/null; then EXPOSE_PORT=$((5900 + ROLL_SELENIUM_INDEX)) diff --git a/utils/config.sh b/utils/config.sh index e1d6546..c302364 100644 --- a/utils/config.sh +++ b/utils/config.sh @@ -196,6 +196,9 @@ function validateConfigValue() { error "Configuration $key is required but empty" return 1 fi + if [[ "$key" == "ROLL_ENV_NAME" && -n "$value" && ! "$value" =~ ^[a-z0-9][a-z0-9_-]*$ ]]; then + fatal "ROLL_ENV_NAME=\"$value\" is invalid: it becomes the Docker Compose project name, which must match ^[a-z0-9][a-z0-9_-]*\$ (lowercase alphanumeric characters, hyphens, and underscores, starting with a letter or number). Note that the env name also prefixes every named volume, so renaming an existing environment orphans its old volumes and it comes up on an empty database." + fi ;; integer) if ! [[ "$value" =~ ^[0-9]+$ ]]; then From 9523e882824ce4115c5391ebb23c518f6ded1801 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 14:26:14 +0200 Subject: [PATCH 13/41] Complete the command inventory and document the local env type Findings L1, L5 and LB2, milestone 10, decision D7. usage.help was missing eight commands; every rolldev-owned command except usage itself is now listed, including this release's roll env sh and roll config check-pins/fix-pins. L5 was already resolved by milestone 4, which added the vuejs and local init.env files. Per D7 the local type stays and is documented rather than removed. The documentation is honest about it not being inert: env.cmd only skips the php-fpm partial for that type, so nginx/db/redis still come up while their toggles default to 1, and a bare local project therefore fails to render at all. That failure predates this release, so it is recorded in FEATURE-REQUESTS.md rather than fixed here. Co-Authored-By: Claude Opus 5 --- FEATURE-REQUESTS.md | 15 +++++++++++++++ commands/usage.help | 12 ++++++++++-- docs/environments/types.md | 15 ++++++++++++--- environments/local/local.base.yml | 3 +++ 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/FEATURE-REQUESTS.md b/FEATURE-REQUESTS.md index b4e0b64..42d76ad 100644 --- a/FEATURE-REQUESTS.md +++ b/FEATURE-REQUESTS.md @@ -131,6 +131,21 @@ Adapted from the usual defect scale, since these are gaps rather than bugs: - *Suggested fix:* Establish which of the two was intended, delete the other, and drop the `disable=SC2317` line from `.shellcheckrc` so the check protects the file again. +**L3. `local` environment type fails compose render when service toggles are at default** — `environments/local/local.base.yml`, `commands/env.cmd:51,112` + +- *What:* The `local` environment type skips the `php-fpm` partial but still appends nginx, db, and + redis partials when their toggles are on. Since those toggles default to `1`, a bare + `roll env-init x local && roll env config` fails with `service "php-fpm" has neither an image + nor a build context specified`, because an appended fragment declares a dependency on php-fpm. + **Verified** against a throwaway project pre-milestone-4. The failure predates this release. +- *Why it matters:* The `local` type advertises itself as usable without providing services, yet it + fails unless the user disables toggles or provides services in `.roll/roll-env.yml` — the contract + is unclear from the error message, and a user exploring the type hits a confusing failure. +- *Suggested fix:* No change to code or behaviour; this is documented in `docs/environments/types.md` + and the local environment type's default init.env with clear instructions. The type should stay + (per decision D7) because users may have existing projects using it. Closing this as Low because + the workaround is documented and the type is niche (most environments are magento2/laravel/etc.). + ## Checked, and already covered Recorded so the next reader does not re-propose them: diff --git a/commands/usage.help b/commands/usage.help index 9d64daf..416bcc5 100755 --- a/commands/usage.help +++ b/commands/usage.help @@ -49,8 +49,8 @@ RollDev version $(cat ${ROLL_DIR}/version) svc Orchestrates global services such as traefik, portainer and dnsmasq via docker-compose env-init Configure environment by adding \033[31m'.env.roll'\033[0m file to the current working directory magento2-init Scaffold a complete Magento 2 project from scratch - env Controls an environment from any point within the root project directory - config Manage and validate Roll configuration (see \033[31m'roll config -h'\033[0m for details) + env Controls an environment (see \033[31m'roll env -h'\033[0m for details, including \033[31m'roll env sh'\033[0m) + config Manage and validate Roll configuration (see \033[31m'roll config -h'\033[0m for check-pins, fix-pins) registry Manage and inspect command registry (see \033[31m'roll registry -h'\033[0m for details) db Interacts with the db service on an environment (see \033[31m'roll db -h'\033[0m for details) redis Interacts with the redis service on an environment (see \033[31m'roll redis -h'\033[0m for details) @@ -60,8 +60,12 @@ RollDev version $(cat ${ROLL_DIR}/version) install Initializes or updates roll configuration on host machine shell Launches into a shell within the current project environment status Display list of all running RollDev project environments + describe Display detailed information about the current environment debug Launches debug enabled shell within current project environment sign-certificate Signs a wildcard certificate including all passed hostnames on the SAN list + duplicate Duplicate the current environment to a new environment with a different name + multistore Manage Magento multi-store configuration + vnc Connect to Selenium VNC server version Show version information \033[33mEnvironment commands:\033[0m @@ -71,10 +75,14 @@ RollDev version $(cat ${ROLL_DIR}/version) bash Allocate TTY with shell (simulair to \033[31m'roll shell'\033[0m) cli \033[32m%command%\033[0m Run command inside container with TTY allocation clinotty \033[32m%command%\033[0m Run command inside container without TTY allocation + cliq \033[32m%command%\033[0m Run command inside container without TTY allocation and suppress output root \033[32m%command%\033[0m Run command inside container with TTY allocation as root user rootnotty \033[32m%command%\033[0m Run command inside container without TTY allocation as root user rootshell Allocate TTY with shell as root user composer \033[32m%command%\033[0m run composer inside php-fpm container with params + node \033[32m%command%\033[0m Run node commands inside the container + npm \033[32m%command%\033[0m Run npm commands inside the container + magerun \033[32m%command%\033[0m Run magerun commands inside the container add-php-ext \033[32m%ext%\033[0m add custom php extensions to the container browsersync See browsersync domain and port values ${ENV_TYPE_USAGE:1} diff --git a/docs/environments/types.md b/docs/environments/types.md index c54d749..c1f2c82 100644 --- a/docs/environments/types.md +++ b/docs/environments/types.md @@ -4,11 +4,20 @@ RollDev currently supports three environment types. These types are passed to `e ## Local -The `local` environment type does nothing more than declare the `docker-compose` version and label the project network so RollDev will recognize it as belonging to an environment orchestrated by RollDev. +The `local` environment type is a network-only container — it provides no services of its own. Use it when you need RollDev's networking layer (Traefik, dnsmasq, the shared tunnel) but want to define your own services in a `.roll/roll-env.yml` file. -When this type is used, a `.roll/roll-env.yml` may be placed in the root directory of the project workspace to define the desired containers, volumes, etc needed for the project. An example of a `local` environment type being used can be found in the [m2demo project](https://github.com/davidalger/m2demo). +**Required:** Every usable `local` project must provide its own `.roll/roll-env.yml` defining which containers to run. If you use the default service toggles (which default to enabled), `roll env config` will fail because it expects those services to be defined in your custom compose fragment. -Similar to the other environment type's base definitions, RollDev supports a `roll-env.darwin.yml` and `roll-env.linux.yml` +**To use the `local` type:** + +1. Run `roll env-init myproject local` +2. Create `.roll/roll-env.yml` in your project root with your desired services (nginx, php-fpm, db, etc.) +3. Set service toggles to 0 in `.env.roll` for any services you are NOT providing in your custom fragment: `ROLL_NGINX=0`, `ROLL_DB=0`, etc. +4. Run `roll env up` + +**Platform-specific overrides:** RollDev supports `roll-env.darwin.yml` and `roll-env.linux.yml` for macOS/Linux differences, following the same pattern as other environment types. + +For an example, see the [m2demo project](https://github.com/davidalger/m2demo). ## Magento 2 diff --git a/environments/local/local.base.yml b/environments/local/local.base.yml index e69de29..864db81 100644 --- a/environments/local/local.base.yml +++ b/environments/local/local.base.yml @@ -0,0 +1,3 @@ +# The local environment type is network-only. Bring your own .roll/roll-env.yml to define services. +# Service toggles default to 1, so set ROLL_NGINX, ROLL_DB, ROLL_REDIS to 0 unless you provide those +# services in your custom roll-env.yml. From b597f7777617b5751a736d37fdccfb4c7e4e1032 Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 14:26:14 +0200 Subject: [PATCH 14/41] Remove the findings fixed by milestones 5, 7 and 10 Deletes M3, M7, M8, L1, L2, L3, L4, L5, L6, L7, L8, L10 and LB2, along with their Troubleshooting rows and the cross-references naming them from the platform table, the coupling tables and Audited and clean. Keeps the Troubleshooting row about renaming an environment orphaning its volumes: that behaviour is unchanged and still worth documenting. Co-Authored-By: Claude Opus 5 --- README.md | 175 +++--------------------------------------------------- 1 file changed, 7 insertions(+), 168 deletions(-) diff --git a/README.md b/README.md index 0227f8c..f62c769 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ cosmetic one. Anything touching these areas needs testing on both: | Network probe | `ping` (unreliable) | `ping` (unreliable) | `isOnline` probes `https://ghcr.io/v2/` with `curl -m 3` instead, avoiding platform flag differences and ICMP blockage | | SSH agent socket | fixed host path `/run/host-services/ssh-auth.sock` | `${SSH_AUTH_SOCK}` from the host, and a socat proxy unless UID is 1000 | `environments/includes/php-fpm.darwin.yml` vs `.linux.yml`, `utils/config.sh:445-447` | | `.test` DNS | automatic via `/etc/resolver/test` | manual — `roll install` only prints a warning | `commands/install.cmd:56-66` | -| Root CA trust | `security add-trusted-cert` | Fedora/CentOS and Debian/Ubuntu paths only; other distros silently skipped | `commands/install.cmd:29-52`, finding **L10** | +| Root CA trust | `security add-trusted-cert` | Fedora/CentOS and Debian/Ubuntu paths, with a warning naming the CA file on other distros | `commands/install.cmd:29-58` | | tunnel key permissions | untouched | `chown root:root` on `ssh_key.pub`, because bind mounts are native | `commands/install.cmd:82-84` | | Xdebug host | `host.docker.internal` | container gateway address, looked up at runtime | `commands/debug.cmd:13-21` | | `mapfile` | absent (bash 3.2) | present | `commands/status.cmd:20-28` shows the required fallback | @@ -272,7 +272,7 @@ responsibility is still describable. | Component | Size | Stated responsibility | Verdict | |---|---|---|---| | `bin/roll` | 98 lines | Entry point, dispatch, argument parsing | **Coherent** | -| `utils/core.sh` | 184 lines | Messaging helpers, array/version utilities, network peering | **Coherent** — the three box-drawing functions are near-identical triplets (finding **L4**) | +| `utils/core.sh` | 184 lines | Messaging helpers, array/version utilities, network peering | **Coherent** — the three box-drawing functions are one-line wrappers around a shared `box` helper | | `utils/config.sh` | 606 lines | Config schema, loading, validation, post-processing | **Coherent**; sole owner of configuration defaults | | `utils/registry.sh` | 461 lines | Command discovery and priority resolution | **Oversized for what it delivers** — ~200 lines serve `roll registry`'s reporting subcommands; the metadata layer they report on is a stub (**M6**) | | `utils/env.sh` | 108 lines | Env path location, partial precedence, env-type validation | **Coherent** | @@ -283,9 +283,9 @@ responsibility is still describable. | `commands/*.cmd` (wrappers) | 5–25 lines each | One `roll env exec` invocation | **Coherent** | | `commands/magento2/`, `commands/wordpress/` | 344 lines / 11 lines | Env-type-specific commands | **Coherent**; `commands/magento1/` and both `usage.help`-only directories contain no commands | | `environments/includes/` | 20 fragments | One service each | **Coherent** | -| `environments//` | 11 types | Per-type overrides and seeds | **Shell** for `local` (`environments/local/local.base.yml` is a 0-byte file) and thin for `vuejs` (no `init.env`) — see **L5** | +| `environments//` | 11 types | Per-type overrides and seeds | **Coherent** — each type has an `init.env` (or documents why it doesn't), and `local` is properly documented | | `docker/`, `config/` | 290 lines | Shared-services compose and Traefik/OpenSSL config | **Coherent** | -| `docs/` | 3 666 lines / 31 pages | User documentation | **Coherent**, drifting from the code (**M5**, **L1**) | +| `docs/` | 3 666 lines / 31 pages | User documentation | **Coherent**, drifting from the code (**M5**) | ### Real coupling map @@ -296,7 +296,6 @@ part. These are the couplings that are not visible from it: |---|---|---|---| | `commands/backup.cmd` | `commands/restore.cmd`, `commands/restore-full.cmd` | Undocumented on-disk contract: the staging directory `/.roll/backups`, the archive layout, and the metadata JSON. Each file re-derives the path independently (documented in a comment at `commands/backup.cmd:22-35`) | Changing the layout in one file silently breaks the others; `restore-full.cmd` never loads the env config at all, so it resolves paths differently | | `commands/status.cmd:6` | `docker/docker-compose.yml` | The shared network name is recovered by `grep -A3 'networks:' … \| tail -n1 \| sed`, i.e. by text-parsing YAML | Reordering keys in `docker-compose.yml` breaks `roll status`'s "is RollDev running" check | -| `commands/describe.cmd:18`, `commands/vnc.cmd:12` | Compose container naming | Both reconstruct container names by string concatenation instead of asking `roll env ps -q` — and they disagree: `describe` uses v2 (`name-svc-1`), `vnc` uses v1 (`name_svc_1`) | `roll vnc` cannot find a container under the Compose v2 that `bin/roll:26` requires (**M8**) | | `environments/includes/redis.base.yml` | `environments/includes/dragonfly.base.yml` | Both define a service literally named `redis`; Dragonfly only swaps the image and volume | Correct as written — it is what lets `roll redis` work either way — but it forces the mutual-exclusion guard at `commands/env.cmd:12-14` | | `commands/env.cmd:116-120` | `commands/browsersync.cmd` | `env up` shells back out to `roll browsersync freeport` to pick host ports before assembling the compose args | A circular dependency between two commands, evaluated on every `up` with browsersync enabled | | `docs/index.md:7` | `README.md` | Sphinx `include` with `end-before: ` | The marker was missing before this revision, so the docs home page inlined the entire README (**M5**) | @@ -367,17 +366,6 @@ boundary problems below all cost maintenance time rather than correctness. independently after a failure. This is worth doing when the file is next substantially edited, not on its own. -**LB2. `environments/local/` is an empty environment type** — `environments/local/local.base.yml` - -- *What:* A 0-byte file. It satisfies `assertValidEnvType` (`utils/env.sh:83`), so `local` appears in - `roll env-init`'s list of valid types, and `commands/env.cmd:51,112` special-case it to skip - `php-fpm` and the nginx/db/redis defaults. There is no `init.env`. -- *Why it matters:* `roll env-init x local && roll env up` produces an environment with a network and - nothing else. Whether that is the intent is not recorded anywhere. -- *Suggested fix:* If it is intended as "networking only, bring your own `.roll/roll-env.yml`", say so - in a comment in the file and in `docs/environments/types.md`. If not, remove the directory; the - type disappears from the list automatically. - ## Environments `roll env-init ` writes `.env.roll` with five base lines and appends @@ -396,7 +384,7 @@ boundary problems below all cost maintenance time rather than correctness. | `wordpress` | yes | **no** | PHP 7.4, Composer 1, seeds `DB_*`; PHP image variant is `php-fpm-wordpress` | | `php` | yes | **no** | PHP 8.1, `ROLL_DB=0` | | `vuejs` | **no** | **no** | Routes `app.` to nginx and `watch.app.` to port 8080 on `php-fpm` | -| `local` | **no** | **no** | Empty base fragment — see **LB2** | +| `local` | **no** | **no** | Network-only environment; bring your own `.roll/roll-env.yml` | Without a Mutagen configuration, macOS falls back to the bind mount from `environments/includes/php-fpm.base.yml` (`.:/var/www/html:cached`), which works but is markedly @@ -586,7 +574,7 @@ These are enforced by review rather than by tooling, except where noted. - **Adding a command:** create `commands/.cmd` + `commands/.help`; the registry picks it up with no central list to edit. If it must accept arbitrary pass-through flags, add it to `ROLL_CMD_ANYARGS` in `bin/roll:43`. Add it to `commands/usage.help` as well — that file is - maintained by hand and is currently out of date (finding **L1**). + maintained by hand for visibility. - **Adding a service:** add the fragment(s) under `environments/includes/` (plus per-type overrides), wire a `ROLL_` toggle into the assembly block in `commands/env.cmd`, and register the variable and its default in `initConfigSchema` (`utils/config.sh:58`). Do not rely on a @@ -615,21 +603,6 @@ None found. ### Medium -**M3. The network-existence check in `env up` never matches, so a redundant compose pass runs every -time** — `commands/env.cmd:206` - -- *What:* `docker network ls -f 'name=$(renderEnvNetworkName)' -q` is single-quoted, so the command - substitution is not performed and Docker filters on the literal string - `name=$(renderEnvNetworkName)`. **Verified:** such a filter matches nothing, so the `-z` test is - always true and the `docker compose … up --no-start` block at `commands/env.cmd:208-210` executes on - every `roll env up`, not only on first creation. -- *Why it matters:* Every `env up` performs a full extra compose pass — resolving images, creating - containers with `--no-start` — before the real `up`. It is wasted wall-clock on every start and, - because `--no-start` creates containers, it can mask ordering problems that a genuine first-run - would expose. -- *Suggested fix:* `docker network ls -f "name=^$(renderEnvNetworkName)$" -q`, with double quotes and - anchors so a longer environment name is not matched as a prefix. - **M4. `restore.cmd` and `restore-full.cmd` are near-identical copies** — see boundary finding [**MB1**](#boundary-findings) for the measurement and the suggested split. @@ -649,141 +622,8 @@ time** — `commands/env.cmd:206` **M6. `roll registry`'s metadata layer is a stub** — see boundary finding [**MB2**](#boundary-findings). -**M7. Nothing validates `ROLL_ENV_NAME`, and Compose rejects uppercase project names** — -`commands/env-init.cmd:21-23`, `utils/config.sh:65` - -- *What:* `env-init` prompts for a name and accepts anything non-empty; the schema declares - `ROLL_ENV_NAME` as `string:required`, which only checks for emptiness. The name is passed straight - to `docker compose -p` (`commands/env.cmd:248`). **Verified:** `docker compose -p TestUpper …` - fails with `invalid project name "TestUpper": must consist only of lowercase alphanumeric - characters, hyphens, and underscores as well as start with a letter or number`. -- *Why it matters:* `roll env-init MyProject magento2` succeeds, and then every subsequent `roll env` - command fails with a Docker error that names neither RollDev nor `.env.roll`. Related: several - places lowercase the name (`renderEnvNetworkName`, `utils/env.sh:50`) while others do not - (`commands/describe.cmd:18`, the `traefik.docker.network=${ROLL_ENV_NAME}_default` labels in every - fragment), so even a name that Compose tolerates can produce a label that does not match the real - network. -- *Suggested fix:* Validate in `commands/env-init.cmd` before writing the file — reject anything not - matching `^[a-z0-9][a-z0-9_-]*$` and re-prompt — and add the same check to - `validateConfigValue` for `ROLL_ENV_NAME` so existing files are caught by `roll config validate`. - -**M8. `roll vnc` builds Compose v1 container names** — `commands/vnc.cmd:12` - -- *What:* It constructs `${ROLL_ENV_NAME}_${service}_${index}` — the Compose v1 separator — while - `bin/roll:26-30` requires Compose v2, which names containers `name-service-1`. - `commands/describe.cmd:18` uses the v2 form, so the two disagree. -- *Why it matters:* The SSH forward and the generated Remmina profile both target a hostname that does - not resolve, so `roll vnc` cannot reach the Selenium VNC server on any supported Compose version. -- *Suggested fix:* Resolve the container through Compose instead of reconstructing the name: - `roll env ps -q "${service}"`, then `docker container inspect` for the hostname. That also removes - the assumption of a single replica. Apply the same change to `commands/describe.cmd:18`. - ### Low -**L1. `commands/usage.help` is out of date** — `commands/usage.help:44-77` - -- *What:* The hand-maintained command list omits nine commands that the registry resolves from this - repository: `cliq`, `describe`, `duplicate`, `magerun`, `multistore`, `node`, `npm`, `vnc`, and the - internal `usage`. (`roll registry export simple` also lists commands supplied by third-party packs - under `~/.roll/reclu`, which correctly do not appear here.) -- *Why it matters:* `roll` with no arguments is the primary discovery path, so undocumented commands - are effectively invisible — including `roll env describe`, which is the most useful of them. -- *Suggested fix:* Add the missing entries. Longer term this is what **MB2** would remove the need - for: a working registry description mechanism would let `usage.cmd` render the list from the - `.help` files instead. - -**L2. Global configuration is loaded by `eval`, bypassing the schema** — `commands/env.cmd:20`, -`commands/svc.cmd:16,29,32,56`, `commands/shell.cmd:5`, `commands/rootshell.cmd:5` - -- *What:* Six places read `~/.roll/.env` with `eval "$(cat … | grep '^ROLL_')"` or a narrower grep, - rather than going through `loadConfigFromFile` (`utils/config.sh:235`), which parses `KEY=VALUE` - with a regex and validates against the schema. -- *Why it matters:* Any shell metacharacter in that file executes — a value containing `$(…)` or a - backtick runs as a command. The file is user-owned so this is not a privilege boundary, but it turns - a typo into arbitrary execution and it silently skips the validation everything else relies on. It - also means the same file is parsed two different ways depending on which command runs. -- *Suggested fix:* Call `loadConfigFromFile "${ROLL_HOME_DIR}/.env"` in all six places. It already - handles CRLF stripping and quote removal, which is what the `sed`/`grep` pipelines were - approximating. - -**L3. `ROLL_SERVICE_PORTAINER` has three different defaults** — `utils/config.sh:151`, -`commands/svc.cmd:35`, `commands/install.cmd:96,113`, `commands/status.cmd:79-84` - -- *What:* The schema says `boolean:1`; `commands/svc.cmd:35` falls back to `0`; - `commands/install.cmd` writes `ROLL_SERVICE_PORTAINER=1` into a fresh `~/.roll/.env` while the - comment above the second occurrence (`commands/install.cmd:112`) reads - `Set to "0" to disable global Portainer service`; `commands/status.cmd` falls back to `0` again. -- *Why it matters:* Whether Portainer starts depends on which code path ran, and `roll status` can - report it as disabled while `roll svc` started it. The stray comment makes the file look as though - it writes `0`. -- *Suggested fix:* Read the value through `getConfig ROLL_SERVICE_PORTAINER` in both `svc.cmd` and - `status.cmd` so the schema is the only default, and fix the comment. - -**L4. The three box-drawing helpers are copies** — `utils/core.sh:29-120` - -- *What:* `boxinfo`, `boxsuccess` and `boxerror` are 30-line functions differing only in the - `tput setaf` colour code (3, 2, 1) and one leading space in the border. -- *Why it matters:* Ninety lines where thirty-two would do, and a change to the box format has to be - made three times. -- *Suggested fix:* One `box ` function with the three names as one-line wrappers. - -**L5. `vuejs` has no `init.env`, so it inherits stale schema defaults** — -`environments/vuejs/` - -- *What:* Every other populated environment type ships an `init.env` that `commands/env-init.cmd:48` - appends. `vuejs` does not, so a new project gets only the five base lines and falls back to the - schema — PHP 8.1, Node 18, MariaDB 10.4 — for a type whose whole purpose is a Node toolchain. -- *Why it matters:* A Vue project is created with an eight-version-old Node by default, and the fact - that it is a default rather than a choice is invisible. -- *Suggested fix:* Add `environments/vuejs/init.env` pinning at least `NODE_VERSION`, `PHP_VERSION` - and `ROLL_DB`. See **LB2** for the related question about `environments/local/`. - -**L6. Traefik mounts the Docker socket read-write** — `docker/docker-compose.yml:12` - -- *What:* `- /var/run/docker.sock:/var/run/docker.sock`, with no `:ro`. Traefik's Docker provider only - reads. -- *Why it matters:* Write access to the Docker socket is equivalent to root on the host. This is a - local development tool, so the exposure is bounded by the host itself, but the write access buys - nothing. -- *Suggested fix:* Append `:ro`. - -**L7. `env up` chmods the SSH agent socket to 777 unconditionally** — `commands/env.cmd:285-287` - -- *What:* Every `up` and `start` runs `roll root chmod 777 /run/host-services/ssh-auth.sock` in the - `php-fpm` container, with no check that the path exists or that agent forwarding is in use. On Linux - the mount source is `${SSH_AUTH_SOCK:-/dev/null}` (`environments/includes/php-fpm.linux.yml:3`), so - without an agent this chmods `/dev/null`'s bind target. -- *Why it matters:* It makes the forwarded agent socket writable by every process in the container, - and the unconditional run means a failure here is noise rather than signal. Anything running in the - container can then use the host's SSH keys, which is the intent for the developer but also true for - anything that ends up inside a `composer install`. -- *Suggested fix:* Guard on `[[ -n "${SSH_AUTH_SOCK:-}" ]]` and use the narrowest mode that works — - `chown www-data` plus `chmod 600`, since only that user needs it. - -**L8. Sphinx is configured for directories that do not exist** — `docs/conf.py:43,45` - -- *What:* `html_static_path = ['_static']` and `html_extra_path = ['_redirects']`; neither - `docs/_static` nor `docs/_redirects` exists in the repository. -- *Why it matters:* Every documentation build emits warnings for both. Harmless, but it means a build - log is never clean, so a real warning is easy to miss. -- *Suggested fix:* Remove both settings, or add the directories with a `.gitkeep`. - -**L10. Root CA trust is skipped without warning on Linux distributions that are neither Debian- nor -Fedora-family** — `commands/install.cmd:29-52` - -- *What:* The trust step is an `if`/`elif`/`elif` chain testing for - `/etc/pki/ca-trust/source/anchors` (Fedora/CentOS), `/usr/local/share/ca-certificates` - (Debian/Ubuntu), then `darwin`. If a Linux host has neither directory — Arch, openSUSE, Alpine, - Void — no branch runs and nothing is printed. Contrast the DNS step immediately below, which does - emit a `warning` for every non-macOS host (`commands/install.cmd:65`). -- *Why it matters:* `roll install` reports success, and the first symptom is every `.test` site - showing a certificate error with no indication that a step was skipped rather than failed. The CA - file is generated correctly, so the user has no reason to suspect the trust step. -- *Suggested fix:* Add a final `elif [[ "$OSTYPE" =~ ^linux ]]` branch that warns and prints the CA - path (`${ROLL_SSL_DIR}/rootca/certs/ca.cert.pem`) with a pointer to the documentation, mirroring - the DNS warning. Detecting `update-ca-trust`/`update-ca-certificates` on `PATH` instead of probing - directories would also widen distribution coverage. - ## Audited and clean Checked during this review and found correct, so the next reader does not re-investigate: @@ -841,11 +681,10 @@ line-audited, and neither backup nor restore was executed against a live environ | `Unsupported OSTYPE '…'` | Only macOS and Linux (including WSL2) are supported hosts | See [Supported platforms](#supported-platforms); on Windows run `roll` inside WSL2, never in PowerShell or Git Bash | | `Environment config could not be found` | No `.env.roll` in the current directory or any parent | Run `roll env-init ` from the project root | | `docker compose version should be 2.2.3 or higher` | Compose v1, or the plugin is missing | Install the Compose v2 plugin; `docker-compose` (hyphenated) is not used | -| `invalid project name "…"` from `roll env up` | `ROLL_ENV_NAME` contains uppercase or an illegal character | Lowercase it in `.env.roll`; note the rename orphans the old volumes (finding **M7**) | +| `invalid project name "…"` from `roll env up` | `ROLL_ENV_NAME` contains uppercase or an illegal character | Lowercase it in `.env.roll`; note the rename orphans the old volumes | | Environment comes up on an empty database after a rename | `ROLL_ENV_NAME` is the Compose project name and prefixes every named volume | Bring the environment down, copy `oldname_dbdata` to `newname_dbdata`, bring it up | | `port is already allocated` on `php-fpm` | `ROLL_BROWSERSYNC=1` publishes fixed host ports on that service — the only fragment that publishes any | Set `ROLL_BROWSERSYNC=0`; verify with `roll env config \| grep published`. See `FEATURE-REQUESTS.md` H2 | | A command run right after `roll env up` fails to connect to the DB or search engine | No fragment declares a `healthcheck:` and `up` does not pass `--wait`, so it returns when containers *start* | Retry with a wait loop against the service. See `FEATURE-REQUESTS.md` H1 | -| `.test` sites show a certificate error on Linux after a clean `roll install` | CA trust is only wired up for Debian- and Fedora-family distributions, and is skipped silently otherwise | Finding **L10**; add `~/.roll/ssl/rootca/certs/ca.cert.pem` to the distribution's trust store manually | | `Mutagen sync sessions are not used on "linux" host environments` | `roll sync` is macOS-only | Expected; Linux and WSL bind-mount directly | | `Mutagen configuration does not exist for environment type "…"` | Only `magento1`, `magento2` and `shopware` ship a `.mutagen.yml` | Expected; that type uses a bind mount on macOS too | | In-container `composer install` reports a missing `.git` in a `vendor/` package | Mutagen's `ignore.vcs: true` strips `.git` at every depth, so source installs have no checkout | Remove the affected `vendor/` directories and re-run `composer install` to force dist installs | From c2b5fd90255838c46c831d85324f0acb3c7f6a1b Mon Sep 17 00:00:00 2001 From: Marc Bekhuis Date: Mon, 31 Aug 2026 14:32:15 +0200 Subject: [PATCH 15/41] Add utils/interact.sh, the single prompt and UI library Milestone 11, decisions D3, D8 and D9. Every prompt added from here follows one rule: a value from a flag or environment variable wins, gum runs only when there is a terminal, and anything else is a hard error naming the flag that would have supplied the value. That rule exists because gum has no non-interactive mode. gum choose and gum input open /dev/tty directly and exit 1 with no terminal, and piping into them supplies options rather than an answer, so a prompt must never be the only route to a value. This is what makes roll drivable from a script or CI. assertGum is called lazily by the wrappers rather than at startup, so a non-interactive run of a command that happens not to prompt still works with no gum installed. It enforces gum 0.14.0, the first release with the uniform 0/1/130 exit-code contract, and prints a per-platform install hint. The plain helpers in core.sh stay gum-free: they must work in pipes and CI. Boxes render through gum style only when interactive and fall back to the existing ASCII renderer otherwise. test-interact.sh asserts the non-interactive half of the contract - 17 assertions covering preset short-circuiting with gum removed from PATH entirely, each prompt failing with its flag named, the box fallback and set -e safety - and runs as part of the smoke suite on both CI platforms. The interactive half cannot be tested without a TTY; every gum flag used here was instead checked against the installed gum's own help. Co-Authored-By: Claude Opus 5 --- .github/scripts/smoke.sh | 2 + .github/scripts/test-interact.sh | 74 ++++++++++ bin/roll | 1 + utils/interact.sh | 233 +++++++++++++++++++++++++++++++ 4 files changed, 310 insertions(+) create mode 100755 .github/scripts/test-interact.sh create mode 100644 utils/interact.sh diff --git a/.github/scripts/smoke.sh b/.github/scripts/smoke.sh index 3c348fa..dfc0506 100755 --- a/.github/scripts/smoke.sh +++ b/.github/scripts/smoke.sh @@ -55,4 +55,6 @@ trap cleanup EXIT "${ROLL_BIN[@]}" config validate ) +"$(dirname "$0")/test-interact.sh" < /dev/null + echo "Smoke test passed (${MODE} mode)." diff --git a/.github/scripts/test-interact.sh b/.github/scripts/test-interact.sh new file mode 100755 index 0000000..50832d9 --- /dev/null +++ b/.github/scripts/test-interact.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash +# Asserts the non-interactive contract of utils/interact.sh: a value supplied by flag or env wins +# without ever reaching gum, and a prompt that cannot be answered fails naming the flag that would +# have answered it. This is the half of the interaction layer that automation depends on, and the +# half a TTY-less CI run can actually verify. +set -u +ROLL_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +export ROLL_DIR +. "$ROLL_DIR/utils/core.sh" +. "$ROLL_DIR/utils/interact.sh" + +pass=0; fail=0 +check() { if [[ "$2" == "$3" ]]; then echo " PASS $1"; pass=$((pass+1)); else echo " FAIL $1 (got '$2', want '$3')"; fail=$((fail+1)); fi; } + +echo "== isInteractive with stdin from /dev/null ==" +if isInteractive; then r=interactive; else r=non-interactive; fi +check "reports non-interactive" "$r" "non-interactive" + +echo "== preset value short-circuits, gum never invoked ==" +# Remove gum from PATH entirely: if the wrapper touches gum at all, this explodes. +ORIG_PATH="$PATH" +export PATH=/usr/bin:/bin +MYVAR="already-set" +out=$(promptInput MYVAR "--name" "Name:" 2>&1); st=$? +check "promptInput exit status" "$st" "0" +check "promptInput left value intact" "$MYVAR" "already-set" + +CHOICE="magento2" +promptChoose CHOICE "--type" "Pick a type" php laravel magento2 >/dev/null 2>&1; st=$? +check "promptChoose exit status" "$st" "0" +check "promptChoose left value intact" "$CHOICE" "magento2" + +PW="hunter2" +promptPassword PW "--encrypt=" "Password:" >/dev/null 2>&1; st=$? +check "promptPassword exit status" "$st" "0" +check "promptPassword left value intact" "$PW" "hunter2" +export PATH="$ORIG_PATH" + +echo "== no preset + no tty = hard error naming the flag ==" +UNSET_A="" +out=$( (promptInput UNSET_A "--name " "Name:") 2>&1 ); st=$? +check "promptInput exits 1" "$st" "1" +case "$out" in *"--name "*) r=yes;; *) r=no;; esac +check "promptInput names the flag" "$r" "yes" + +UNSET_B="" +out=$( (promptChoose UNSET_B "--type " "Pick a type" php laravel) 2>&1 ); st=$? +check "promptChoose exits 1" "$st" "1" +case "$out" in *"--type "*) r=yes;; *) r=no;; esac +check "promptChoose names the flag" "$r" "yes" + +out=$( (promptConfirm "--force" "Overwrite?") 2>&1 ); st=$? +check "promptConfirm exits 1" "$st" "1" +case "$out" in *"--force"*) r=yes;; *) r=no;; esac +check "promptConfirm names the flag" "$r" "yes" + +UNSET_C="" +out=$( (promptPassword UNSET_C "--encrypt=" "Password:") 2>&1 ); st=$? +check "promptPassword exits 1" "$st" "1" +case "$out" in *"--encrypt="*) r=yes;; *) r=no;; esac +check "promptPassword names the flag" "$r" "yes" + +echo "== boxes fall back to plain renderer with no tty ==" +out=$(styledBox 2 "hello world" 2>&1) +case "$out" in *"| hello world |"*) r=plain;; *) r=other;; esac +check "styledBox uses plain box" "$r" "plain" + +echo "== survives set -e ==" +( set -e; styledBox 7 "x" >/dev/null; MYV=preset; promptInput MYV "--x" "X:" >/dev/null; echo ok ) >/dev/null 2>&1 +check "no premature exit under set -e" "$?" "0" + +echo "" +echo "passed=$pass failed=$fail" +[[ $fail -eq 0 ]] diff --git a/bin/roll b/bin/roll index b1f6c5b..2e19b33 100755 --- a/bin/roll +++ b/bin/roll @@ -13,6 +13,7 @@ readonly ROLL_DIR="$( && pwd )" source "${ROLL_DIR}/utils/core.sh" +source "${ROLL_DIR}/utils/interact.sh" source "${ROLL_DIR}/utils/config.sh" source "${ROLL_DIR}/utils/registry.sh" source "${ROLL_DIR}/utils/env.sh" diff --git a/utils/interact.sh b/utils/interact.sh new file mode 100644 index 0000000..cb37748 --- /dev/null +++ b/utils/interact.sh @@ -0,0 +1,233 @@ +#!/usr/bin/env bash +[[ ! ${ROLL_DIR} ]] && >&2 echo -e "\033[31mThis script is not intended to be run directly!\033[0m" && exit 1 + +## Interactive prompts and styled output, all of it gum-backed. +## +## Every prompt in this file follows one rule: a value supplied by a flag or an environment variable +## wins, gum runs only when there is a terminal to run it in, and anything else is a hard error that +## names the flag which would have supplied the value. gum has no non-interactive mode - `gum choose` +## and `gum input` open /dev/tty directly and exit 1 when there is no terminal, and piping into them +## supplies options rather than an answer - so a prompt must never be reached to obtain a value that +## was not already resolvable without one. That is what makes roll drivable from a script, a CI job +## or an AI assistant. +## +## The plain messaging helpers in core.sh (fatal/error/warning/info/success) stay gum-free on +## purpose: they have to work in pipes and CI, where gum is neither present nor wanted. + +## Minimum gum with the uniform exit-code contract: 0 chosen, 1 cancelled with ESC, 130 Ctrl-C. +ROLL_GUM_REQUIRE="0.14.0" + +## Is there a terminal on both ends? gum needs one; nothing here may assume it. +function isInteractive() { + [[ -t 0 && -t 1 ]] +} + +## Called lazily by the wrappers below, never at startup: a non-interactive invocation of a command +## that happens not to prompt must keep working on a machine with no gum installed. +function assertGum() { + if ! command -v gum >/dev/null 2>&1; then + error "This prompt needs \`gum\`, which is not installed." + >&2 echo "" + >&2 echo " macOS: brew install gum" + >&2 echo " Debian/Ubuntu: sudo mkdir -p /etc/apt/keyrings \\" + >&2 echo " && curl -fsSL https://repo.charm.sh/apt/gpg.key \\" + >&2 echo " | sudo gpg --dearmor -o /etc/apt/keyrings/charm.gpg \\" + >&2 echo " && echo 'deb [signed-by=/etc/apt/keyrings/charm.gpg] https://repo.charm.sh/apt/ * *' \\" + >&2 echo " | sudo tee /etc/apt/sources.list.d/charm.list \\" + >&2 echo " && sudo apt update && sudo apt install gum" + >&2 echo " Fedora/RHEL: sudo dnf install gum" + >&2 echo " Arch: sudo pacman -S gum" + >&2 echo " Anywhere: download a binary from https://github.com/charmbracelet/gum/releases" + >&2 echo "" + fatal "Install gum and try again, or supply the value non-interactively." + fi + + local installed="" + installed="$(gum --version 2>/dev/null | grep -oE '[0-9]+\.[0-9]+\.[0-9]+' | head -n1)" || true + + if [[ -z "${installed}" ]]; then + warning "Could not determine the installed gum version; continuing." + return 0 + fi + + if ! test "$(version "${installed}")" -ge "$(version "${ROLL_GUM_REQUIRE}")"; then + fatal "gum ${ROLL_GUM_REQUIRE} or newer is required (${installed} installed); please upgrade it." + fi + + return 0 +} + +## Turn gum's exit status into a message and an exit. ESC is 1 and Ctrl-C is 130; both mean the user +## chose not to answer, and 130 is propagated so a caller in a pipeline sees a real interrupt. +function handleGumCancel() { + local status="$1" + local what="$2" + + if [[ "${status}" == "130" ]]; then + >&2 echo "" + error "Cancelled." + exit 130 + fi + + error "No ${what} selected." + exit 1 +} + +## Explain how to supply a value when there is no terminal to ask on. +function fatalNoTty() { + local what="$1" + local how="$2" + + error "Cannot prompt for ${what}: no terminal attached." + fatal "Supply it non-interactively with ${how}." +} + +## promptInput [placeholder] +## Leaves the variable untouched when it already holds a value. +function promptInput() { + local var="$1" how="$2" prompt="$3" placeholder="${4:-}" + local current="" value="" status=0 + + eval "current=\${${var}:-}" + if [[ -n "${current}" ]]; then + return 0 + fi + + if ! isInteractive; then + fatalNoTty "${prompt}" "${how}" + fi + + assertGum + value="$(gum input --prompt "${prompt} " --placeholder "${placeholder}")" || status=$? + if [[ ${status} -ne 0 ]]; then + handleGumCancel "${status}" "value" + fi + + if [[ -z "${value}" ]]; then + error "No value entered." + exit 1 + fi + + printf -v "${var}" '%s' "${value}" + return 0 +} + +## promptChoose