Conversation
…nfig.php - Replace isSqliteIntegrationInstalled() check with hasMysqlWpConfig() for determining WordPress install mode. The new helper reads wp-config.php to detect real MySQL credentials (non-placeholder DB_NAME/DB_USER), mirroring Playground's own internal detection logic. - Make skipSqliteSetup dynamic: false for SQLite/new sites (Playground handles setup in-memory), true for MySQL sites (skip SQLite entirely). - getWordPressInstallMode() is now synchronous since the async fs check is gone. This is phase 1 of removing Studio's SQLite plugin management responsibility.
…d CLI Playground CLI ships sqlite-database-integration.zip and handles setup in-memory when skipSqliteSetup is false, so Studio no longer needs to download, bundle, copy, or install the plugin itself. - Remove all keepSqliteIntegrationUpdated() call sites from create, start, export, import, pull, push commands and Studio ipc-handlers/sync - Remove build-time SQLite download from download-wp-server-files.ts - Remove copyBundledSqlite() and its entry from setupServerFiles() - Remove SQLITE_DATABASE_INTEGRATION_RELEASE_URL constant - Delete sqlite-integration provider classes (cli, studio, common) and their tests since nothing references them anymore - Remove getSqlitePluginPath() and getSqlitePath() which are now unused
…g check - Move hasMysqlWpConfig() to tools/common/lib/fs-utils so it can be shared - Replace the old on-disk plugin/db.php/database detection in SiteServer with a simple isSqliteSite() that returns !hasMysqlWpConfig(), consistent with how the CLI child process determines SQLite vs MySQL - Remove the now-unused fsExtra import from site-server.ts
Older Studio versions wrote sqlite-database-integration/ and db.php directly into the site's wp-content directory. Playground CLI now manages these in-memory, so they can be removed on startup to keep site directories clean. - Add cleanupLegacySqliteFiles() to mu-plugins.ts alongside the existing cleanupLegacyMuPlugins() pattern - Call it on every SQLite site start (skipped for MySQL sites where skipSqliteSetup is true)
…-command filter The sqlite-command tool uses the sqlite_command_sqlite_plugin_directories filter to locate the SQLite plugin. Previously Studio put the plugin on disk at wp-content/mu-plugins/sqlite-database-integration, but Playground now manages it in-memory at /internal/shared/sqlite-database-integration. Add that path so database export works for new sites (and old sites after cleanup).
📊 Performance Test ResultsComparing 16b40d9 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) |
…in-memory WP-CLI commands for import/export need a live Playground instance so that the SQLite integration is available. Starting the site before proceeding ensures WP-CLI routes through the daemon rather than a standalone instance.
…d's in-memory VFS Playground CLI mounts the SQLite integration plugin at /internal/shared/sqlite-database-integration in its virtual filesystem rather than writing it to disk. The bundled sqlite-command only checks on-disk paths (wp-content/plugins/ and wp-content/mu-plugins/) and reads the version from db.php, which Playground never writes. This patch is applied to the installed copy in ~/.studio/server-files/ after copyBundledSqliteCommand runs. It adds the /internal/shared/ path to get_plugin_directory() and replaces get_plugin_version() to read SQLITE_DRIVER_VERSION from wp-includes/database/version.php instead. TODO: Remove once https://github.com/wp-cli/sqlite-command has been updated upstream.
041a6cc to
157046e
Compare
…p-to-playground-cli
| } | ||
| } | ||
|
|
||
| async function patchSqliteCommandForStudio() { |
There was a problem hiding this comment.
@JanJakes I started exploring the possibility of removing SQLite plugin management responsibility from Studio in favor of Playground CLI.
I needed to update how the command gets the SQLite plugin path and version. I opened PR (Automattic/wp-cli-sqlite-command#26) with those changes and applied them here on the fly to test them.
Let me know your thoughts.
| exporters: NewExporter[] = defaultExporterOptions | ||
| ): Promise< boolean > { | ||
| // Export commands run WP-CLI which needs a live Playground instance in the daemon | ||
| // so that SQLite is available in-memory. Ensure the site is running before proceeding. |
There was a problem hiding this comment.
@fredrikekelund @bcotrim custom WP CLI SQLite command used during export relies on the SQLite plugin. When we remove the plugin from the Studio site code and move responsibility for providing the plugin to the Playground CLI, it breaks commands that are executed without a full Playground instance.
Any thoughts on a better approach here than starting the site?
CC @JanJakes
There was a problem hiding this comment.
Prior to #2376, we initiated a Playground instance to run WP-CLI commands without an existing running Playground instance. We can use that as a reference for going back to a playground.cli( … ) approach.
However, the reason we moved away from that was that it was more than twice as slow. A simple wp option get operation took ~5s with the old approach, and ~2s with the current approach. Native WP-CLI takes ~0.2s.
I think we need to consider this change carefully from a performance perspective. IMO, we (the engineering team) should tolerate weirdness and/or duplication if the tradeoff is improved performance for end users.
There was a problem hiding this comment.
In this case, would you be up for keeping the SQLite plugin of a Studio site?
@adamziel any thoughts?
There was a problem hiding this comment.
I took another look at the Playground source code, and I think we can keep going with your approach in this PR, @wojtekn, but we need to call preloadSqliteIntegration from @wp-playground/wordpress in apps/cli/lib/run-wp-cli-command.ts.
That function takes a File instance containing the sqlite-database-integration plugin. @wp-playground/wordpress ships that plugin. It seems sensible to read the plugin from node_modules/@wp-playground/wordpress to make sure there are no inconsistencies between running a WP-CLI command when a site is stopped compared to when it's started.
There was a problem hiding this comment.
If we do this ☝️, then we should also remove this code in export-manager.ts that starts a server before doing the export.
There was a problem hiding this comment.
PHP-WASM was crashing when I tried with preloadSqliteIntegration - I tried to repeat those steps in Studio code. It works, but seems to be hacky.
| if ( ! dbNameMatch || ! dbUserMatch ) { | ||
| return false; | ||
| } | ||
| return dbNameMatch[ 1 ] !== 'database_name_here' && dbUserMatch[ 1 ] !== 'username_here'; |
There was a problem hiding this comment.
Is it an acceptable check? Or do we want to handle cases in which user uses MySQL with those defaults?
There was a problem hiding this comment.
Actually, MySQL is not a huge focus for us at this point, so it seems reasonable to rely on this check.
| define('QM_TESTS', true); | ||
| // It's not the best fix, but it's simple and for consistency it's the same as used in wordpress-playground (https://github.com/WordPress/wordpress-playground/pull/2415) | ||
| // Playground's SQLite preload shim also defines QM_TESTS, so guard against redefinition. | ||
| if ( ! defined( 'QM_TESTS' ) ) { |
There was a problem hiding this comment.
This is to cover for the Playground, which also adds this fix.
| // Ensure SQLite command can find the plugin. | ||
| // For new sites, Playground manages the plugin in-memory at /internal/shared/sqlite-database-integration. | ||
| // For existing sites that haven't been cleaned up yet, the plugin is on disk at the legacy path. | ||
| add_filter( 'sqlite_command_sqlite_plugin_directories', function( $directories ) { |
There was a problem hiding this comment.
I'm not really sure why we define this filter in the first place. 🤔
There was a problem hiding this comment.
On the face of it, I would assume this filter does the same thing as patchSqliteCommandForStudio. Is that not the case..?
There was a problem hiding this comment.
I assumed the same, but the filter change didn't help.
get_plugin_directory doesn't execute any filters. Also, I couldn't see anything like {plugin_name}_plugin_directories or similar in WP core.
I see it was added in #845, but there is no explanation there. @bgrgicak do you know where this came from?
There was a problem hiding this comment.
Sorry, I can't remember why I added it. I checked my notes, code, git history, but there are no mentions of it anywhere.
I guess it's safe to remove.
…p-to-playground-cli
When a site is stopped, WP-CLI runs in a standalone PHP-WASM instance without access to the live Playground daemon. Preloading the SQLite integration plugin into the VFS makes sqlite-command work correctly in this path. The upstream preloadSqliteIntegration() from @wp-playground/wordpress uses php.run() internally to extract the zip, which corrupts the PHP WASM runtime state before php.cli() runs. Instead we extract the zip in Node.js using yauzl and write the files directly into the VFS.
1acf94d to
16b40d9
Compare
|
@wojtekn, I came to think of a flaw with this approach: site directories will no longer be portable and users won't be able to use non-Playground tools (like native WP-CLI) to work with the site. I think this is a pretty significant regression from the user perspective. The non-portable question also means that I believe this PR breaks preview site creation. We'd need to inject the |
|
@fredrikekelund good points. This change is not really urgent. Let's keep discussing it, and wait for @adamziel and @JanJakes thoughts. I'm changing it to a draft for now. |
Related issues
How AI was used in this PR
I used Claude for implementation, and debugging.
Proposed Changes
wp-config.phprather than by checking for the SQLite plugin on diskdb.php,mu-plugins/sqlite-database-integration) on site start for existing sites/internal/shared/sqlite-database-integration) to the query monitor mu-plugin path filtersqlite-commandafter copy to support Playground's in-memory VFS: adds/internal/shared/toget_plugin_directory()and replacesget_plugin_version()to read fromversion.phpinstead ofdb.php. To be removed once the fix lands in wp-cli/sqlite-command.Testing Instructions
db.php,mu-plugins/sqlite-database-integration) migrate cleanly on first startwp-config.php) are still detected correctly and SQLite setup is skippedPre-merge Checklist