[PHP] Support extraFiles for custom PHP extensions - #3580
Conversation
6ab5887 to
5121349
Compare
105ae20 to
696a5a3
Compare
696a5a3 to
7eea0f4
Compare
7eea0f4 to
51cfce5
Compare
51cfce5 to
290b2ce
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for manifest-declared “sidecar” files/directories when resolving PHP extensions from external manifests, enabling assets (e.g., writable data dir, UI files) to be staged alongside the .so.
Changes:
- Extend
PHPExtensionManifest/PHPExtensionManifestArtifactto declareextraFilesand resolve+fetch those assets relative tomanifestUrl/baseUrl. - Merge manifest-provided
extraFileswith caller-providedextraFilesand expose them on the resolved extension for staging. - Add/adjust tests (universal + node) and improve WordPress legacy boot test resiliency (retry on known console error, admin indicator predicate).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/playground/wordpress/tests/test-legacy-wp-version-boot.mjs | Refactors admin indicators and adds a targeted retry path when the front page boot fails due to a known console error. |
| packages/php-wasm/universal/src/lib/load-extension.ts | Adds manifest extraFiles types, validation, fetch/merge logic, and plumbs resolved sidecar assets through extension resolution. |
| packages/php-wasm/universal/src/lib/load-extension.spec.ts | Adds coverage for resolving sidecar files and rejecting escaping paths. |
| packages/php-wasm/universal/src/lib/index.ts | Re-exports new manifest sidecar types. |
| packages/php-wasm/node/src/test/with-php-extensions.spec.ts | Extends node integration test to ensure sidecar directories/files are staged into the VFS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (extraFiles.files ?? []).map(async (file) => { | ||
| const fileUrl = new URL(file.file, baseUrl); | ||
| const response = await fetchFn(fileUrl); |
There was a problem hiding this comment.
Leaving this one open for now. A protocol allowlist should be decided consistently for extension artifact URLs and sidecar URLs, while preserving the Node wrapper behavior for local manifests / file: URLs.
Inline single-call helpers, drop redundant validation passes on extra files, collapse fetch+ok+arrayBuffer into one helper, and tighten the extra-files merge so groups must fully agree on targetPath (no mixing declared and implicit roots).
ResolvedPHPExtension structurally only carries one targetPath, so the merged result can only ever stage files under a single VFS root. Take the first declared targetPath and let the install path use it; callers who want a different root should use a separate extension entry.
Resolved ResolvedPHPExtension.extraFiles is now a flat record of absolute VFS paths. Manifest groups use vfsRoot+vfsPath (joined into absolute paths during resolution), and directory nodes are split out from file nodes. installPHPExtensionFilesSync writes each path verbatim instead of joining against a separate targetPath. Schema and consumers updated to match.
Inline the source resolver into resolvePHPExtension and produce absolute VFS paths up front by joining each manifest group's vfsRoot with the node's vfsPath. Drop the mergeExtraFiles helper and trust the input files. Make manifest extra-file sourcePath optional so directory nodes don't need a phantom URL. Resolved extraNodes is now a flat record of absolute paths, and installPHPExtensionFilesSync writes each one verbatim. Schema and consumers updated to match.
Add an integration recipe to load-extension.ts (loadNodeRuntime/loadWebRuntime example, then the resolvePHPExtension + withResolvedPHPExtensions pair) and update 07-php-extensions.md for the new manifest shape — sourcePath instead of file, an extraFiles example with vfsRoot + nodes, absolute keys for caller-supplied sidecar files, and dropped sha256.
…tallOptions The rolled .d.ts in playground-client choked on the "ResolvedInstallOptions as PHPExtensionInstallOptions" alias, leaving PHPExtensionInstallOptions undefined. Use ResolvedInstallOptions everywhere.
…tallOptions Same fix as before — the rolled .d.ts in playground-client chokes on "DataToResolvePhpExtension as ResolvedInstallOptions". Make DataToResolvePhpExtension a real type alias and export ResolvedInstallOptions directly. Also rename a stale extraNodes doc reference to extraFiles.
|
I'll go ahead and merge |
…tra-files (#3596) ## What it does Updates `@php-wasm/compile-extension` to emit manifests in the shape the loader merged in #3580 expects, and adds a `--extra-files <hostDir>:<vfsRoot>` CLI flag for staging sidecar assets (web UIs, data dirs, etc.) alongside the `.so` files. The package's manifest output now matches `PHPExtensionManifest` from `@php-wasm/universal` exactly: - `artifacts[i].file` → `artifacts[i].sourcePath` - `sha256` removed (the loader dropped verification in #3580) - `extraFiles` with `vfsRoot` + `nodes` is supported at both the top-level and per-artifact ## Why The loader merged in #3580 validates manifests against a strict schema generated from `PHPExtensionManifest`, and rejects the old shape. Without this update, anything built with the published 3.1.25 fails at runtime with "Invalid PHP extension manifest". This is the prerequisite for stacking an SPX build PR that uses `@php-wasm/compile-extension` to produce the `.so` and the new manifest API to stage SPX's web UI files. ## How to use --extra-files ```bash npx @php-wasm/compile-extension \ --source ./spx-src \ --name spx \ --php-versions 8.2 \ --extra-files ./web-ui:/internal/shared/spx \ --out ./dist ``` `./web-ui` is copied next to the manifest. Each file becomes one node under `extraFiles.nodes` with `vfsPath` relative to `vfsRoot` and `sourcePath` relative to the manifest URL. Empty directories are recorded as `type: "directory"` nodes so the loader creates them before PHP starts. Multiple `--extra-files` entries are allowed; they must agree on `vfsRoot` because the manifest format stores a single root per group. ## Testing instructions ```bash npm exec nx -- test php-wasm-compile-extension npm exec nx -- typecheck php-wasm-compile-extension npm exec nx -- lint php-wasm-compile-extension ``` The existing integration suite (`tests/run-integration-tests.sh`) keeps working because the loader-side tests already pass `manifestUrl` straight through and use the new manifest shape.
## What it does Builds a Playground-loadable `wp_mysql_parser` WASM PHP extension bundle for PHP `8.0` through `8.5`. On `trunk` pushes or manual `workflow_dispatch`, the new publish workflow: 1. Builds one JSPI side module per PHP version. 2. Uploads the collected bundle as an Actions artifact named `wp_mysql_parser-wasm-extension`. 3. Publishes the same files to the `gh-pages` branch: ```text wp_mysql_parser-wasm-extension/latest/manifest.json wp_mysql_parser-wasm-extension/<commit-sha>/manifest.json ``` The `latest/` path is replaced on every publish. The `<commit-sha>/` path is immutable. ## Rationale Playground needs public `manifest.json` and `.so` files it can fetch at runtime. An Actions artifact is useful for debugging, but it is not a stable public URL and should not be treated as a production plugin release. Publishing the bundle from `gh-pages` gives Playground a static URL without repackaging the WordPress plugin itself. The repository still needs GitHub Pages configured to publish from the `gh-pages` branch root before these URLs resolve: ```text https://wordpress.github.io/sqlite-database-integration/wp_mysql_parser-wasm-extension/latest/manifest.json https://wordpress.github.io/sqlite-database-integration/wp_mysql_parser-wasm-extension/<commit-sha>/manifest.json ``` PHP `7.4` is intentionally excluded. The Rust path uses `ext-php-rs` `0.15`, which depends on PHP 8 Zend APIs. ## Implementation Adds `.github/workflows/publish-wasm-extension-artifact.yml` with a serialized PHP matrix for `8.5`, `8.4`, `8.3`, `8.2`, `8.1`, and `8.0`. The package job downloads the per-version `.so` artifacts, writes a combined Playground extension manifest, writes `SHA256SUMS`, uploads the bundle, then commits the static bundle to `gh-pages`. The manifest follows the WordPress Playground extension format from WordPress/wordpress-playground#3580: ```json { "name": "wp_mysql_parser", "mode": "php-extension", "artifacts": [ { "phpVersion": "8.4", "sourcePath": "wp_mysql_parser-php8.4-jspi.so" } ] } ``` The manifest uses `sourcePath` and does not include the retired `file` or `sha256` artifact fields. Checksums are published separately in `SHA256SUMS`. The Rust build now runs the published `@php-wasm/compile-extension@3.1.27` CLI from an isolated temporary npm prefix. A sparse `WordPress/wordpress-playground` checkout is still required for `packages/php-wasm/compile` Docker assets and for the Playground load smoke test. Follow-up for Playground: make `@php-wasm/compile-extension` self-contained so external extension projects do not need to shallow or sparse checkout `WordPress/wordpress-playground` just to access Docker assets or test harness files. ## Testing instructions Run the local static checks: ```bash bash -n packages/php-ext-wp-mysql-parser/wasm-spike/build-in-docker-rust.sh actionlint .github/workflows/wasm-spike.yml .github/workflows/publish-wasm-extension-artifact.yml node --check packages/php-ext-wp-mysql-parser/wasm-spike/write-extension-manifest.mjs git diff --check ``` Verify the PR `WASM extension build` check is green for PHP `8.0` through `8.5`. The latest PR run passed all six PHP versions. After merge, run `Publish WASM extension artifact` with `workflow_dispatch` or let a matching `trunk` push trigger it, then verify both published manifests resolve from GitHub Pages once Pages is configured for the `gh-pages` branch root.
What it does
Adds manifest-declared sidecar files to the PHP extension loading API introduced in #3566, and folds in #3590's generated AJV validator for extension manifests.
A manifest can now describe files and directories that must be staged before PHP starts, alongside the selected
.soartifact. Each node's final VFS path isvfsRoot + vfsPath:{ "name": "spx", "artifacts": [ { "phpVersion": "8.4", "sourcePath": "spx.so", "extraFiles": { "vfsRoot": "/internal/shared/spx", "nodes": [ { "vfsPath": "data", "type": "directory" }, { "vfsPath": "ui/index.html", "sourcePath": "ui/index.html" } ] } } ] }Rationale
#3255 needs SPX to load more than a WASM side module. The extension also expects a writable data directory and web UI assets such as HTML, CSS, and JS files.
This PR keeps that work focused on the loader contract. It does not rebuild PHP 8.2, ship SPX artifacts, or claim that SPX loads today. A follow-up can use this manifest shape when adding or rebuilding SPX artifacts.
Integration
For most consumers nothing changes structurally — manifest sources slot into the same
extensionsarray as bundled extension names:For hand-rolled Emscripten setups,
resolvePHPExtension+withResolvedPHPExtensionsin@php-wasm/universalare the lower-level pair.Implementation
PHPExtensionManifestacceptsextraFilesat two levels:extraFilesfor files shared by every artifactextraFilesfor PHP-version-specific filesEach group has a
vfsRoot(the absolute VFS prefix) and an array ofnodeswith avfsPath, an optionaltype: 'file' | 'directory', and asourcePath(the URL to fetch, only required for files). The resolver joinsvfsRoot + vfsPathinto absolute VFS paths and returns:The resolver validates untrusted manifest JSON with a standalone AJV validator generated from the public
PHPExtensionManifestTypeScript type. Sidecar files are fetched relative to the manifest URL or inline manifestbaseUrl, with manifest-level and selected artifact-level nodes fetched in parallel through a shared concurrency limit.Caller-supplied
extraFilesonPHPExtensionInstallOptionsuse the same absolute-path shape ({ files, directories? }) — notargetPathindirection.External extension manifests remain JSPI-only.
Breaking changes
artifact.file→artifact.sourcePath;extraFiles.targetPath→vfsRoot;extraFiles.files/extraFiles.directories→ unifiednodesarray withvfsPath/type/sourcePath.PHPExtensionExtraFiles(caller-supplied sidecar files) is now flat absolute VFS paths underfilesanddirectories. The previoustargetPath + relative pathsshape is gone.sha256fields removed from sources and artifacts.ResolvePHPExtensionOptionsis no longer exported (usePHPExtensionInstallOptions & { phpVersion: string }).Testing instructions
```bash
npm exec nx -- test php-wasm-universal --testFile=load-extension.spec.ts
npm exec nx -- typecheck php-wasm-universal
npm exec nx -- lint php-wasm-universal
npm exec nx -- build php-wasm-universal
cd packages/php-wasm/node && npx vitest run src/test/with-php-extensions.spec.ts --config vite.jspi.config.ts
npm exec nx -- typecheck php-wasm-node
npm exec nx -- lint php-wasm-node
npm exec nx -- typecheck php-wasm-web
```