From c0b4a3438ac66a4f0c25d4aa9f6f54f2f9f24816 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:28:46 -0700 Subject: [PATCH] fix(install): scope Playwright override to too-new apt releases + keep step interruptible Follow-up on #54032 for #35166: - Gate the PLAYWRIGHT_HOST_PLATFORM_OVERRIDE retry on the host being an apt release newer than Playwright recognizes (Ubuntu >24.04 / Debian >13) via playwright_host_unrecognized(), instead of retrying on ANY install failure. A network/disk/permission failure on a supported host now surfaces unchanged rather than getting a mismatched-glibc build forced onto it. - detect_os() now captures DISTRO_VERSION from os-release. - Fold in the interruptibility fix (was PR #35304, self-closed): wrap the download in 'timeout --foreground -k 10' (probed, with plain-timeout fallback) so a terminal Ctrl+C reaches the child and a wedged download is force-killed after the deadline. - Add behavioral tests that source the helpers and assert the retry fires only on Ubuntu 26.04 / Debian 14, not on supported hosts, non-apt distros, native-success, operator-pinned override, or unsupported arch. --- scripts/install.sh | 72 ++++++++-- tests/test_install_sh_browser_install.py | 172 ++++++++++++++++++++++- 2 files changed, 230 insertions(+), 14 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index db765362ca9..141a3e290ed 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -509,8 +509,13 @@ detect_os() { if [ -f /etc/os-release ]; then . /etc/os-release DISTRO="$ID" + # VERSION_ID (e.g. "26.04", "14") lets us tell whether the + # apt release is newer than the newest one Playwright's + # platform resolver recognizes — the #35166 hang condition. + DISTRO_VERSION="${VERSION_ID:-}" else DISTRO="unknown" + DISTRO_VERSION="" fi fi ;; @@ -1885,12 +1890,45 @@ run_browser_install_with_timeout() { shift if command -v timeout >/dev/null 2>&1; then - timeout "$timeout_seconds" "$@" + # GNU `timeout` runs the command in its own process group, so a terminal + # Ctrl+C is delivered to `timeout` but never reaches the child — the + # download looks frozen and ignores Ctrl+C (#35166). `--foreground` + # keeps the command in the shell's foreground group so Ctrl+C reaches + # it; `-k 10` sends SIGKILL 10s after the deadline so a wedged download + # can't outlive the timeout. Both flags are GNU-only — probe once and + # fall back to plain `timeout` on BusyBox (Alpine), and to direct exec + # when `timeout` is absent (stock macOS, where Ctrl+C works natively). + if timeout --foreground -k 10 1 true >/dev/null 2>&1; then + timeout --foreground -k 10 "$timeout_seconds" "$@" + else + timeout "$timeout_seconds" "$@" + fi else "$@" fi } +# Return success only when the host is an apt release NEWER than the newest one +# Playwright's platform resolver recognizes — the exact condition that makes +# `playwright install` hang uninterruptibly (#35166). We scope the override +# retry to this case rather than retrying on *any* failure, so a genuine +# network/disk/permission failure doesn't get a mismatched-glibc build forced +# onto it. Newest Playwright-known apt releases as of this writing: Ubuntu +# 24.04, Debian 13. Anything above triggers the fallback; everything Playwright +# already handles (and every non-apt distro) does not. +playwright_host_unrecognized() { + # Compare dotted versions: returns 0 if $1 > $2. + _ver_gt() { + [ "$1" = "$2" ] && return 1 + [ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | tail -n1)" = "$1" ] + } + case "$DISTRO" in + ubuntu) _ver_gt "${DISTRO_VERSION:-0}" "24.04" ;; + debian) _ver_gt "${DISTRO_VERSION:-0}" "13" ;; + *) return 1 ;; # Non-apt or unknown — not the #35166 hang condition. + esac +} + # Compute the PLAYWRIGHT_HOST_PLATFORM_OVERRIDE value to retry an install with # when Playwright's platform resolver rejects the host. ubuntu24.04 is the # newest Linux build Playwright has shipped across recent releases and runs on @@ -1912,14 +1950,19 @@ playwright_fallback_platform() { # Debian 14, future distros — see #35166), retry it ONCE with # PLAYWRIGHT_HOST_PLATFORM_OVERRIDE pinned to the newest known build. # -# This is deliberately try-native-first, override-only-on-failure rather than a -# hardcoded distro/version table: when Playwright already supports the host -# (e.g. Ubuntu 26.04 on Playwright >=1.61), the first attempt succeeds and the -# override never runs — so we never force a mismatched-glibc build onto a -# release Playwright handles correctly (microsoft/playwright#35114). It is also -# self-correcting: new distro releases work the moment Playwright adds them, -# with no change here. Playwright's maintainers bless this env var as the -# supported escape hatch for unrecognized platforms (microsoft/playwright#33434). +# The override retry is scoped to the actual hang condition: it fires only when +# the host is an apt release NEWER than Playwright recognizes +# (playwright_host_unrecognized). On every release Playwright already supports +# (Ubuntu <=24.04, Debian <=13) and every non-apt distro, the first attempt is +# authoritative and a failure is reported as-is — we never force a +# mismatched-glibc build (microsoft/playwright#35114) onto a host Playwright +# handles correctly. This is deliberately narrower than a retry-on-any-failure: +# a network/disk/permission error on a supported host should surface, not get +# papered over with a platform override. Playwright's maintainers bless this +# env var as the supported escape hatch for unrecognized platforms +# (microsoft/playwright#33434); a hardcoded full distro/version table was +# rejected upstream (microsoft/playwright#33432), so we only need the +# newest-known floor here. # # An operator-provided PLAYWRIGHT_HOST_PLATFORM_OVERRIDE is always respected: # it is inherited by the first attempt, and the retry is skipped. @@ -1940,14 +1983,21 @@ run_playwright_install() { return 1 fi + # Only retry with an override on the apt releases too new for Playwright to + # recognize (the #35166 hang). Any other failure is a real failure and is + # surfaced unchanged. + if ! playwright_host_unrecognized; then + return 1 + fi + local fallback fallback="$(playwright_fallback_platform)" if [ -z "$fallback" ]; then return 1 # No usable fallback build for this arch. fi - log_warn "Playwright didn't recognize this platform — retrying with PLAYWRIGHT_HOST_PLATFORM_OVERRIDE=$fallback" - log_info "(common on apt releases newer than Playwright knows, e.g. Ubuntu 26.04 / Debian 14; see #35166)" + log_warn "Playwright doesn't recognize ${DISTRO} ${DISTRO_VERSION} yet — retrying with PLAYWRIGHT_HOST_PLATFORM_OVERRIDE=$fallback" + log_info "(apt releases newer than Playwright knows hang at this step; see #35166)" PLAYWRIGHT_HOST_PLATFORM_OVERRIDE="$fallback" \ run_browser_install_with_timeout "$timeout_seconds" "$@" } diff --git a/tests/test_install_sh_browser_install.py b/tests/test_install_sh_browser_install.py index 80fb61a5efd..212bacfaa23 100644 --- a/tests/test_install_sh_browser_install.py +++ b/tests/test_install_sh_browser_install.py @@ -102,14 +102,16 @@ def test_playwright_install_retries_with_platform_override_on_failure() -> None: On apt releases newer than Playwright knows (Ubuntu 26.04, Debian 14, future distros) `playwright install` hangs/fails (#35166). run_playwright_install must retry ONCE with PLAYWRIGHT_HOST_PLATFORM_OVERRIDE pinned to the newest - known build — but only on failure (try-native-first, so a host Playwright - already supports keeps its native build and avoids the glibc mismatch in - microsoft/playwright#35114), and never when the operator pinned the value. + known build — but only when the host is one of those too-new apt releases + (playwright_host_unrecognized), never on a host Playwright already supports + (which would force a glibc mismatch, microsoft/playwright#35114), and never + when the operator pinned the value. """ text = INSTALL_SH.read_text() assert "run_playwright_install()" in text assert "playwright_fallback_platform()" in text + assert "playwright_host_unrecognized()" in text # Fallback target is the newest known build, arch-aware. assert 'echo "ubuntu24.04-x64"' in text assert 'echo "ubuntu24.04-arm64"' in text @@ -117,6 +119,170 @@ def test_playwright_install_retries_with_platform_override_on_failure() -> None: assert 'if run_browser_install_with_timeout "$timeout_seconds" "$@" 2>/dev/null; then' in text # Operator-pinned override is respected (retry skipped). assert 'if [ -n "${PLAYWRIGHT_HOST_PLATFORM_OVERRIDE:-}" ]; then' in text + # The retry is gated on the unrecognized-apt-release check, not any failure. + assert "if ! playwright_host_unrecognized; then" in text # The retry actually sets the override for the child process. assert 'PLAYWRIGHT_HOST_PLATFORM_OVERRIDE="$fallback" \\' in text + +def test_browser_install_timeout_stays_interruptible() -> None: + """The Playwright download must stay Ctrl+C-able and force-kill if wedged. + + GNU `timeout` runs the child in its own process group, so a terminal Ctrl+C + reaches `timeout` but never the download — it looks frozen and ignores + Ctrl+C (#35166). `--foreground` keeps it in the shell's foreground group; + `-k 10` guarantees a SIGKILL after the deadline. Both are GNU-only, so the + installer probes support once and falls back to plain `timeout`. + """ + text = INSTALL_SH.read_text() + + # GNU-flag probe + the guarded invocation must both be present. + assert "timeout --foreground -k 10 1 true" in text + assert 'timeout --foreground -k 10 "$timeout_seconds" "$@"' in text + # Plain-timeout fallback preserved for BusyBox/non-GNU. + assert 'timeout "$timeout_seconds" "$@"' in text + + +# --------------------------------------------------------------------------- +# Behavioral tests: source the install.sh helpers in a stubbed shell and assert +# the override retry fires ONLY on a too-new apt release (#35166), and not on a +# host Playwright already supports. +# --------------------------------------------------------------------------- + +import subprocess + + +def _run_install_fn(distro: str, version: str, *, native_fails: bool, + arch: str = "x86_64", operator_override: str = "") -> dict: + """Source the relevant functions from install.sh and drive run_playwright_install. + + Stubs `npx` (the install command) to fail/succeed, `uname -m` for arch, and + `log_warn`/`log_info` to no-ops. Returns parsed observations: how many times + the install command ran, and the override value seen on each run. + """ + # Extract the functions we need so we don't execute the whole installer. + fn_names = [ + "run_browser_install_with_timeout", + "playwright_host_unrecognized", + "playwright_fallback_platform", + "run_playwright_install", + ] + src = INSTALL_SH.read_text() + import re + + extracted = [] + for name in fn_names: + m = re.search(rf"^{re.escape(name)}\(\) \{{.*?^\}}", src, re.MULTILINE | re.DOTALL) + assert m, f"could not extract {name}() from install.sh" + extracted.append(m.group(0)) + body = "\n\n".join(extracted) + + native_rc = 1 if native_fails else 0 + harness = f""" +set -u +DISTRO={distro!r} +DISTRO_VERSION={version!r} +export PLAYWRIGHT_HOST_PLATFORM_OVERRIDE={operator_override!r} +[ -z "$PLAYWRIGHT_HOST_PLATFORM_OVERRIDE" ] && unset PLAYWRIGHT_HOST_PLATFORM_OVERRIDE + +log_warn() {{ :; }} +log_info() {{ :; }} + +# Stub `uname -m` for arch control without touching the real binary. +uname() {{ if [ "$1" = "-m" ]; then echo {arch!r}; else command uname "$@"; fi }} + +# Stub `timeout`: just run the command, ignoring flags/duration. We only care +# about how the npx stub behaves, not real timeout semantics here. +timeout() {{ + while [ $# -gt 0 ]; do + case "$1" in -*|[0-9]*) shift ;; *) break ;; esac + done + "$@" +}} + +# Stub the install command. Record each invocation + the override in effect. +npx() {{ + echo "RUN override=${{PLAYWRIGHT_HOST_PLATFORM_OVERRIDE:-}}" >>"$RUNLOG" + # First run reflects native_fails; the override retry (if any) succeeds. + if [ -n "${{PLAYWRIGHT_HOST_PLATFORM_OVERRIDE:-}}" ]; then return 0; fi + return {native_rc} +}} + +{body} + +run_playwright_install 600 npx playwright install --with-deps chromium +echo "FINAL_RC=$?" +""" + import tempfile, os + with tempfile.NamedTemporaryFile("w", suffix=".log", delete=False) as lf: + runlog = lf.name + try: + env = dict(os.environ, RUNLOG=runlog) + proc = subprocess.run(["bash", "-c", harness], capture_output=True, + text=True, env=env) + runs = Path(runlog).read_text().strip().splitlines() + final_rc = None + for line in proc.stdout.splitlines(): + if line.startswith("FINAL_RC="): + final_rc = int(line.split("=", 1)[1]) + return {"runs": runs, "final_rc": final_rc, "stderr": proc.stderr} + finally: + Path(runlog).unlink(missing_ok=True) + + +def test_override_retry_fires_on_ubuntu_26() -> None: + """Ubuntu 26.04 (too new) → native fails → retry with ubuntu24.04 override.""" + r = _run_install_fn("ubuntu", "26.04", native_fails=True) + assert len(r["runs"]) == 2, r["runs"] + assert "override=" in r["runs"][0] + assert "override=ubuntu24.04-x64" in r["runs"][1] + assert r["final_rc"] == 0 + + +def test_override_retry_does_not_fire_on_supported_ubuntu() -> None: + """Ubuntu 24.04 is recognized by Playwright → a failure is surfaced, no override.""" + r = _run_install_fn("ubuntu", "24.04", native_fails=True) + assert len(r["runs"]) == 1, r["runs"] + assert "override=" in r["runs"][0] + assert r["final_rc"] == 1 + + +def test_override_retry_does_not_fire_on_fedora() -> None: + """Non-apt distro never triggers the override retry, even on failure.""" + r = _run_install_fn("fedora", "42", native_fails=True) + assert len(r["runs"]) == 1, r["runs"] + assert r["final_rc"] == 1 + + +def test_override_retry_fires_on_debian_14() -> None: + """Debian 14 (> 13) is the too-new apt case → retry with override.""" + r = _run_install_fn("debian", "14", native_fails=True) + assert len(r["runs"]) == 2, r["runs"] + assert "override=ubuntu24.04-x64" in r["runs"][1] + assert r["final_rc"] == 0 + + +def test_no_retry_when_native_succeeds_on_ubuntu_26() -> None: + """Even on Ubuntu 26.04, a successful native install is never retried.""" + r = _run_install_fn("ubuntu", "26.04", native_fails=False) + assert len(r["runs"]) == 1, r["runs"] + assert "override=" in r["runs"][0] + assert r["final_rc"] == 0 + + +def test_operator_override_respected_no_second_run() -> None: + """An operator-pinned override applies to attempt 1; no second run on failure.""" + r = _run_install_fn("ubuntu", "26.04", native_fails=True, + operator_override="ubuntu22.04-x64") + # The override is set, so the npx stub returns 0 on the first run. + assert len(r["runs"]) == 1, r["runs"] + assert "override=ubuntu22.04-x64" in r["runs"][0] + assert r["final_rc"] == 0 + + +def test_override_retry_skipped_on_unsupported_arch() -> None: + """Ubuntu 26.04 on an arch with no Playwright build → no fallback retry.""" + r = _run_install_fn("ubuntu", "26.04", native_fails=True, arch="riscv64") + assert len(r["runs"]) == 1, r["runs"] + assert r["final_rc"] == 1 +