From 20f9475a7dd2b3e3a124cf7471c57cab790615c1 Mon Sep 17 00:00:00 2001 From: beatzaplenty Date: Mon, 20 Jul 2026 16:57:43 +1000 Subject: [PATCH] Fix nix_extra_opts falsely reporting nix-cache's SSH remote builder down The reachability check used `cat < /dev/tcp/${NIX_CACHE_HOST}/22`, which blocks forever reading for EOF that never comes -- sshd sends its banner and then holds the connection open waiting for the client to speak next. Every single check hit the 3s timeout and reported "unreachable" unconditionally, regardless of whether the remote builder was actually up. Confirmed live: a plain TCP connect (`exec 3<>/dev/tcp/...`, no read) returns in ~60ms against a healthy nix-cache instead of always timing out. Fixing that exposed a second, previously-dormant bug: `printf -v NIX_EXTRA_OPTS '%q ' "${NIX_OPTS[@]}"` on a genuinely empty NIX_OPTS array still runs one format pass and yields the literal `'' ` rather than an empty string. A subprocess (e.g. sync-host-keys.sh) reusing this process's decision via `eval "NIX_OPTS=(${NIX_EXTRA_OPTS})"` then rebuilt a 1-element array holding an empty string instead of a 0-element array, which broke `nix-shell "${NIX_OPTS[@]}" -p ` with a bogus positional argument the moment NIX_OPTS was legitimately empty (nix-cache reachable) -- something the first bug had made impossible to ever hit before. Also adds a couple of retries (1s apart) to both checks as a secondary safety net against genuine multi-second blips, on top of fixing the checks themselves. Co-Authored-By: Claude Sonnet 5 --- scripts/env.sh | 57 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/scripts/env.sh b/scripts/env.sh index 08456f9..b0c48c1 100755 --- a/scripts/env.sh +++ b/scripts/env.sh @@ -88,13 +88,60 @@ nix_extra_opts() { fi export NIX_EXTRA_OPTS_DECIDED=1 NIX_OPTS=() - if ! curl --silent --fail --max-time 3 "http://${NIX_CACHE_HOST}/nix-cache-info" >/dev/null 2>&1; then + + # Retry a couple of times, 1s apart, before believing either check -- + # belt-and-suspenders against a genuine multi-second blip (nix-cache + # restarting), on top of the fix below. Worst case (~11s total, host + # genuinely gone) is still nowhere near the 15s+ *per lookup* nix's own + # substituter retries would cost if this check didn't exist at all. + local attempt cache_up=0 builder_up=0 + for attempt in 1 2 3; do + if curl --silent --fail --max-time 3 "http://${NIX_CACHE_HOST}/nix-cache-info" >/dev/null 2>&1; then + cache_up=1 + break + fi + [[ "$attempt" -lt 3 ]] && sleep 1 + done + + if [[ "$cache_up" -eq 0 ]]; then echo "nix-cache (http://${NIX_CACHE_HOST}) is unreachable -- skipping it (substituter + remote builder) for the rest of this run." >&2 NIX_OPTS=(--option substituters "https://cache.nixos.org/" --builders "") - elif ! timeout 3 bash -c "cat < /dev/tcp/${NIX_CACHE_HOST}/22" >/dev/null 2>&1; then - echo "nix-cache's SSH remote builder (nixremote@${NIX_CACHE_HOST}:22) is unreachable -- disabling remote builds for the rest of this run." >&2 - NIX_OPTS=(--builders "") + else + for attempt in 1 2 3; do + # `exec 3<>/dev/tcp/...` just opens the fd and returns -- it does NOT + # read from it. Confirmed live this is load-bearing, not stylistic: + # the previous `cat < /dev/tcp/.../22` blocked forever and always hit + # the timeout even against a perfectly healthy nix-cache, because + # sshd sends its banner and then holds the connection open waiting + # for the client to speak next -- `cat` never sees EOF, so this + # check reported "unreachable" unconditionally, 100% of the time, + # regardless of whether the remote builder was actually up. + if timeout 3 bash -c "exec 3<>/dev/tcp/${NIX_CACHE_HOST}/22" 2>/dev/null; then + builder_up=1 + break + fi + [[ "$attempt" -lt 3 ]] && sleep 1 + done + if [[ "$builder_up" -eq 0 ]]; then + echo "nix-cache's SSH remote builder (nixremote@${NIX_CACHE_HOST}:22) is unreachable -- disabling remote builds for the rest of this run." >&2 + NIX_OPTS=(--builders "") + fi + fi + + # `printf '%q '` with a genuinely empty NIX_OPTS still runs one format + # pass over a missing argument and yields the literal `'' ` rather than + # an empty string (confirmed live) -- a subprocess that later does + # `eval "NIX_OPTS=(${NIX_EXTRA_OPTS})"` (the branch above, for e.g. + # sync-host-keys.sh reusing this process's decision) would then rebuild + # a 1-element array holding an empty string instead of a 0-element + # array, and `nix-shell "${NIX_OPTS[@]}" -p ` chokes on that stray + # element as a bogus positional argument. Guard the empty case + # explicitly so nix-cache being reachable (NIX_OPTS legitimately empty) + # round-trips as truly empty instead. + if [[ ${#NIX_OPTS[@]} -gt 0 ]]; then + printf -v NIX_EXTRA_OPTS '%q ' "${NIX_OPTS[@]}" + else + NIX_EXTRA_OPTS="" fi - printf -v NIX_EXTRA_OPTS '%q ' "${NIX_OPTS[@]}" export NIX_EXTRA_OPTS }