Add support for two other standard locations for SQLite plugin#3418
Add support for two other standard locations for SQLite plugin#3418wojtekn wants to merge 3 commits intoWordPress:trunkfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds fallback loading for the SQLite “pdo-mysql-on-sqlite” driver from additional standard WordPress plugin locations so Adminer/phpMyAdmin tooling can work when the integration isn’t located under Playground’s internal shared path.
Changes:
- Try multiple known install paths (internal shared,
mu-plugins, andplugins) when loading the SQLite driver. - Apply the same fallback logic to both Adminer extensions and the phpMyAdmin mysqli shim.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/playground/website/src/components/site-manager/site-database-panel/adminer-extensions/adminer-mysql-on-sqlite-driver.php | Adds multi-path fallback search for the SQLite driver include. |
| packages/playground/tools/src/phpmyadmin/DbiMysqli.php | Adds multi-path fallback search for the SQLite driver include used by phpMyAdmin. |
| packages/playground/personal-wp/src/components/site-manager/site-database-panel/adminer-extensions/adminer-mysql-on-sqlite-driver.php | Mirrors the Adminer fallback logic for the personal-wp package. |
Comments suppressed due to low confidence (1)
packages/playground/personal-wp/src/components/site-manager/site-database-panel/adminer-extensions/adminer-mysql-on-sqlite-driver.php:1
- The same driver-location list and loading loop is repeated in 3 files in this PR. To prevent future drift (e.g., adding a 4th location but missing one file), consider extracting this into a small shared helper (function or include) used by all three call sites, or at least centralizing the path list into a shared constant.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| foreach ($sqlite_driver_locations as $sqlite_driver_path) { | ||
| if (file_exists($sqlite_driver_path)) { | ||
| require_once $sqlite_driver_path; | ||
| break; | ||
| } |
There was a problem hiding this comment.
If none of these paths exist and the driver wasn’t already loaded elsewhere, this code silently continues and will likely fail later when WP_SQLite_Driver is referenced (harder to diagnose than the previous unconditional require_once). Consider tracking whether a file was loaded and, if not (and class_exists('WP_SQLite_Driver') is still false), failing fast with a clear error (e.g., throwing an exception or triggering a descriptive fatal error). Also consider skipping the filesystem scan entirely when the class is already loaded.
| foreach ($sqlite_driver_locations as $sqlite_driver_path) { | |
| if (file_exists($sqlite_driver_path)) { | |
| require_once $sqlite_driver_path; | |
| break; | |
| } | |
| // If the driver class is not already loaded, try to load it from the known locations. | |
| if (!\class_exists('\WP_SQLite_Driver', false)) { | |
| $sqlite_driver_loaded = false; | |
| foreach ($sqlite_driver_locations as $sqlite_driver_path) { | |
| if (\file_exists($sqlite_driver_path)) { | |
| \require_once $sqlite_driver_path; | |
| $sqlite_driver_loaded = true; | |
| break; | |
| } | |
| } | |
| // Fail fast with a clear error if the driver class is still not available. | |
| if (!\class_exists('\WP_SQLite_Driver', false)) { | |
| \trigger_error( | |
| 'WP_SQLite_Driver could not be loaded. Checked paths: ' | |
| . \implode(', ', $sqlite_driver_locations), | |
| E_USER_ERROR | |
| ); | |
| } |
| foreach ($sqlite_driver_locations as $sqlite_driver_path) { | ||
| if (file_exists($sqlite_driver_path)) { | ||
| require_once $sqlite_driver_path; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
file_exists() only checks existence; require_once can still emit warnings/fatal if the file is not readable (permissions, sandboxing). Prefer is_readable($sqlite_driver_path) (or file_exists + is_readable) before including to reduce noisy failures and make behavior more predictable across environments.
brandonpayton
left a comment
There was a problem hiding this comment.
I left one comment containing a suggestion that changed the constant name to WP_SQLITE_LOCATION to WP_SQLITE_DRIVER_DIR.
If we're going to support custom locations for the WP SQLite driver, are there any other places in the Playground codebase that would need to be updated to respect this constant?
| // Load the SQLite driver from the first available location. | ||
| // The path varies depending on the environment (Playground, Studio, etc.). | ||
| $sqlite_driver_locations = array( | ||
| '/internal/shared/sqlite-database-integration/wp-pdo-mysql-on-sqlite.php', | ||
| defined( 'WP_SQLITE_LOCATION' ) ? WP_SQLITE_LOCATION . '/wp-pdo-mysql-on-sqlite.php' : false, | ||
| ); | ||
| foreach ($sqlite_driver_locations as $sqlite_driver_path) { | ||
| if ($sqlite_driver_path && file_exists($sqlite_driver_path)) { | ||
| require_once $sqlite_driver_path; | ||
| break; | ||
| } | ||
| } | ||
| if (!class_exists('WP_SQLite_Driver')) { | ||
| die('Error: Could not load the SQLite driver.'); | ||
| } |
There was a problem hiding this comment.
@wojtekn could we distill this to the following?
| // Load the SQLite driver from the first available location. | |
| // The path varies depending on the environment (Playground, Studio, etc.). | |
| $sqlite_driver_locations = array( | |
| '/internal/shared/sqlite-database-integration/wp-pdo-mysql-on-sqlite.php', | |
| defined( 'WP_SQLITE_LOCATION' ) ? WP_SQLITE_LOCATION . '/wp-pdo-mysql-on-sqlite.php' : false, | |
| ); | |
| foreach ($sqlite_driver_locations as $sqlite_driver_path) { | |
| if ($sqlite_driver_path && file_exists($sqlite_driver_path)) { | |
| require_once $sqlite_driver_path; | |
| break; | |
| } | |
| } | |
| if (!class_exists('WP_SQLite_Driver')) { | |
| die('Error: Could not load the SQLite driver.'); | |
| } | |
| $sqlite_driver_location = defined('WP_SQLITE_DRIVER_DIR') | |
| ? WP_SQLITE_DRIVER_DIR | |
| : '/internal/shared/sqlite-database-integration'; | |
| require_once $sqlite_driver_location . '/wp-pdo-mysql-on-sqlite.php'; |
If there is a configured driver location we should probably just use that and fail if nothing exists at that path.
I also suggest we change WP_SQLITE_LOCATION to WP_SQLITE_DRIVER_DIR because:
- WP_SQLITE_LOCATION sounds ambiguous and could refer to an actual DB location or something else.
- "DIR" is more specific than "LOCATION"
|
This one was actually superseded by #3420. Sorry about not closing this earlier, @brandonpayton. |
Motivation for the change, related issues
Currently, database admin tool patches included in Playground assume the internal path for the SQLite plugin. Playground also allows skipping SQLite setup, so user can manage their SQLite and keep it in
plugins/ormu-plugins.Implementation details
I propose adding a fallback for two other paths.
Testing Instructions (or ideally a Blueprint)
Test if phpMyAdmin still loads fine.