-
Notifications
You must be signed in to change notification settings - Fork 73
Native PHP worker pool on all platforms #3602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
e9db4d4
Add native PHP worker pool POC
bcotrim 630d1db
Track the request queue per worker
fredrikekelund 98bd09b
Potential fix for pull request finding 'CodeQL / Information exposure…
bcotrim ce185f3
Merge branch 'trunk' into add-native-php-worker-pool-poc
fredrikekelund daa7d8a
Always spawn the php pool the "poor man's" way
fredrikekelund 96ed96a
Merge branch 'trunk' into add-native-php-worker-pool-poc
fredrikekelund 8258fbd
Merge branch 'trunk' into add-native-php-worker-pool-poc
fredrikekelund 7700985
Improve grandchild process cleanup on Windows
fredrikekelund fd3df18
Unit test
fredrikekelund f04bfdd
Merge branch 'trunk' into add-native-php-worker-pool-poc
fredrikekelund ea71db1
Refactor native PHP child helpers
bcotrim a2fc287
Merge remote PR branch
bcotrim 9cc6be7
Improved E2E cleanup retry behavior
fredrikekelund 71c6125
Improved native PHP child process cleanup
fredrikekelund 4b06c8b
Reap wp, import, export CLI command children
fredrikekelund f54e74f
Recursively kill php children in happy path
fredrikekelund 23f47bd
Kill grandchildren in stopPhpChild
fredrikekelund 45e903b
Move files around
fredrikekelund b4f5cd1
Consider that OPcache is included in binary in PHP >=8.5
fredrikekelund 7c0e893
Tweaking
fredrikekelund 625fb8b
Fix Windows e2e cleanup retry for native PHP
bcotrim 6a9bf7f
Pin e2e locale to English
bcotrim 40b2bb5
Collect all process logs when E2E tests fail
fredrikekelund 797dc28
Stabilize Windows native PHP e2e cleanup
bcotrim cd64c4a
Merge remote-tracking branch 'origin/add-native-php-worker-pool-poc' …
bcotrim d9bbd0c
Merge remote-tracking branch 'origin/trunk' into add-native-php-worke…
bcotrim c829764
Try a simplified rimraf config
fredrikekelund File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| import fs from 'node:fs'; | ||
| import path from 'node:path'; | ||
| import { getBlueprintsPharPath } from 'cli/lib/dependency-management/paths'; | ||
| import { runPhpCommand } from './php-process'; | ||
| import type { NativePhpSupportedVersion } from '@studio/common/lib/php-binary-metadata'; | ||
| import type { ServerConfig } from 'cli/lib/types/wordpress-server-ipc'; | ||
|
|
||
| export async function runBlueprint( | ||
| config: ServerConfig, | ||
| blueprint: NonNullable< ServerConfig[ 'blueprint' ] >, | ||
| phpVersion: NativePhpSupportedVersion, | ||
| signal: AbortSignal | ||
| ): Promise< void > { | ||
| // blueprints.phar accepts local paths only; remote URIs need the Playground runtime. | ||
| if ( blueprint.uri.startsWith( 'http://' ) || blueprint.uri.startsWith( 'https://' ) ) { | ||
| throw new Error( | ||
| `Remote blueprint URIs are not supported by the native PHP runtime: ${ blueprint.uri }` | ||
| ); | ||
| } | ||
|
|
||
| const enableDebugLog = config.enableDebugLog ?? false; | ||
| const enableDebugDisplay = config.enableDebugDisplay ?? false; | ||
| const defaultConstants: Record< string, boolean | string > = { | ||
| // The SQLite driver requires a non-empty DB_NAME at runtime. | ||
| DB_NAME: 'wordpress', | ||
| WP_DEBUG: enableDebugLog || enableDebugDisplay, | ||
| WP_DEBUG_LOG: enableDebugLog, | ||
| WP_DEBUG_DISPLAY: enableDebugDisplay, | ||
| }; | ||
|
|
||
| blueprint.contents.constants = { | ||
| ...blueprint.contents.constants, | ||
| ...defaultConstants, | ||
| }; | ||
| // Native PHP selects PHP and installs WordPress before Blueprint execution. | ||
| // Passing preferredVersions makes blueprints.phar validate versions it does not manage here. | ||
| delete blueprint.contents.preferredVersions; | ||
|
|
||
| const blueprintDir = path.dirname( blueprint.uri ); | ||
| const tmpPath = path.join( blueprintDir, `studio-blueprint-${ config.siteId }.json` ); | ||
| await fs.promises.writeFile( tmpPath, JSON.stringify( blueprint.contents ) ); | ||
|
|
||
| // blueprints.phar detects SQLite under plugins, while Studio installs it under mu-plugins. | ||
| const muPluginsSqlite = path.join( | ||
| config.sitePath, | ||
| 'wp-content', | ||
| 'mu-plugins', | ||
| 'sqlite-database-integration' | ||
| ); | ||
| const pluginsSqlite = path.join( | ||
| config.sitePath, | ||
| 'wp-content', | ||
| 'plugins', | ||
| 'sqlite-database-integration' | ||
| ); | ||
| const needsSymlink = fs.existsSync( muPluginsSqlite ) && ! fs.existsSync( pluginsSqlite ); | ||
| let symlinkIno: number | undefined; | ||
| if ( needsSymlink ) { | ||
| fs.symlinkSync( muPluginsSqlite, pluginsSqlite, 'junction' ); | ||
| // Remove only the entry created here, not unrelated content that replaced it. | ||
| symlinkIno = fs.statSync( pluginsSqlite ).ino; | ||
| } | ||
|
|
||
| try { | ||
| await runPhpCommand( | ||
| [ | ||
| getBlueprintsPharPath(), | ||
| 'exec', | ||
| tmpPath, | ||
| '--mode=apply-to-existing-site', | ||
| `--site-path=${ config.sitePath }`, | ||
| `--site-url=${ config.absoluteUrl ?? `http://localhost:${ config.port }` }`, | ||
| '--db-engine=sqlite', | ||
| ], | ||
| { phpVersion, signal } | ||
| ); | ||
| } finally { | ||
| await fs.promises.unlink( tmpPath ).catch( () => {} ); | ||
| if ( needsSymlink ) { | ||
| try { | ||
| if ( fs.statSync( pluginsSqlite ).ino === symlinkIno ) { | ||
| await fs.promises.rm( pluginsSqlite, { recursive: true, force: true } ); | ||
| } | ||
| } catch { | ||
| // Best effort - leaving the symlink behind is non-fatal. | ||
| } | ||
| } | ||
| } | ||
| } |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The changes in this file fix RSM-3965 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in this file and in
.buildkite/pipeline.ymlserve to collect all site logs when an E2E test fails. Site logs are typically forwarded to the main process anyway, but this seems like a sensible thing to keep as the default. It will only make debugging easier