Make WordPress Core

Opened 3 weeks ago

Closed 2 weeks ago

Last modified 2 hours ago

#60461 closed defect (bug) (fixed)

Running the install script shows database error after plugin dependencies changeset

Reported by: desrosj's profile desrosj Owned by: costdev's profile costdev
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch commit
Focuses: Cc:

Description

Follow up to #22316.

After [57545], there's a new error being shown when running npm run env:install. It doesn't appear in the logs for the changeset prior.

<div id="error"><p class="wpdberror"><strong>WordPress database error:</strong> [Table &#039;wordpress_develop.wp_options&#039; doesn&#039;t exist]<br /><code>INSERT INTO `wp_options` (`option_name`, `option_value`, `autoload`) VALUES (&#039;plugin_data&#039;, &#039;a:1:{s:9:\&quot;hello.php\&quot;;a:15:{s:4:\&quot;Name\&quot;;s:11:\&quot;Hello Dolly\&quot;;s:9:\&quot;PluginURI\&quot;;s:41:\&quot;http://wordpress.org/plugins/hello-dolly/\&quot;;s:7:\&quot;Version\&quot;;s:5:\&quot;1.7.2\&quot;;s:11:\&quot;Description\&quot;;s:295:\&quot;This is not just a plugin, it symbolizes the hope and enthusiasm of an entire generation summed up in two words sung most famously by Louis Armstrong: Hello, Dolly. When activated you will randomly see a lyric from &lt;cite&gt;Hello, Dolly&lt;/cite&gt; in the upper right of your admin screen on every page.\&quot;;s:6:\&quot;Author\&quot;;s:14:\&quot;Matt Mullenweg\&quot;;s:9:\&quot;AuthorURI\&quot;;s:13:\&quot;http://ma.tt/\&quot;;s:10:\&quot;TextDomain\&quot;;s:0:\&quot;\&quot;;s:10:\&quot;DomainPath\&quot;;s:0:\&quot;\&quot;;s:7:\&quot;Network\&quot;;b:0;s:10:\&quot;RequiresWP\&quot;;s:0:\&quot;\&quot;;s:11:\&quot;RequiresPHP\&quot;;s:0:\&quot;\&quot;;s:9:\&quot;UpdateURI\&quot;;s:0:\&quot;\&quot;;s:15:\&quot;RequiresPlugins\&quot;;s:0:\&quot;\&quot;;s:5:\&quot;Title\&quot;;s:11:\&quot;Hello Dolly\&quot;;s:10:\&quot;AuthorName\&quot;;s:14:\&quot;Matt Mullenweg\&quot;;}}&#039;, &#039;yes&#039;) ON DUPLICATE KEY UPDATE `option_name` = VALUES(`option_name`), `option_value` = VALUES(`option_value`), `autoload` = VALUES(`autoload`)</code></p></div>Success: WordPress installed successfully.

Despite this warning, it seems like all the tests pass successfully. There are more tests and assertions performed, but wondering if some are being skipped as a result.

Change History (18)

#1 @swissspidy
3 weeks ago

We noticed this in WP-CLI tests as well and @costdev mentioned he is working on this in #60457 already. So I suggest closing this as a duplicate.

See https://wordpress.slack.com/archives/C02RP4T41/p1707298003074869

Running some of the WP-CLI tests in the core test suite would help uncover this kind of issues sooner. Let's discuss this!

#2 @swissspidy
3 weeks ago

Or actually, Colin suggests keeping this one open for visibility, which makes sense.

This ticket was mentioned in PR #6071 on WordPress/wordpress-develop by @costdev.


3 weeks ago
#3

  • Keywords has-patch added

This PR:

  • removes two update_option() calls during bootstrapping.
  • only updates the plugin_data option in get_plugins() when not installing WordPress.

#4 @costdev
3 weeks ago

  • Keywords needs-testing added

@desrosj @swissspidy PR 6071 removes some update_option() calls during bootstrapping and guards an update_option() call in get_plugins() during installation.

Please test if you can 🙂

#5 @huzaifaalmesbah
2 weeks ago

I tested your patch. after applying your patch everything is fine, no error now.

https://i.ibb.co/tZrcVHc/Screenshot-2024-02-10-at-12-58-43-AM.png

#6 @costdev
2 weeks ago

  • Owner set to costdev
  • Status changed from new to assigned

#7 @afragen
2 weeks ago

Test Report

Description

This report validates whether the indicated patch works as expected.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/6071

Environment

  • WordPress: 6.5-alpha-56966-src
  • PHP: 8.2.15
  • Server: nginx/1.25.3
  • Database: mysqli (Server: 8.0.36 / Client: mysqlnd 8.2.15)
  • Browser: Safari 17.2.1
  • OS: macOS
  • Theme: Twenty Twenty-Four 1.0
  • MU Plugins:
    • WP-CLI: Force Auto-Updates 1.0.0
  • Plugins:
    • Test Reports 1.1.0

Actual Results

  1. ✅ Issue resolved with patch.

Additional Notes

  • Any additional details worth mentioning.

Supplemental Artifacts

Success: Database reset.
[+] Creating 2/0
 ✔ Container public-mysql-1  Running                                                                                              0.0s 
 ✔ Container public-php-1    Running                                                                                              0.0s 
<div id="error"><p class="wpdberror"><strong>WordPress database error:</strong> [Table &#039;wordpress_develop.wp_options&#039; doesn&#039;t exist]<br /><code>SHOW FULL COLUMNS FROM `wp_options`</code></p></div>Success: WordPress installed successfully.
➜  AJF-M1-MBA arm64: ~/Local_Sites/wpdevelop/app/public git:(trunk) ✗ npm run env:install

Success: Database reset.
[+] Creating 2/0
 ✔ Container public-php-1    Running                                                                                              0.0s 
 ✔ Container public-mysql-1  Running                                                                                              0.0s 
Success: WordPress installed successfully.
➜  AJF-M1-MBA arm64: ~/Local_Sites/wpdevelop/app/public git:(plugin_dependencies_remove_plugin_data_update_in_bootstrap) ✗

#8 @costdev
2 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 57592:

Upgrade/Install: Avoid update_option() calls during bootstrap.

[57545] introduced the Plugin Dependencies feature, which contains a new plugin_data option.

Previously, the plugin_data option was being updated during bootstrap and in get_plugins(), causing an error when using the install script as the options database table does not yet exist, and also risked an "out of sync" issue between the database and the cache on websites with heavy traffic.

This removes the calls to update_option() during Core's bootstrap, and guards the call in get_plugins() to ensure that it doesn't run when WordPress is installing.

Follow-up to [57545].

Props desrosj, swisspidy, huzaifaalmesbah, afragen, dd32, azaozz, costdev.
Fixes #60461. See #60457, #60491.

@youknowriad commented on PR #6071:


2 weeks ago
#9

Unfortunately, according to codevitals, this was not enough to recover the performance loss. There's probably something else at play.

@costdev commented on PR #6071:


2 weeks ago
#10

@youknowriad Thanks for looking into this! It's likely that we'll need to strip back/ remove and relocate a lot of the bootstrapping logic, not unlike a problematic callback hooked on init. We'll also need to address automatic deactivation - something I'm discussing with the feature team for thoughts and ideas

@costdev commented on PR #6071:


2 weeks ago
#11

I forgot to close the PR after this was committed. Let's continue discussion on the related tickets.

Committed in r57592.

This ticket was mentioned in Slack in #cli by costdev. View the logs.


2 weeks ago

#13 @costdev
2 weeks ago

  • Keywords needs-patch added; has-patch needs-testing removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening to address something missed in the commit:

If a plugin isn't in the plugin_data option, it will throw an error for a missing array key.

Originally reported by @swissspidy on Slack.

This ticket was mentioned in PR #6103 on WordPress/wordpress-develop by @costdev.


2 weeks ago
#14

  • Keywords has-patch added; needs-patch removed

In r57592, some update_option() calls were removed from bootstrapping. However, this also removed a check to ensure an array key existed, and populated it if not.

Scaffolding tests by WP-CLI revealed that a plugin in the active_plugins option may not have data already stored within the plugin_data option, causing a PHP warning for an undefined array key.

This adds a condition to ensure the requirements checks are only performed on plugins whose data is already stored in the plugin_data option.

#16 @costdev
2 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 57622:

Upgrade/Install: Only check plugins whose data is already stored.

In [57592], some update_option() calls were removed from bootstrapping. However, this also removed a check to ensure an array key existed, and populated it if not.

Scaffolding tests by WP-CLI revealed that a plugin in the active_plugins option may not have data already stored within the plugin_data option, causing a PHP warning for an undefined array key. This data will be added the next time get_plugins() is called.

This adds a condition to ensure the requirements checks are only performed on plugins whose data is already stored in the plugin_data option.

Follow-up to [57592].

Props swissspidy, hellofromTonya, costdev.
Fixes #60461.

@costdev commented on PR #6103:


2 weeks ago
#17

Committed in 57622.

This ticket was mentioned in Slack in #core-upgrade-install by costdev. View the logs.


12 days ago

Note: See TracTickets for help on using tickets.