Skip to content

Commit 76c80cf

Browse files
committed
Puppeteer/Playwright: drop cache_dir field, derive purely from install_root
Per maintainer feedback: ``cache_dir`` was always 1:1 with ``install_root``, so the writable field was redundant. Collapse it to a single ``@computed_field`` on both providers: cache_dir = <install_root>/cache when install_root is set cache_dir = None when install_root is unset When ``install_root`` is unset the provider exports neither ``PUPPETEER_CACHE_DIR`` nor ``PLAYWRIGHT_BROWSERS_PATH`` to subprocesses, so ``puppeteer-browsers`` / ``playwright`` fall back to whatever the ambient env + their own OS default dictates. When ``install_root`` is pinned, the computed ``cache_dir`` flows through to ``ENV``, the sudo ``env KEY=VAL`` wrapper, ``--path=...`` install args, ``default_uninstall_handler``, and ``default_abspath_handler`` scope checks — all call sites already guarded on ``is not None`` so removing the field changes no behaviour when install_root is set. Provider field surface is now just ``install_root`` + ``bin_dir`` (plus the class-level ``INSTALLER_BIN`` constant), matching the maintainer's "only install_root, cache_dir, bin_dir; no duplicates" direction.
1 parent 2d52f0b commit 76c80cf

3 files changed

Lines changed: 43 additions & 49 deletions

File tree

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,11 +1074,11 @@ INSTALLER_BIN = "puppeteer-browsers"
10741074
PATH = ""
10751075
install_root = $ABXPKG_PUPPETEER_ROOT or $ABXPKG_LIB_DIR/puppeteer
10761076
bin_dir = <install_root>/bin
1077-
cache_dir = <install_root>/cache # hydrated from PUPPETEER_CACHE_DIR env when install_root is unset
1077+
cache_dir = <install_root>/cache # computed; None when install_root is unset
10781078
```
10791079

10801080
- Install root: set `install_root` for the root dir and `bin_dir` for symlinked executables. Leave it unset for ambient/global mode, where cache ownership stays with the host and `INSTALLER_BINARY` must already be resolvable from the ambient provider set.
1081-
- Cache dir / `PUPPETEER_CACHE_DIR`: browser downloads land in `cache_dir`, resolved with four-tier precedence at validate time: explicit `cache_dir` kwarg wins → else `PUPPETEER_CACHE_DIR` env var → else `<install_root>/cache` when an install root is pinned → else `None`. The resolved value is exported as `PUPPETEER_CACHE_DIR` to every subprocess the provider runs.
1081+
- Cache dir / `PUPPETEER_CACHE_DIR`: `cache_dir` is a computed property — it's always `<install_root>/cache` when `install_root` is pinned (and exported as `PUPPETEER_CACHE_DIR` to every subprocess), and `None` otherwise. When `install_root` is unset, abxpkg exports nothing and `puppeteer-browsers` falls back to its own default (`$PUPPETEER_CACHE_DIR` from the ambient env, otherwise `~/.cache/puppeteer`).
10821082
- Auto-switching: bootstraps `@puppeteer/browsers` through `NpmProvider` and then uses that CLI for browser installs.
10831083
- `dry_run`: shared behavior.
10841084
- Security: `min_release_age` is unsupported for browser installs and is ignored with a warning if explicitly requested. `postinstall_scripts=False` is supported for the underlying npm bootstrap path, and `ABXPKG_POSTINSTALL_SCRIPTS` hydrates the provider default here.
@@ -1097,12 +1097,12 @@ INSTALLER_BIN = "playwright"
10971097
PATH = ""
10981098
install_root = None # abxpkg-managed root dir for bin_dir / nested npm prefix
10991099
bin_dir = <install_root>/bin # symlink dir for resolved browsers
1100-
cache_dir = <install_root>/cache # hydrated from PLAYWRIGHT_BROWSERS_PATH env when install_root is unset
1100+
cache_dir = <install_root>/cache # computed; None when install_root is unset
11011101
euid = 0 # routes exec() through sudo-first-then-fallback
11021102
```
11031103

