Fix native php blueprint#3491
Conversation
📊 Performance Test ResultsComparing e8a9402 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
fredrikekelund
left a comment
fredrikekelund
left a comment
There was a problem hiding this comment.
This fixes the problem, which is great! @bcotrim, you mentioned seeing some trouble with images on this branch, but I can upload and view images on the site fine. Let me know what problems you were experiencing.
Really, the only thing I think we should explore here (although it could be a follow-up PR) is using php.ini for all PHP processes on Windows, not just when running blueprints. This would help with simplicity in the implementation and predictability at runtime.
There was a problem hiding this comment.
I now know you did patch it with the changes from WordPress/php-toolkit#280. I've since also patched it with the changes from WordPress/php-toolkit#281
There was a problem hiding this comment.
I guess it doesn't hurt, but I'm a little bit allergic to unit tests that LLMs generate out of habit. I think E2E tests serve us much better for truly testing this functionality (we already have E2E tests that do that)
There was a problem hiding this comment.
I went ahead and removed the test file in e834425
| // blueprints.phar spawns its own PHP subprocesses while applying a blueprint. | ||
| // Those subprocesses inherit PHPRC but not the parent process's `-d` argv, so | ||
| // this php.ini carries the bundled extension and CA settings they need. | ||
| export async function createNativePhpSubprocessIniDirectory( | ||
| phpVersion: NativePhpSupportedVersion | ||
| ): Promise< string > { | ||
| const tempRoot = | ||
| process.platform === 'win32' ? fs.realpathSync.native( os.tmpdir() ) : os.tmpdir(); | ||
| const phpIniDirectory = await fs.promises.mkdtemp( path.join( tempRoot, 'studio-native-php-' ) ); | ||
| const caBundlePath = getNativePhpCaBundlePath( phpIniDirectory ); | ||
| await fs.promises.writeFile( caBundlePath, rootCertificates.join( os.EOL ), 'utf8' ); | ||
| await fs.promises.writeFile( | ||
| path.join( phpIniDirectory, 'php.ini' ), | ||
| getNativePhpSubprocessIniContents( phpVersion, caBundlePath ), | ||
| 'utf8' | ||
| ); | ||
| return phpIniDirectory; | ||
| } |
There was a problem hiding this comment.
The php.ini contents only change with PHP versions. To keep things simple, I think we should explore writing the php.ini file to the PHP executable's directory and using that as the source of truth for the PHP config for all Windows PHP processes. Did you try this already, @bcotrim?
|
I implemented my suggestion to always load the |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // and is fussy about backslashes (which also act as escape characters). Use | ||
| // forward slashes everywhere and escape stray double quotes. | ||
| function toPhpIniPath( filePath: string ): string { | ||
| return filePath.replace( /\\/g, '/' ).replace( /"/g, '\\"' ); |
|
The E2E tests aren't passing on Windows, but that's expected. We need to build and upload the new PHP packages to the CDN before it'll work |
### Related issues - Resolves STU-1763 - Upstream fix: WordPress/php-toolkit#294 - Upstream issue: WordPress/php-toolkit#292 ### Proposed Changes - Replace bundled `wp-files/blueprints/blueprints.phar` with the v0.8.1 release asset from `WordPress/php-toolkit`. - Old sha256: `ff7f5657961f8b9a604f7131c4eac5c2673a3f7665a304268e2287d24c0c7a0f` (2,294,271 bytes) - New sha256: `05503c954e4c3caea0da8e7d9c28bb5e5470e8121c41e4713c262e2cf4af0d16` (1,781,056 bytes) - Asset: https://github.com/WordPress/php-toolkit/releases/download/v0.8.1/blueprints.phar The previously bundled PHAR built `refs/heads/HEAD` as a literal branch name when callers passed `ref: "HEAD"`, so any Blueprint using `git:directory` with `ref: "HEAD"` failed on the native PHP runtime with `GitRemoteException: Branch "refs/heads/HEAD" not found on remote origin`. v0.8.1 ships the upstream fix. This PR is intentionally scoped to the PHAR bump — auto-install (removed in #3491) is intentionally not touched here. ### Testing Instructions 1. Pull this branch and run `npm install` (no source changes; the PHAR is the only file that moves). 2. In Studio, enable the **Native PHP runtime** beta flag. 3. Create a new site from the **WooCommerce** blueprint. 4. Confirm the site is created without the `Failed to start WordPress server: PHP command failed` error and that `wp-content/plugins/wc-smooth-generator/` is present in the new site. ### Pre-merge Checklist - [ ] Have you checked for TypeScript, React or other console errors? --------- Co-authored-by: Fredrik Rombach Ekelund <fredrik.rombach.ekelund@automattic.com> Co-authored-by: Fredrik Rombach Ekelund <fredrik@f26d.dev>
4 participants
Related issues
How AI was used in this PR
Codex was used interactively to debug the native PHP blueprint startup failure, inspect daemon/PHP behavior on Windows, and draft the focused fix. The behavior was manually reproduced before the fix and validated afterward in both CLI and Studio flows.
Proposed Changes
php.inifor blueprint PHAR execution.pdo_sqliteandsqlite3, so the PHAR can detect the SQLite-backed WordPress install correctly.openssl.cafileandcurl.cainfo, fixing HTTPS downloads from blueprint steps under native PHP.php.inidirectory after the blueprint run.Testing Instructions
npx eslint --fix apps/cli/lib/native-php.ts apps/cli/php-server-child.ts apps/cli/lib/tests/native-php.test.tsnpm run typechecknpm test -- apps/cli/lib/tests/native-php.test.ts apps/cli/lib/tests/wordpress-server-manager.test.tsnpm run cli:build{}and verify native PHP site creation succeeds:$env:STUDIO_RUNTIME = "native-php"node apps/cli/dist/cli/main.mjs --path "$env:USERPROFILE\Studio\native-empty-blueprint-test" site create --name "Empty Blueprint" --wp latest --php 8.4 --blueprint "$env:TEMP\empty-blueprint.json" --skip-browserphp_xdebug.dll.Pre-merge Checklist