diff --git a/__tests__/brew.test.ts b/__tests__/brew.test.ts new file mode 100644 index 00000000..be39476c --- /dev/null +++ b/__tests__/brew.test.ts @@ -0,0 +1,317 @@ +import {spawn, spawnSync} from 'child_process'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; + +const brew = fs.readFileSync( + path.join(__dirname, '../src/scripts/tools/brew.sh'), + 'utf8' +); +const describeUnix = process.platform === 'win32' ? describe.skip : describe; + +describeUnix('Homebrew inactivity watchdog', () => { + let root: string; + let fixture: string; + let buildScript: string; + + const pids = () => + fs.existsSync(path.join(root, 'pids')) + ? fs + .readFileSync(path.join(root, 'pids'), 'utf8') + .trim() + .split('\n') + .map(Number) + : []; + + const running = (pid: number) => { + const result = spawnSync('ps', ['-p', String(pid), '-o', 'stat='], { + encoding: 'utf8' + }); + return result.status === 0 && !result.stdout.trim().startsWith('Z'); + }; + + const cleanup = () => { + for (const pid of pids().reverse()) { + try { + process.kill(pid, 'SIGKILL'); + } catch { + // The watchdog may have already terminated this process. + } + } + }; + + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'setup-php-brew-test-')); + fixture = path.join(root, 'source.cjs'); + buildScript = path.join(root, 'Homebrew', 'build.rb'); + fs.mkdirSync(path.dirname(buildScript)); + fs.symlinkSync(fixture, buildScript); + fs.writeFileSync( + fixture, + `const fs = require('fs'); +const {spawn, spawnSync} = require('child_process'); +const role = process.argv[2] || 'brew'; +const pidFile = process.env.TEST_ROOT + '/pids'; +if (role === 'brew') { + if (fs.existsSync(pidFile)) { + for (const pid of fs.readFileSync(pidFile, 'utf8').trim().split('\\n')) { + const state = spawnSync('ps', ['-p', pid, '-o', 'stat='], {encoding: 'utf8'}); + if (state.status === 0 && !state.stdout.trim().startsWith('Z')) { + fs.appendFileSync(process.env.TEST_ROOT + '/overlap', pid + '\\n'); + } + } + } + fs.appendFileSync(process.env.TEST_ROOT + '/attempts', 'attempt\\n'); + process.stdout.write('==> make\\n'); +} +fs.appendFileSync(pidFile, process.pid + '\\n'); +if (role === 'compiler') { + process.on('SIGTERM', () => {}); + if (process.env.TEST_BUILD_DURATION) { + setTimeout(() => process.exit(0), Number(process.env.TEST_BUILD_DURATION)); + } +} else { + const script = role === 'brew' ? process.env.TEST_BUILD_SCRIPT : __filename; + const child = spawn(process.execPath, [script, role === 'brew' ? 'builder' : 'compiler'], { + detached: true, + stdio: process.env.TEST_BUILD_STDIO + }); + child.on('exit', status => { + if (process.env.TEST_BUILD_DURATION) { + if (role === 'brew') fs.writeFileSync(process.env.TEST_ROOT + '/built', 'done'); + const delay = role === 'brew' ? Number(process.env.TEST_AFTER_BUILD_DELAY || 0) : 0; + setTimeout(() => process.exit(status || 0), delay); + } + }); + child.unref(); +} +setInterval(() => {}, 1000); +` + ); + }); + + afterEach(() => { + cleanup(); + fs.rmSync(root, {recursive: true, force: true}); + }); + + const run = (script: string, env: NodeJS.ProcessEnv = {}) => + new Promise<{status: number | null; stdout: string; stderr: string}>( + (resolve, reject) => { + const child = spawn('bash', ['-c', brew + '\n' + script], { + detached: true, + env: { + ...process.env, + TEST_NODE: process.execPath, + TEST_ROOT: root, + TEST_FIXTURE: fixture, + TEST_BUILD_SCRIPT: buildScript, + TEST_BUILD_STDIO: 'ignore', + SETUP_PHP_BREW_WATCHDOG: 'true', + SETUP_PHP_BREW_INACTIVITY_TIMEOUT: '1', + SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '1', + SETUP_PHP_BREW_WATCHDOG_POLL: '0.05', + SETUP_PHP_BREW_RETRY_ATTEMPTS: '3', + ...env + } + }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', data => (stdout += data)); + child.stderr.on('data', data => (stderr += data)); + const timer = setTimeout(() => { + cleanup(); + if (child.pid) { + try { + process.kill(-child.pid, 'SIGKILL'); + } catch { + // The shell may have just exited. + } + } + reject(new Error('Watchdog did not finish: ' + stderr)); + }, 20000); + child.on('error', error => { + clearTimeout(timer); + reject(error); + }); + child.on('close', status => { + clearTimeout(timer); + resolve({status, stdout, stderr}); + }); + } + ); + + it.each(['ignore', 'inherit'])( + 'kills the complete source-build tree with %s stdio before returning', + async stdio => { + const result = await run( + 'run_with_inactivity_watchdog "$TEST_NODE" "$TEST_FIXTURE"', + {TEST_BUILD_STDIO: stdio} + ); + expect(result.status).toBe(124); + expect(pids()).toHaveLength(3); + expect(pids().filter(running)).toEqual([]); + expect(result.stderr).toContain('brew produced no output'); + expect(result.stderr).not.toContain('retrying'); + }, + 25000 + ); + + it('cleans up each timed-out build before retrying, then stops at the limit', async () => { + const result = await run(` +brew() { "$TEST_NODE" "$TEST_FIXTURE"; } +sleep() { case "$1" in 5|10) return 0;; *) command sleep "$@";; esac; } +safe_brew install php@8.4 +`); + expect(result.status).toBe(124); + expect(fs.readFileSync(path.join(root, 'attempts'), 'utf8')).toBe( + 'attempt\nattempt\nattempt\n' + ); + expect(fs.existsSync(path.join(root, 'overlap'))).toBe(false); + expect(pids().filter(running)).toEqual([]); + expect(result.stderr.match(/retrying brew command/g)).toHaveLength(2); + expect(result.stderr).not.toContain('attempt 4'); + }, 25000); + + it('recovers on the next attempt after cleaning up a timed-out source build', async () => { + const result = await run(` +brew() { + if [ ! -e "$TEST_ROOT/attempts" ]; then + "$TEST_NODE" "$TEST_FIXTURE" + else + echo recovered + fi +} +sleep() { case "$1" in 5) return 0;; *) command sleep "$@";; esac; } +safe_brew install php@8.4 +`); + expect(result.status).toBe(0); + expect(result.stdout).toContain('recovered\n'); + expect(result.stderr.match(/retrying brew command/g)).toHaveLength(1); + expect(result.stderr).toContain('attempt 2/3, exit 124'); + expect(pids().filter(running)).toEqual([]); + }, 10000); + + it('retries an ordinary failure and stops after success', async () => { + const result = await run(` +brew() { + if [ ! -e "$TEST_ROOT/failed" ]; then + touch "$TEST_ROOT/failed" + return 37 + fi + printf recovered +} +sleep() { case "$1" in 5) return 0;; *) command sleep "$@";; esac; } +safe_brew install php@8.4 +`); + expect(result.status).toBe(0); + expect(result.stdout).toBe('recovered'); + expect(result.stderr.match(/retrying brew command/g)).toHaveLength(1); + expect(result.stderr).toContain('attempt 2/3, exit 37'); + }); + + it('preserves the opt-out from the watchdog and retries', async () => { + const result = await run( + 'brew() { printf disabled; return 37; }; safe_brew install php@8.4', + {SETUP_PHP_BREW_WATCHDOG: 'false'} + ); + expect(result).toEqual({status: 37, stdout: 'disabled', stderr: ''}); + }); + + it.each([0, 37])('preserves output and exit status %s', async status => { + const result = await run( + `run_with_inactivity_watchdog bash -c 'printf out; printf err >&2; exit ${status}'` + ); + expect(result).toEqual({status, stdout: 'out', stderr: 'err'}); + }); + + it('keeps the bottle timeout when no source build is running', async () => { + const result = await run( + 'run_with_inactivity_watchdog "$TEST_NODE" -e "setTimeout(() => {}, 6000)"', + {SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '4'} + ); + expect(result.status).toBe(124); + expect(result.stderr).toContain('no output for 1s; terminating'); + }, 10000); + + it('ignores source builds outside the watched process tree', async () => { + const otherBuild = spawn( + process.execPath, + ['-e', 'setTimeout(() => {}, 10000)', buildScript], + {stdio: 'ignore'} + ); + try { + const result = await run( + 'run_with_inactivity_watchdog "$TEST_NODE" -e "setTimeout(() => {}, 6000)"', + {SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '4'} + ); + expect(result.status).toBe(124); + expect(result.stderr).toContain('no output for 1s; terminating'); + } finally { + otherBuild.kill('SIGKILL'); + } + }, 10000); + + it.each(['', '4'])( + 'allows quiet source builds past the bottle timeout with source timeout=%s', + async sourceTimeout => { + const result = await run( + 'run_with_inactivity_watchdog "$TEST_NODE" "$TEST_FIXTURE"', + { + SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: sourceTimeout, + TEST_BUILD_DURATION: '2500' + } + ); + expect(result.status).toBe(0); + expect(fs.existsSync(path.join(root, 'built'))).toBe(true); + expect(pids().filter(running)).toEqual([]); + expect(result.stderr).not.toContain('terminating'); + }, + 10000 + ); + + it('terminates a stalled source build at its longer timeout', async () => { + const result = await run( + 'run_with_inactivity_watchdog "$TEST_NODE" "$TEST_FIXTURE"', + {SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '3'} + ); + expect(result.status).toBe(124); + expect(result.stderr).toContain('no output for 3s; terminating'); + expect(pids().filter(running)).toEqual([]); + }, 10000); + + it('restores the bottle timeout after the source build finishes', async () => { + const result = await run( + 'run_with_inactivity_watchdog "$TEST_NODE" "$TEST_FIXTURE"', + { + SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT: '4', + TEST_BUILD_DURATION: '2500', + TEST_AFTER_BUILD_DELAY: '6000' + } + ); + expect(result.status).toBe(124); + expect(fs.existsSync(path.join(root, 'built'))).toBe(true); + expect(result.stderr).toContain('no output for 1s; terminating'); + expect(pids().filter(running)).toEqual([]); + }, 10000); + + it.each(['stdout', 'stderr'] as const)( + 'counts partial output on %s as activity', + async stream => { + const result = await run( + `run_with_inactivity_watchdog "$TEST_NODE" -e ' +let count = 0; +const timer = setInterval(() => { + process.${stream}.write("."); + if (++count === 25) clearInterval(timer); +}, 100); +'`, + {SETUP_PHP_BREW_INACTIVITY_TIMEOUT: '2'} + ); + expect(result.status).toBe(0); + expect(result[stream]).toBe('.'.repeat(25)); + expect(result.stderr).not.toContain('terminating'); + }, + 10000 + ); +}); diff --git a/__tests__/darwin.test.ts b/__tests__/darwin.test.ts new file mode 100644 index 00000000..d4fa9d5e --- /dev/null +++ b/__tests__/darwin.test.ts @@ -0,0 +1,151 @@ +import {spawnSync} from 'child_process'; +import fs from 'fs'; +import path from 'path'; + +const darwin = fs + .readFileSync(path.join(__dirname, '../src/scripts/darwin.sh'), 'utf8') + .split('\n# Variables\n')[0]; +const describeUnix = process.platform === 'win32' ? describe.skip : describe; + +describeUnix('macOS PHP installation', () => { + const run = (env: NodeJS.ProcessEnv) => + spawnSync( + 'bash', + [ + '-c', + `${darwin} +uname() { echo "$TEST_ARCH"; } +setup_cached_versions() { echo cache; return "$TEST_CACHE_STATUS"; } +update_dependencies() { echo update; } +add_brew_tap() { echo tap; } +safe_brew() { + echo "brew $*" + case "$*" in + *--only-dependencies*) return "$TEST_DEPENDENCY_STATUS";; + install*) return "$TEST_INSTALL_STATUS";; + upgrade*) return "$TEST_UPGRADE_STATUS";; + esac +} +brew() { echo "brew $*"; } +add_php "$TEST_ACTION" "$TEST_EXISTING_VERSION" +` + ], + { + encoding: 'utf8', + timeout: 5000, + env: { + ...process.env, + TEST_ARCH: 'arm64', + TEST_ACTION: 'install', + TEST_EXISTING_VERSION: 'false', + TEST_CACHE_STATUS: '0', + TEST_DEPENDENCY_STATUS: '0', + TEST_INSTALL_STATUS: '0', + TEST_UPGRADE_STATUS: '0', + version: '8.4', + debug: 'none', + ts: 'nts', + runner: 'github', + use_package_cache: 'true', + php_tap: 'shivammathur/homebrew-php', + ...env + } + } + ); + + it.each([ + ['arm64', '0', false, 0], + ['arm64', '37', true, 0], + ['x86_64', '0', false, 0], + ['x86_64', '37', false, 1] + ])( + 'preserves the cache fallback on %s with cache status %s', + (arch, cacheStatus, fallback, status) => { + const result = run({TEST_ARCH: arch, TEST_CACHE_STATUS: cacheStatus}); + expect(result.error).toBeUndefined(); + expect(result.stderr).toBe(''); + expect(result.status).toBe(status); + expect(result.stdout).toContain('cache\n'); + expect(result.stdout.includes('brew install')).toBe(fallback); + } + ); + + it.each([ + ['install', '124', '0', '0', 124, 1], + ['install', '37', '0', '0', 37, 1], + ['install', '0', '124', '0', 124, 2], + ['install', '0', '37', '124', 124, 3], + ['install', '0', '37', '37', 37, 3], + ['upgrade', '124', '0', '0', 124, 1], + ['upgrade', '0', '0', '124', 124, 2], + ['upgrade', '0', '0', '37', 37, 2] + ])( + 'stops %s after failures (dependencies=%s, install=%s, upgrade=%s)', + (action, dependencyStatus, installStatus, upgradeStatus, status, calls) => { + const result = run({ + TEST_ACTION: action, + TEST_EXISTING_VERSION: action === 'upgrade' ? '8.4.10' : 'false', + TEST_CACHE_STATUS: '37', + TEST_DEPENDENCY_STATUS: dependencyStatus, + TEST_INSTALL_STATUS: installStatus, + TEST_UPGRADE_STATUS: upgradeStatus + }); + expect(result.error).toBeUndefined(); + expect(result.stderr).toBe(''); + expect(result.status).toBe(status); + expect(result.stdout.match(/^brew /gm)).toHaveLength(calls); + expect(result.stdout).not.toContain('brew link'); + } + ); + + it('retains the upgrade fallback for an install failure other than a timeout', () => { + const result = run({TEST_CACHE_STATUS: '37', TEST_INSTALL_STATUS: '1'}); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(0); + expect(result.stdout).toContain( + 'brew upgrade -f --overwrite shivammathur/php/php@8.4\n' + ); + expect(result.stdout).toContain('brew link --force --overwrite php@8.4\n'); + }); + + it('reuses an existing installation without invoking the cache or Homebrew install', () => { + const result = run({TEST_EXISTING_VERSION: '8.4.10'}); + expect(result.status).toBe(0); + expect(result.stdout).toBe( + 'brew unlink php@8.4\nbrew link --force --overwrite php@8.4\n' + ); + }); + + it.each([ + ['debug', 'nts', '-debug'], + ['none', 'zts', '-zts'], + ['debug', 'zts', '-debug-zts'] + ])('keeps cache fallback for debug=%s, ts=%s', (debug, ts, suffix) => { + const result = run({ + TEST_EXISTING_VERSION: '8.4.10', + TEST_CACHE_STATUS: '37', + debug, + ts + }); + expect(result.status).toBe(0); + expect(result.stdout).toContain('cache\n'); + expect(result.stdout).toContain( + `brew install --skip-link -f --overwrite shivammathur/php/php@8.4${suffix}\n` + ); + }); + + it.each([ + ['self-hosted', 'true'], + ['github', 'false'] + ])('uses Homebrew for runner=%s, cache=%s', (runner, use_package_cache) => { + const result = run({runner, use_package_cache, TEST_CACHE_STATUS: '37'}); + expect(result.error).toBeUndefined(); + expect(result.stderr).toBe(''); + expect(result.status).toBe(0); + expect(result.stdout).not.toContain('cache\n'); + expect(result.stdout).toContain( + 'brew install --skip-link -f --overwrite shivammathur/php/php@8.4\n' + ); + expect(result.stdout).toContain('brew link --force --overwrite php@8.4\n'); + }); +}); diff --git a/src/scripts/darwin.sh b/src/scripts/darwin.sh index 49018876..a2f76d0a 100644 --- a/src/scripts/darwin.sh +++ b/src/scripts/darwin.sh @@ -183,6 +183,7 @@ setup_cached_versions() { # Function to setup PHP 5.6 and newer using Homebrew. add_php() { + local exit_code action=$1 existing_version=$2 suffix="$(get_php_formula_suffix)" @@ -198,14 +199,18 @@ add_php() { fi if [[ "$existing_version" != "false" && -z "$suffix" ]]; then if [ "$action" = "upgrade" ]; then - safe_brew install --only-dependencies "$php_formula" - safe_brew upgrade -f --overwrite "$php_formula" + safe_brew install --only-dependencies "$php_formula" || return $? + safe_brew upgrade -f --overwrite "$php_formula" || return $? else brew unlink "$php_keg" fi else - safe_brew install --only-dependencies "$php_formula" - safe_brew install --skip-link -f --overwrite "$php_formula" 2>/dev/null || safe_brew upgrade -f --overwrite "$php_formula" + safe_brew install --only-dependencies "$php_formula" || return $? + safe_brew install --skip-link -f --overwrite "$php_formula" 2>/dev/null || { + exit_code=$? + [ "$exit_code" -ne 124 ] || return "$exit_code" + safe_brew upgrade -f --overwrite "$php_formula" || return $? + } fi brew link --force --overwrite "$php_keg" || (sudo chown -R "$(id -un)":"$(id -gn)" "$brew_prefix" && brew link --force --overwrite "$php_keg") } diff --git a/src/scripts/tools/brew.sh b/src/scripts/tools/brew.sh index 6644ab9e..b2677565 100644 --- a/src/scripts/tools/brew.sh +++ b/src/scripts/tools/brew.sh @@ -55,25 +55,45 @@ get_file_mtime() { fi } -# Function to terminate a process and its direct children. -terminate_process_tree() { +# Function to list descendants before their parents, including separate sessions. +get_process_tree() { local pid=$1 local children child children=$(pgrep -P "$pid" 2>/dev/null || true) - kill -TERM "$pid" >/dev/null 2>&1 || true for child in $children; do - terminate_process_tree "$child" + get_process_tree "$child" + done + echo "$pid" +} + +# Function to detect Homebrew's source-build worker, even with buffered output. +is_brew_building_from_source() { + local pid + for pid in $(get_process_tree "$1"); do + if ps -ww -p "$pid" -o command= 2>/dev/null | grep -qE '/Homebrew/build[.]rb([[:space:]]|$)'; then + return 0 + fi + done + return 1 +} + +# Function to terminate the entire tree captured before any parents can exit. +terminate_process_tree() { + local pids pid + pids=$(get_process_tree "$1") + for pid in $pids; do + kill -TERM "$pid" >/dev/null 2>&1 || true done sleep 2 - kill -KILL "$pid" >/dev/null 2>&1 || true - for child in $children; do - terminate_process_tree "$child" + for pid in $pids; do + kill -KILL "$pid" >/dev/null 2>&1 || true done } # Function to run a command with an inactivity watchdog. run_with_inactivity_watchdog() { local timeout_secs="${SETUP_PHP_BREW_INACTIVITY_TIMEOUT:-180}" + local source_timeout_secs="${SETUP_PHP_BREW_SOURCE_INACTIVITY_TIMEOUT:-1800}" local poll_secs="${SETUP_PHP_BREW_WATCHDOG_POLL:-5}" local tmp_dir stdout_fifo stderr_fifo stdout_log stderr_log timeout_file local command_pid stdout_reader_pid stderr_reader_pid monitor_pid exit_code @@ -93,36 +113,39 @@ run_with_inactivity_watchdog() { ("$@" >"$stdout_fifo" 2>"$stderr_fifo") & command_pid=$! - ( - while IFS= read -r line || [ -n "$line" ]; do - printf '%s\n' "$line" - printf '%s\n' "$line" >>"$stdout_log" - done <"$stdout_fifo" - ) & + tee "$stdout_log" <"$stdout_fifo" & stdout_reader_pid=$! - ( - while IFS= read -r line || [ -n "$line" ]; do - printf '%s\n' "$line" >&2 - printf '%s\n' "$line" >>"$stderr_log" - done <"$stderr_fifo" - ) & + tee "$stderr_log" <"$stderr_fifo" >&2 & stderr_reader_pid=$! ( local last_activity current_activity current_err_activity now + local building_from_source=false was_building_from_source=false active_timeout_secs last_activity=$(get_file_mtime "$stdout_log") current_err_activity=$(get_file_mtime "$stderr_log") [ "$current_err_activity" -gt "$last_activity" ] && last_activity="$current_err_activity" while kill -0 "$command_pid" >/dev/null 2>&1; do sleep "$poll_secs" + kill -0 "$command_pid" >/dev/null 2>&1 || break + now=$(date +%s) + active_timeout_secs="$timeout_secs" + building_from_source=false + if is_brew_building_from_source "$command_pid"; then + building_from_source=true + active_timeout_secs="$source_timeout_secs" + fi + if [ "$building_from_source" != "$was_building_from_source" ]; then + last_activity="$now" + was_building_from_source="$building_from_source" + fi current_activity=$(get_file_mtime "$stdout_log") [ "$current_activity" -gt "$last_activity" ] && last_activity="$current_activity" current_err_activity=$(get_file_mtime "$stderr_log") [ "$current_err_activity" -gt "$last_activity" ] && last_activity="$current_err_activity" now=$(date +%s) - if [ $((now - last_activity)) -ge "$timeout_secs" ]; then - printf "\nsetup-php: brew produced no output for %ss; terminating and retrying...\n" "$timeout_secs" >&2 + if [ $((now - last_activity)) -ge "$active_timeout_secs" ]; then + printf "\nsetup-php: brew produced no output for %ss; terminating...\n" "$active_timeout_secs" >&2 : >"$timeout_file" terminate_process_tree "$command_pid" break @@ -131,12 +154,15 @@ run_with_inactivity_watchdog() { ) & monitor_pid=$! - wait "$command_pid" - exit_code=$? + exit_code=0 + wait "$command_pid" || exit_code=$? + # Let timeout cleanup finish killing source-build descendants before retrying. + if [ ! -e "$timeout_file" ]; then + kill "$monitor_pid" >/dev/null 2>&1 || true + fi + wait "$monitor_pid" 2>/dev/null || true wait "$stdout_reader_pid" 2>/dev/null || true wait "$stderr_reader_pid" 2>/dev/null || true - kill "$monitor_pid" >/dev/null 2>&1 || true - wait "$monitor_pid" 2>/dev/null || true if [ -e "$timeout_file" ]; then rm -rf "$tmp_dir"