From 634a66b1e40466abb07273616a1e62008ecf4df0 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sat, 19 Sep 2026 11:30:50 +0200 Subject: [PATCH] build(deps): declare node-gyp for the embedded MPV native build (#1625) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `apps/electron-backend/build-embedded-mpv.js` resolves the compiler with `require.resolve('node-gyp/bin/node-gyp.js')` and its comment claimed the package was "a declared devDependency" — it never was. node-gyp reached the tree only as a transitive of `@electron/rebuild`, in pnpm's hidden hoist (`node_modules/.pnpm/node_modules`). pnpm's `.bin` shims export that directory on NODE_PATH, which is why `pnpm nx …`, `pnpm run build:backend` and CI kept building the addon, while a plain `node apps/electron-backend/build-embedded-mpv.js` on a clean install failed with "Unable to resolve node-gyp". Declare node-gyp 12.4.0 (the version already in the lockfile store) as a root devDependency so the resolution no longer depends on a shim implementation detail, correct the stale comment, and record the contract in the embedded-MPV architecture doc plus the Agent Bootstrap notes. No packaged-build change: the no-runtime skip and its `embedded-mpv-unavailable.txt` marker are untouched. Co-authored-by: Claude Fable 5.1 --- .changes/deps-node-gyp-devdependency.md | 9 +++++++++ AGENTS.md | 7 +++++++ CLAUDE.md | 7 +++++++ apps/electron-backend/build-embedded-mpv.js | 14 +++++++++----- docs/architecture/embedded-mpv-native.md | 7 +++++++ package.json | 1 + pnpm-lock.yaml | 3 +++ 7 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 .changes/deps-node-gyp-devdependency.md diff --git a/.changes/deps-node-gyp-devdependency.md b/.changes/deps-node-gyp-devdependency.md new file mode 100644 index 000000000..b467983fc --- /dev/null +++ b/.changes/deps-node-gyp-devdependency.md @@ -0,0 +1,9 @@ +--- +type: internal +area: deps +--- + +The Embedded MPV native addon build now declares its `node-gyp` dependency +explicitly, so `node apps/electron-backend/build-embedded-mpv.js` and the +Homebrew development scripts work on a clean checkout instead of failing with +"Unable to resolve node-gyp". Packaged builds are unchanged. diff --git a/AGENTS.md b/AGENTS.md index 82a8400e6..17098fbd9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,6 +37,13 @@ This file provides guidance to coding agents working in this repository. and run `pnpm run deps:electron-builder:test` after related dependency updates — the test fails when the patched version no longer matches the installed one. +- `node-gyp` is a declared root devDependency because + `apps/electron-backend/build-embedded-mpv.js` resolves it with + `require.resolve`. Do not drop it as "unused": without the declaration it is + reachable only through pnpm's hidden hoist (`node_modules/.pnpm/node_modules`), + which pnpm's `.bin` shims put on `NODE_PATH` — so `pnpm nx …` and CI keep + working while a plain `node apps/electron-backend/build-embedded-mpv.js` + fails on a clean install with "Unable to resolve node-gyp". - `nx-electron@22.0.0` uses a local Nx 23 export-path patch and an explicit `webpack-node-externals` package extension. Scoped peer allowances for it and `ngx-indexed-db@22.0.0` live in `pnpm-workspace.yaml`; they are project diff --git a/CLAUDE.md b/CLAUDE.md index 32f074aa9..9a585212f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -115,6 +115,13 @@ pnpm nx show projects and run `pnpm run deps:electron-builder:test` after related dependency updates — the test fails when the patched version no longer matches the installed one. +- `node-gyp` is a declared root devDependency because + `apps/electron-backend/build-embedded-mpv.js` resolves it with + `require.resolve`. Do not drop it as "unused": without the declaration it is + reachable only through pnpm's hidden hoist (`node_modules/.pnpm/node_modules`), + which pnpm's `.bin` shims put on `NODE_PATH` — so `pnpm nx …` and CI keep + working while a plain `node apps/electron-backend/build-embedded-mpv.js` + fails on a clean install with "Unable to resolve node-gyp". - `nx-electron@22.0.0` uses a local Nx 23 export-path patch and an explicit `webpack-node-externals` package extension. Scoped peer allowances for it and `ngx-indexed-db@22.0.0` live in `pnpm-workspace.yaml`; they are project diff --git a/apps/electron-backend/build-embedded-mpv.js b/apps/electron-backend/build-embedded-mpv.js index ad7ca63ae..a21142c89 100644 --- a/apps/electron-backend/build-embedded-mpv.js +++ b/apps/electron-backend/build-embedded-mpv.js @@ -677,11 +677,15 @@ function copyGenericRuntimeToNativeBuild(runtime) { return manifest; } -// Upstream node-gyp, resolved as a declared devDependency. This used to scan -// node_modules/.pnpm for the `@electron/node-gyp` fork, which was only ever in -// the tree as a transitive of `@electron/rebuild` 3 — rebuild 4 moved to -// upstream `node-gyp` and the scan started throwing. The Electron target is -// selected through the npm_config_* env below, not by the binary. +// Upstream node-gyp, declared as a root devDependency so this resolves from a +// plain `node apps/electron-backend/build-embedded-mpv.js`. Until it was +// declared it was only in the tree as a transitive of `@electron/rebuild`, +// reachable through pnpm's hidden hoist (node_modules/.pnpm/node_modules) — +// which pnpm's `.bin` shims put on NODE_PATH, so `pnpm nx …` and CI worked +// while a direct invocation on a clean install threw. Before that, this +// scanned node_modules/.pnpm for the `@electron/node-gyp` fork, which +// rebuild 4 dropped for upstream `node-gyp`. The Electron target is selected +// through the npm_config_* env below, not by the binary. function resolveNodeGypBin() { try { return require.resolve('node-gyp/bin/node-gyp.js'); diff --git a/docs/architecture/embedded-mpv-native.md b/docs/architecture/embedded-mpv-native.md index 9fff4d5f0..c8541e829 100644 --- a/docs/architecture/embedded-mpv-native.md +++ b/docs/architecture/embedded-mpv-native.md @@ -987,6 +987,13 @@ The Electron main process holds an `electron.powerSaveBlocker` of type `prevent- Current development behavior: - The addon build supports `darwin`, `win32`, and `linux`; Windows and Linux builds require running on that target OS. +- The addon is compiled by upstream `node-gyp`, declared as a root + `devDependency` and resolved with `require.resolve` in + `apps/electron-backend/build-embedded-mpv.js`. That resolution must not rely + on the `NODE_PATH` that pnpm's `.bin` shims export: it only reaches pnpm's + hidden hoist (`node_modules/.pnpm/node_modules`), so an undeclared + `node-gyp` works under `pnpm nx …` and in CI but fails from a plain + `node apps/electron-backend/build-embedded-mpv.js` on a clean install. - The build script first looks for staged inputs at `vendor/embedded-mpv/-/`. On Linux, local development can fall back to distribution `libmpv-dev` headers and libraries. `LIBMPV_INCLUDE_DIR` overrides the header root. `LINUX_NATIVE_LIBRARY_DIR` is a link-time override and must name a directory already visible to the system dynamic loader; it is never inherited as helper `LD_LIBRARY_PATH`. - When the staged-input path is used, it must contain `include/mpv/client.h`, `runtime-manifest.json`, and the platform runtime/build files. The Linux diff --git a/package.json b/package.json index 6847a3d1f..1b1eaeebb 100644 --- a/package.json +++ b/package.json @@ -225,6 +225,7 @@ "material-design-icons-iconfont": "6.7.0", "mrmime": "2.0.1", "ng-mocks": "14.17.6", + "node-gyp": "12.4.0", "nx": "23.2.1", "nx-electron": "22.0.0", "prettier": "^3.9.6", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 247db21cc..7cc645c09 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -438,6 +438,9 @@ importers: ng-mocks: specifier: 14.17.6 version: 14.17.6(c5ca909b0ec8f50b848b5bca41287c00) + node-gyp: + specifier: 12.4.0 + version: 12.4.0 nx: specifier: 23.2.1 version: 23.2.1(@swc-node/register@1.12.1(@emnapi/core@1.11.3)(@emnapi/runtime@1.11.3)(@swc/core@1.16.1(@swc/helpers@0.5.23))(@swc/types@0.1.28)(typescript@6.0.3))(@swc/core@1.16.1(@swc/helpers@0.5.23))