11041104
- Install root: set `install_root` to pin the abxpkg-managed root dir (where `bin_dir` symlinks and the nested npm prefix live). Leave it unset to let playwright use its own OS-default browsers path (`~/.cache/ms-playwright` on Linux etc.) — in that case abxpkg maintains no symlink dir or npm prefix at all, the `playwright` npm CLI bootstraps against the host's npm default, and `load()` returns the resolved `executablePath()` directly. `bin_dir` overrides the symlink directory when `install_root` is pinned.
1105-
- Cache dir / `PLAYWRIGHT_BROWSERS_PATH`: browser downloads land in `cache_dir`, resolved with four-tier precedence at validate time: explicit `cache_dir` kwarg wins → else `PLAYWRIGHT_BROWSERS_PATH` env var → else `<install_root>/cache` when an install root is pinned → else `None` (let playwright pick its OS default). The final value is exported as `PLAYWRIGHT_BROWSERS_PATH` to every subprocess the provider runs. `uninstall()` deletes matching `<browser>-*/` dirs from the resolved `cache_dir` (never from playwright's untouched OS-default cache when nothing is pinned).
1105+
- Cache dir / `PLAYWRIGHT_BROWSERS_PATH`: `cache_dir` is a computed property — it's always `<install_root>/cache` when `install_root` is pinned (and exported as `PLAYWRIGHT_BROWSERS_PATH` to every subprocess, including the `env KEY=VAL -- ...` wrapper used when we go through sudo), and `None` otherwise. When `install_root` is unset, abxpkg exports nothing and `playwright` falls back to its own default (`$PLAYWRIGHT_BROWSERS_PATH` from the ambient env, otherwise `~/.cache/ms-playwright` on Linux). `uninstall()` deletes matching `<browser>-*/` dirs from the resolved `cache_dir` and never touches playwright's OS-default cache when nothing is pinned.
11061106
- Auto-switching: bootstraps the `playwright` npm package through `NpmProvider`, then runs `playwright install --with-deps <install_args>` against it. Resolves each installed browser's real executable via the `playwright-core` Node.js API (`chromium.executablePath()` etc.) and writes a symlink into `bin_dir` when one is configured.
11071107
- `dry_run`: shared behavior — the install handler short-circuits to a placeholder without touching the host.
11081108
- Privilege handling: `--with-deps` installs system packages and requires root on Linux. ``euid`` defaults to ``0``, which routes every ``exec()`` call through the base ``BinProvider.exec`` sudo-first-then-fallback path — it tries ``sudo -n -- playwright install --with-deps ...`` first on non-root hosts, falls back to running the command directly if sudo fails or isn't available, and merges both stderr outputs into the final error if both attempts fail.

abxpkg/binprovider_playwright.py

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,8 @@
88
import sys
99
import platform
1010
from pathlib import Path
11-
from typing import Self
1211

13-
from pydantic import Field, TypeAdapter, computed_field, model_validator
12+
from pydantic import Field, TypeAdapter, computed_field
1413

1514
from .base_types import (
1615
BinName,
@@ -37,15 +36,14 @@ class PlaywrightProvider(BinProvider):
3736
``playwright`` npm package. When ``playwright_root`` is set it acts
3837
as the abxpkg install root: a dedicated npm prefix is nested under
3938
it, ``bin_dir`` surfaces each requested browser so ``load(bin_name)``
40-
finds it directly, and ``PLAYWRIGHT_BROWSERS_PATH`` defaults to
41-
``<install_root>/cache`` for the actual browser downloads. Callers
42-
that want to pin the browsers path somewhere else can set the
43-
``browser_cache_dir`` field (hydrated from ``PLAYWRIGHT_BROWSERS_PATH``
44-
env) — that override always wins over ``install_root/cache``.
39+
finds it directly, and ``PLAYWRIGHT_BROWSERS_PATH`` is pinned to
40+
``<install_root>/cache`` for every subprocess the provider runs.
4541
When ``playwright_root`` is left unset, playwright picks its own
46-
default browsers path, the npm CLI bootstraps against the host's
47-
npm default, and ``load()`` returns the resolved ``executablePath()``
48-
directly without creating any install_root/bin_dir symlinks.
42+
default browsers path (``$PLAYWRIGHT_BROWSERS_PATH`` from the
43+
ambient env, otherwise ``~/.cache/ms-playwright`` on Linux), the
44+
npm CLI bootstraps against the host's npm default, and ``load()``
45+
returns the resolved ``executablePath()`` directly without
46+
creating any install_root/bin_dir symlinks.
4947
5048
``--with-deps`` installs system packages and requires root on
5149
Linux, so ``euid`` defaults to ``0``: the base ``BinProvider.exec``
@@ -73,31 +71,27 @@ class PlaywrightProvider(BinProvider):
7371
# Only set in managed mode: setup()/default_abspath_handler() use it to create and read
7472
# stable browser shims under ``<install_root>/bin``; global mode leaves it unset.
7573
bin_dir: Path | None = None
76-
# Where browsers get downloaded. Exported as ``PLAYWRIGHT_BROWSERS_PATH``
77-
# to every subprocess. Precedence at validate time:
78-
# 1. explicit ``cache_dir`` kwarg wins (more-specific override)
79-
# 2. else ``PLAYWRIGHT_BROWSERS_PATH`` env var wins (same semantics
80-
# for external callers invoking via env)
81-
# 3. else ``<install_root>/cache`` when an install root is pinned
82-
# 4. else ``None`` (global mode — let playwright pick its OS default)
83-
cache_dir: Path | None = Field(
84-
default_factory=lambda: (
85-
Path(os.environ["PLAYWRIGHT_BROWSERS_PATH"]).expanduser()
86-
if os.environ.get("PLAYWRIGHT_BROWSERS_PATH")
87-
else None
88-
),
89-
)
9074

9175
# Only Linux needs the sudo-first execution path for
9276
# ``playwright install --with-deps``. On macOS and elsewhere,
9377
# run as the normal user by default.
9478
euid: int | None = 0 if platform.system().lower() == "linux" else None
9579

96-
@model_validator(mode="after")
97-
def _fill_cache_dir_from_install_root(self) -> Self:
98-
if self.cache_dir is None and self.install_root is not None:
99-
self.cache_dir = self.install_root / "cache"
100-
return self
80+
@computed_field
81+
@property
82+
def cache_dir(self) -> Path | None:
83+
"""Where browser downloads land.
84+
85+
When ``install_root`` is pinned we always use
86+
``<install_root>/cache`` and export it as ``PLAYWRIGHT_BROWSERS_PATH``
87+
to every subprocess. When ``install_root`` is unset we leave it
88+
``None`` and let ``playwright`` fall back to its own default
89+
(``$PLAYWRIGHT_BROWSERS_PATH`` from the ambient env, otherwise
90+
``~/.cache/ms-playwright`` on Linux etc.).
91+
"""
92+
if self.install_root is not None:
93+
return self.install_root / "cache"
94+
return None
10195

10296
@computed_field
10397
@property

abxpkg/binprovider_puppeteer.py

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,22 @@ class PuppeteerProvider(BinProvider):
6666
# Only set in managed mode: setup()/default_abspath_handler() use it to expose stable
6767
# browser launch shims under ``<install_root>/bin``; global mode leaves it unset.
6868
bin_dir: Path | None = None
69-
# Where browsers get downloaded. Precedence at validate time:
70-
# 1. explicit ``cache_dir`` kwarg wins (more-specific override)
71-
# 2. else ``PUPPETEER_CACHE_DIR`` env var wins (same semantics for
72-
# external callers invoking via env)
73-
# 3. else ``<install_root>/cache`` when an install root is pinned
74-
# 4. else ``None`` (global mode — let puppeteer-browsers pick its
75-
# own default cache)
76-
cache_dir: Path | None = Field(
77-
default_factory=lambda: (
78-
Path(os.environ["PUPPETEER_CACHE_DIR"]).expanduser()
79-
if os.environ.get("PUPPETEER_CACHE_DIR")
80-
else None
81-
),
82-
)
69+
70+
@computed_field
71+
@property
72+
def cache_dir(self) -> Path | None:
73+
"""Where browser downloads land.
74+
75+
When ``install_root`` is pinned we always use
76+
``<install_root>/cache`` and export it as ``PUPPETEER_CACHE_DIR``
77+
to every subprocess. When ``install_root`` is unset we leave it
78+
``None`` and let ``puppeteer-browsers`` fall back to its own
79+
default (``$PUPPETEER_CACHE_DIR`` from the ambient env, otherwise
80+
``~/.cache/puppeteer``).
81+
"""
82+
if self.install_root is not None:
83+
return self.install_root / "cache"
84+
return None
8385

8486
@computed_field
8587
@property
@@ -95,8 +97,6 @@ def supports_postinstall_disable(self, action, no_cache: bool = False) -> bool:
9597
def detect_euid_to_use(self) -> Self:
9698
if self.bin_dir is None and self.install_root is not None:
9799
self.bin_dir = self.install_root / "bin"
98-
if self.cache_dir is None and self.install_root is not None:
99-
self.cache_dir = self.install_root / "cache"
100100
return self
101101

102102
def setup_PATH(self, no_cache: bool = False) -> None:

0 commit comments

Comments
 (0)