Place Hello Dolly in containing folder

[afragen]

Currently Hello Dolly is installed as a single file plugin during a WP core installation. According to Plugin Handbook Best Practices, plugins should be in containing folders. ​https://developer.wordpress.org/plugins/plugin-basics/best-practices/#folder-structure

This is a simple PR to fix this issue with Hello Dolly. Having this means that things like r51064 are not necessary.

Related #49338

Currently Hello Dolly is installed as a single file plugin during a WP core installation. According to Plugin Handbook Best Practices, plugins should be in containing folders. ​https://developer.wordpress.org/plugins/plugin-basics/best-practices/#folder-structure

This is a simple PR to fix this issue with Hello Dolly. Having this means that things like r51064 are not necessary.

Related #49338

Trac ticket: https://core.trac.wordpress.org/ticket/53323

[afragen]

Looks like it needs an update to some of the unit tests.

[afragen]

Tests updated

[SergeyBiryukov]

Perhaps we should also add hello.php to the $_old_files array in wp-admin/includes/update-core.php, so that those who upgrade from previous WordPress versions don't get two copies of the plugin.

[afragen]

I can do that tomorrow.

[afragen]

@SergeyBiryukov all recommendations have been added to PR, tests pass 😉

[afragen]

This may actually require the version of Hello Dolly in the plugin repository to be updated to add the header

Text Domain: hello-dolly

[afragen]

In testing, if there is a plugin at hello-dolly/hello.php, updating core, using WP Beta Tester, does not install a new version of hello.php. This should mean that if a user has the older hello.php installed, an update will not over-write the plugin.

Might need more testing.

[justinahinon]

Thank you for explaining how to test @afragen.

During the test team triage on June 16th, we came through others tests scenarios that might be worth being explored:

• On an existing installation, updating WordPress Core (with the patch applied) should leave Hello Dolly as a single file
• On fresh WordPress installation (with the patch applied), Hello Dolly should be in the hello-doly folder

These are the scenarios we are sure about based on the explanations on the ticket.

Now, those that still need to be clarified:

• What happen when updating the Hello Dolly that's a single file? Should it be left in a single file or moved to the directory?
• If it is already activated on an existing install, what happens when it's moved? Does it stay activated? Can it cause fatal error then?

Cc @hellofromtonya @boniu91 @pbearne.

[SergeyBiryukov]

Replying to afragen:

This may actually require the version of Hello Dolly in the plugin repository to be updated to add the header

Text Domain: hello-dolly

Just noting that Hello Dolly's headers are historically also included in core language files, which will probably be redundant if we move forward with adding the Text Domain header to the plugin.

So if Hello Dolly's translations will be loaded separately as for any other plugin, we might need to remove them from core language files. I don't remember off the top of my head where the code that generates them is located, this will need more investigation.

[desrosj]

I'm going to punt this to 5.9 as there are unfortunately too many other tickets that must be addressed in 5.8 and time is running out.

[hellofromTonya]

There are unanswered questions about testing scenarios. Marking for reporter-feedback. It appears to need more discussion and consensus.

Given that 5.9 Feature Freeze is Nov 9 (next week) and this ticket has no activity in the 5.9 cycle, moving it to a Future Release. However, when there is consensus, please move it into a milestone.

[afragen]

Replying to justinahinon:

Sorry this has taken me so long to respond

Thank you for explaining how to test @afragen.

During the test team triage on June 16th, we came through others tests scenarios that might be worth being explored:

• On an existing installation, updating WordPress Core (with the patch applied) should leave Hello Dolly as a single file

The above is not the current behavior. Currently the single file is replaced with the hello-dolly folder.

• On fresh WordPress installation (with the patch applied), Hello Dolly should be in the hello-doly folder

Yes.

These are the scenarios we are sure about based on the explanations on the ticket.

Now, those that still need to be clarified:

• What happen when updating the Hello Dolly that's a single file? Should it be left in a single file or moved to the directory?

An update to a single file Hello Dolly is currently replaced with a version in a containing folder. This does not change.

• If it is already activated on an existing install, what happens when it's moved? Does it stay activated? Can it cause fatal error then?

Currently an update from a single file Hello Dolly to a containing folder version will update the plugin and deactivate it. This is standard WP behavior. No fatal occurs.

Cc @hellofromtonya @boniu91 @pbearne.

[afragen]

PR updated. Applies cleanly. Passing tests. Single failure for unable to checkout WordPress/wordpress-develop.

[afragen]

PR has been updated. All tests passing.

[matt]

I'm fine with this, thank you!

This will require an upgrade routine to check if hello.php is in the active list of plugins and replace it with hello-dolly/hello.php. During testing it deactivated the plugin when I checked out this branch.

For the last few years, Akismet has been ​automatically updated as part of the build process. This allows for a single source of truth and makes code changes to the plugin easier.

I think the approach may be helpful for Hello Dolly once it's moved in to a sub-folder as it will allow for a single source of truth for the code ratehr than having to update both the plugin and the copy in this repo. It would require a systems request but given there is prior art it should be simple enough to introduce.

Does it really matter if the single file hello.php plugin is disabled? If there is ever an update it would also disable.

Since the plugin has no significant effect on the running of the site, I think a seamless transition is overkill.

@peterwilsoncc maybe I'm not understanding. If you check in the Playground a new WP installation will have Hello Dolly in a containing folder.

Given it's possible to have a seamless transition with a few lines of code, I think it's worth the effort.

$active_plugins = get_option( 'active_plugins', array() );
$hello_dolly = array_search( 'hello.php', $active_plugins );
if ( false !== $hello_dolly ) {
        $active_plugins[ $hello_dolly ] = 'hello-dolly/hello.php';
        update_option( 'active_plugins', $active_plugins );
}

I'm not sure what you mean by your second question, are you able to clarify?

[davidbaumwald]

Re: translations: I checked with Dion about Sergey's comments in 18 above, and he mentioned that the code itself is in a private repo, but it calls a public CLI command at ​https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/plugins/wporg-gp-customizations/inc/cli/class-make-core-pot.php?marks=160-168#L133

If you have hello.php installed and there's an update, it will install hello-dolly/hello.php. The original plugin would be deleted and the update will be inactive.

The original plugin would be deleted and the update will be inactive.

@afragen I'm not seeing that, I modified the version number of hello.php and the plugin remained active once it was moved to hello-dolly/hello.php despite the original file being deleted. Maybe there is some snowflake code in core to handle this.

Even though it's a single file plugin, I think WP should handle moving the files as gracefully as it does when other files are relocated. The plugin is active on 700,000+ sites so even when the percentage of sites affected is small, the raw number is large.

I yield. Simply giving a counter argument 😉

Always happy to hear counter arguments. I can be incredibly wrong sometimes.

I'll check with meta (@dd32, I mean @dd32) about using an external to see if we can get a single source of truth for the code.

[peterwilsoncc]

To summarize the discussion on the PR:

• there will need to be some code to prevent the relocation of the plugin from causing it to be deactivated. The plugin has 700K+ active installs.
• the upgrade routine runs too late to modify the active plugins list, by the time it runs the plugin will have been deactivated and the opportunity missed
• to avoid the need to maintain the plugin in two locations, the hello-dolly folder can be added as an external and added during the build process similar to Akismet. I've started ​discussions with the meta team about the process here

If needs be it might be possible to add some snowflake code to wp_get_active_and_valid_plugins() to account for the relocation but it would be optimal if that could be avoided.

There is some snowflake code (either in WP or the upgrade API) to keep the plugin active when updated via the dashboard so there might be some prior art that can used.

[davidbaumwald]

There's still some open #meta questions and considerations prior to completing this in Core.
6.8Future Release given a lack of recent momentum towards a resolution.

If any committer feels the remaining work can be resolved in time for Beta 1 and wishes to assume ownership, feel free to update the milestone accordingly.

[audrasjb]

As per today's bugscrub: since the Meta issues are very unlikely to be resolved in time for 6.8, let's move this issue to Future Release.

[SirLouen]

@afragen currently, this ​PR 1330 is not applying cleanly in 6.9-alpha-60093-src. Could you give it a check for further testing?

[afragen]

Will do.

Clean merge and passing tests.

[afragen]

@SirLouen I think I've gotten everything refreshed. It clean merge and passing tests.

[SirLouen]

Core Updating Helper

[SirLouen]

Test Report

Description

❌ This report can't validate that the indicated patch works as expected.

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/1330.diff

Environment

• WordPress: 6.9-alpha-60093-src
• PHP: 8.2.28
• Server: nginx/1.27.5
• Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
• Browser: Chrome 137.0.0.0
• OS: Windows 10/11
• Theme: Twenty Twenty-Five 1.2
• MU Plugins: None activated
• Plugins:
  • Core Updater Mod 1.0.0
  • Test Reports 1.2.0

Testing Protocol

I'm going to explain step by step how I have tested this patch. It's a little tricky, so I will provide the maximum files and instructions to help in the selection of the best method:

1. First, we need the WordPress update that will come with the patch already deployed. Two methods

• The simplest: Download the update from here: ​https://f003.backblazeb2.com/file/wordpress-videos/wp-videos/2025/05/wp-update.zip
• The slightly more difficult: create a build with npm run build, and add it to a zip file called wp-update.zip and the common of a WordPress update (wordpress/<all_files>). Before running the build, code must already have the patch we are testing here, obviously.

2. Second, we need to edit a little the standard core update protocol, so it can accept our unofficial file. Apply this patch I have attached

3. We need to add some code to use the added filters in a plugin, functions.php or wherever it can be executed. I attach the code in the Supplemental Artifacts: here we specify which folder are we using to put our wp-update.zip file. You can set whatever path you prefer, just make sure it has full permissions from the web server.

4. Now we add the wp-update.zip file to the folder we have set in our previous step

5. We go to plugins and enable Hello Dolly

6. We go to Dashboard > Updates and execute the core update (in wordpress-develop there is always an update available; otherwise, we will need to go to wp-include/version.php and edit the $wp_version to have a lesser version to trigger the update.

7. If everything has been done correctly, after the update you will see something like:

Expected Results

• After the update, we expect that Hello Dolly will be in the new directory format, and it's still enabled.

Actual Results

1. ❌ Hello Dolly disappears after the update

Additional Notes

• @afragen it seems that the patch is not working, I have not reviewed the code because I was expecting that you already did in the past with @peterwilsoncc, and it was ready to be shipped. Please review my steps to check if I'm doing something wrongly, or check the code to see if something has broken in the meantime.

Supplemental Artifacts

add_filter( 'custom_update_package', 'use_local_update_zip', 10, 3 );
function use_local_update_zip( $package_url, $to_download, $current ) {
    $local_zip_path = ABSPATH . '../update/wp-update.zip';

    if ( file_exists( $local_zip_path ) ) {
        return $local_zip_path;
    }
    return $package_url;
}

add_filter( 'check_signature_for_package', 'skip_signature_for_local_zip', 10, 4 );
function skip_signature_for_local_zip( $check_signatures, $package_url, $to_download, $current ) {
    return false;
}

add_filter( 'pre_check_md5_for_custom_package', 'skip_md5_check_for_local_zip', 10, 3 );
function skip_md5_check_for_local_zip( $perform_check, $wp_version, $locale ) {
    return false;
}

[afragen]

The nightly doesn't install Hello Dolly or Akismet. Only a beta/RC/release will install it.

It's unlikely that Hello Dolly will remain active due to what happens in core when the plugin file is moved.

[afragen]

If you test using ​https://playground.wordpress.net/wordpress.html?pr=1330 you will see that Hello Dolly is installed correctly in the containing folder.

[SirLouen]

Replying to afragen:

If you test using ​https://playground.wordpress.net/wordpress.html?pr=1330 you will see that Hello Dolly is installed correctly in the containing folder.

I'm not convinced that this is the right test. AFAIK, calling playground with a PR, just patches the current version of PG with the files, hence it will appear logically there.

The nightly doesn't install Hello Dolly or Akismet. Only a beta/RC/release will install it.

I've added a new wp-update.zip (​same one as in the testing info), I have faked it to version 6.8.2, so it's a final release technically now.

But still the plugin doesnt appear.

[SirLouen]

@afragen another method I have recently discovered to update a custom build, is by using wp core update wp-update.zip with wp-cli. I've been breaking my head with the wall to achieve this kind of core updating for ages.

But still the latest patch is not working as in last report.

I have to start using CLI more, it's definitely packed with surprises

[afragen]

@SirLouen just out of curiosity do you think the changes in wp-admin/includes/update.core.php are causing this test failure?

[SirLouen]

Replying to afragen:

@SirLouen just out of curiosity do you think the changes in wp-admin/includes/update.core.php are causing this test failure?

I have not reviewed the code, just set the testing protocol, but I'm not really sure why the plugin is completely disappearing (akismet is there). Maybe we should take a look to how things are being currently done with Akismet to do something similar?

By the way, here my new steps with wordpress-develop (also a reminder for me if I have to retest, because I'm testing so many reports lately, that now I'm starting to mix memories)

1. Get the patch and modify the version of WordPress to 6.8.2 in version.php
2. npm run build
3. mv build/ wordpress/
4. zip -r wp-update.zip wordpress/
5. rm -r wordpress/
6. Clean all changes to trunk (git restore .)
7. Go into WP and enable Hello Dolly plugin
8. npm run env:cli core update wp-update.zip
9. rm wp-update.zip
10. Check results (Hello Dolly disappearing)

[afragen]

The code referenced above specifically removes hello.php.

[SirLouen]

Replying to afragen:

The code referenced above specifically removes hello.php.

I think I've found why it's not bringing the plugin back:

​In this line you should be adding

'plugins/hello-dolly/'      => '6.9',

But still there is one problem to solve: if hello.php was active before update, it should remind active, but with the current pach is not.

This is something to be updated in the database during the upgrade.

For example in ​this line

// Switch Hello Dolly from file to directory format. See #53323
_upgrade_690_update_active_hello_dolly_option();

And then by the end of the file:

function _upgrade_690_update_active_hello_dolly_option() {
    $active_plugins = get_option('active_plugins');
    $old_plugin = 'hello.php';
    $new_plugin = 'hello-dolly/hello.php';
        $key = array_search($old_plugin, $active_plugins);
    
    if ( $key ) {
        $active_plugins[$key] = $new_plugin;
        update_option('active_plugins', $active_plugins);
    }
}

[afragen]

@SirLouen great sleuthing. I didn’t see the array to add the directory. I’ll update the PR.

My suggestion is to create another PR to keep Hello Dolly active. Just my opinion but if deactivated I doubt the site owner will notice. After all, it’s only present to show the “hope and enthusiasm of a generation”.

[afragen]

@SirLouen PR updated

[SirLouen]

Replying to afragen:

@SirLouen great sleuthing. I didn’t see the array to add the directory. I’ll update the PR.

My suggestion is to create another PR to keep Hello Dolly active. Just my opinion but if deactivated I doubt the site owner will notice. After all, it’s only present to show the “hope and enthusiasm of a generation”.

This is something that was commented by @peterwilsoncc back in the day. It has so many active installs (still 700,000+ nowadays)

More info about my research: I've tested and its not working with the code I provided. This seems that is going to be tricky as hell. Let's see how can I stepdebug a core update run with wp-cli…

Maybe we could open another PR for this, but both should go in at the same time; otherwise the second will be completely useless since it's so specific to this topic. I agree that is nothing massive to keep it up after update (and it feels its going to be a hassle, maybe @peterwilsoncc could give some ideas on how to make this happen based on my last comment)

Also very important don't forget to add the rules in .gitignore for this patch, to avoid deletion of hello for the repository (it's like the default plugin and should be tracked).

[afragen]

Hmm, it's never been in the .gitignore before, only the whole plugins directory.

[SirLouen]

Replying to afragen:

Hmm, it's never been in the .gitignore before, only the whole plugins directory.

That screen capture is what needs to be modified, full plugins ignored, only new hello-dolly directory not ignored (because now its falling into the plugins directory, hence, its going to be ignored by default)

[afragen]

Done.

[SirLouen]

I decided to step debug using my previous upgrade technique, because I've been unable to run Xdebug with WP-Cli 💀

Finally, I found the problem of why my solution despite the code was right (I tested the code independently), wasn't applying during the core upgrade.

It appears that placing this code in that file proposed will only apply when updating via the Update button in /wp-admin/update-core.php but not via wp-cli core update because it happens that CLI is taking another route for upgrading (and maybe automated upgrades also take another route, who knows 🤷‍♂️)

I'm not an expert in Core Upgrades area but from what I've been reviewing a possible solution that is working for me is adding the code in src/wp-admin/includes/upgrade.php (attached patch here). Now it works with CLI, with my hacked core upgrading system, and probably, with automated updates also, because it seems that all upgrading skins use this upgrade.php file to do their diligence.

And with this finally accomplished, I would suggest that this patch is ready to go.

[oglekler]

Related tickets were discussed during the Test team triage session.

We have several tickets about Hello Dolly, and from my point of view they have valid points and can be incorporated into one patch, so I am linking them for further discussion.
#63621, #61215, #61069, #58363

[afragen]

Replying to oglekler:

Related tickets were discussed during the Test team triage session.

We have several tickets about Hello Dolly, and from my point of view they have valid points and can be incorporated into one patch, so I am linking them for further discussion.
#63621, #61215, #61069, #58363

I disagree. This ticket is not about fixing or changing the content of the Hello Dolly plugin or its relationship to the PCP. It's simply about making the plugin conform to the standard of having a containing folder.

[SirLouen]

Replying to afragen:

I disagree. This ticket is not about fixing or changing the content of the Hello Dolly plugin or it's relationship to the PCP.

By the way, shouldn't we milestone it to 6.9?

[whyisjake]

In 60666:

Upgrade/Install: Move Hello Dolly plugin to directory structure.

Changes the Hello Dolly plugin from a single file structure to a proper plugin directory structure, moving from hello.php to hello-dolly/hello.php to align with Plugin Handbook Best Practices.

• Adds proper Text Domain: hello-dolly header to Hello Dolly plugin
• Updates core files to remove special case handling for hello.php
• Updates plugin dependency system to handle new directory structure
• Adds upgrade routine to migrate active plugin references and keep plugin active
• Updates all tests to use new plugin path format hello-dolly/hello.php
• Updates build configuration and .gitignore for new directory structure
• Adds hello.php to old files list for cleanup during core updates
• Adds plugins/hello-dolly/ to new bundled directories list

Props afragen, SergeyBiryukov, peterwilsoncc, SirLouen, matt, davidbaumwald, desrosj, hellofromtonya, justinahinon,audrasjb, oglekler, whyisjake.
Fixes #53323.

[whyisjake]

In 60670:

Upgrade/Install: Actually move Hello Dolly plugin to directory structure.

This fixes an issue from [60666] where I neglected to svn move the files.

Follow-up to [60666].

Reviewed by jeremyfelt.

Props swissspidy.

See #53323.

[johnbillion]

There are still test errors and failures in trunk. Example failing workflow: ​https://github.com/WordPress/wordpress-develop/actions/runs/17250411316/job/48952910236.

There were 2 errors:

1) Tests_Admin_IncludesPlugin::test_get_plugin_data
file_get_contents(/var/www/tests/phpunit/includes/../data/plugins/hello-dolly/hello.php): failed to open stream: No such file or directory

/var/www/src/wp-includes/functions.php:6901
/var/www/src/wp-admin/includes/plugin.php:94
/var/www/tests/phpunit/tests/admin/includesPlugin.php:25
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

2) Tests_Admin_IncludesPlugin::test_is_network_only_plugin_hello
file_get_contents(/var/www/tests/phpunit/data/plugins/hello-dolly/hello.php): failed to open stream: No such file or directory

/var/www/src/wp-includes/functions.php:6901
/var/www/src/wp-admin/includes/plugin.php:94
/var/www/src/wp-admin/includes/plugin.php:604
/var/www/tests/phpunit/tests/admin/includesPlugin.php:553
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

--

There were 4 failures:

1) Tests_Admin_IncludesPlugin::test_is_plugin_active_true
Failed asserting that false is true.

/var/www/tests/phpunit/tests/admin/includesPlugin.php:379
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

2) Tests_Admin_IncludesPlugin::test_is_plugin_inactive_false
Failed asserting that true is false.

/var/www/tests/phpunit/tests/admin/includesPlugin.php:399
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

3) Tests_Admin_IncludesPlugin::test_activate_plugins_single_no_array
Failed asserting that false is true.

/var/www/tests/phpunit/tests/admin/includesPlugin.php:575
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

4) Tests_Admin_IncludesPlugin::test_activate_plugins_single_array
Failed asserting that false is true.

/var/www/tests/phpunit/tests/admin/includesPlugin.php:585
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

[whyisjake]

[60675] fixed the svn:ignore props and [60677] fixed the broken tests.

[whyisjake]

After chatting with @peterwilsoncc and @afragen, I think we need to be a little more explicit in the fallback handling here. In 53323.diff​ I added a filter to the wp_get_active_and_valid_plugins() function to ensure that we get the correct version if active.

Tests attached.

[SirLouen]

Replying to whyisjake:

After chatting with @peterwilsoncc and @afragen, I think we need to be a little more explicit in the fallback handling here. In 53323.diff​ I added a filter to the wp_get_active_and_valid_plugins() function to ensure that we get the correct version if active.

I have mixed feelings with this update specially because its a single-use hook that is going to added to the hook stack forever.

Also, @peterwilsoncc can you review the tests? I feel that some setups or tear-downs could be useful here, what do you think?

[johnbillion]

• I would rather we didn't introduce logic that runs on every request to handle the Hello Dolly migration.
• What's the purpose of the wp_should_migrate_hello_dolly filter?
• Can we get PRs instead of patches too please? So the test suite can run.

Adds some explicit migration handling for the hello dolly plugin.

[mindctrl]

I agree with @johnbillion about this not running on every request. Could this be handled in an upgrade routine instead (wp-admin/includes/upgrade.php)?

[afragen]

Is there a way to run a query to determine how many sites are running the single file plugin vs the version in a containing folder?

That data would be interesting.

[peterwilsoncc]

Replying to johnbillion:

• I would rather we didn't introduce logic that runs on every request to handle the Hello Dolly migration.
• What's the purpose of the wp_should_migrate_hello_dolly filter?
• Can we get PRs instead of patches too please? So the test suite can run.

Could it be handled in this ​block of code so it's only caught if the file does not exist?

Alternatively, if plugins that have been deactivated due to a missing file are stored in an option somewhere, the upgrade routine could check for that and enable the relocated plugin that way.

[whyisjake]

Replying to peterwilsoncc:

Could it be handled in this ​block of code so it's only caught if the file does not exist?

That's essentially what's ​happening here. Check to see if we need to update, and then quickly exit if we don't. Also provides a path to avoid the check altogether.

[jorbin]

I don't love the idea of potentially doing a database write on the front end.

Overall though, I am wondering if this overall change is even necessary. While the changes before [60666] and [60670] do help follow the current best practices for plugin development, this is adding a lot of specialized code to accomplish it and I wonder if the benefit is really there.

[Otto42]

We don't need to change core code for this. It can be handled entirely by the hello.php file that is already there.

Basically, just include the Hello Dolly plugin as an external, much like akismet already is. That way, the new files are there after an update.

Then, replace the contents of the hello PHP file with a small bit of code that does three things. First it checks if the Hello Dolly plugin is there. Second, it rewrites the active plugins option to stop including itself and to include the new hello plugin. Then finally, it simply includes the Hello Dolly plugin directly. This is basically a one-stop shop to the plugin deactivating itself, making itself irrelevant and changing it to including new plugins as part of the normal plugin load system.

Nothing required in core. Nothing needed in updates logic, it's simply a plugin updating its location and then not being loaded anymore. Seems like a very simple process.

[swissspidy]

One of the concerns I raised about this change is that the new hello-dolly/hello.php file in core does not match the one in the plugin directory, so any checksum verifications are failing. Seeing this now in WP-CLI tests for instance. Including it as an external would fix this.

[SergeyBiryukov]

In 60716:

Coding Standards: Move upgrade_690() to the correct place, after upgrade_682().

Follow-up to [60666].

See #53323.

• Moves the call to upgrade_690() so it actually gets called
• Wraps Hello Dolly upgrade code in a version check per standard practice
• Removes the $wpdb->query() call to avoid attempting to create the new index twice, once in the upgrade and once in dbDelta().

Combined fast follows for two tickets:

• https://core.trac.wordpress.org/ticket/53323
• https://core.trac.wordpress.org/ticket/50161

[SergeyBiryukov]

In 60721:

Upgrade/Install: Correct the database upgrade routine for WordPress 6.9.

This commit:

• Moves the call to upgrade_690() to the correct place so it actually gets called.
• Wraps Hello Dolly upgrade code in a version check per standard practice.
• Removes the $wpdb->query() call to avoid attempting to create the new index twice, once in the upgrade and once in dbDelta().

Follow-up to [60666], [60716], [60717].

Props peterwilsoncc, mukesh27.
See #50161, #53323.

Thanks for the PR! Merged in r60721.

[dd32]

get_option() should have a default of an empty array here, to account for Multisite networks where an individual site doesn't have plugins active.

E_ERROR: Uncaught TypeError: array_search(): Argument #2 ($haystack) must be of type array, string given in wp-admin/includes/upgrade.php:2504

Stack trace:
#0 wp-admin/includes/upgrade.php(2504): array_search('hello.php', '', true)
#1 wp-admin/includes/upgrade.php(890): upgrade_690()
#2 wp-admin/includes/upgrade.php(678): upgrade_all()
#3 wp-admin/upgrade.php(166): wp_upgrade()
#4 {main}
 thrown
Source: GET https://example.org/wp-admin/upgrade.php?step=1

FYI @SergeyBiryukov

---

edit: Seems that there's no consistency between get_option( 'active_plugins' ), get_option( 'active_plugins', array() ) and (array) get_option( 'active_plugins', array() ): ​https://github.com/search?q=repo%3AWordPress%2Fwordpress-develop%20get_option(%20%27active_plugins%27&type=code

[SergeyBiryukov]

In 60725:

Upgrade/Install: Cast get_option( 'active_plugins' ) to array in upgrade_690().

This resolves an error on Multisite networks where an individual site doesn't have plugins active:

array_search(): Argument #2 ($haystack) must be of type array, string given

Follow-up to [60666], [60721].

Props dd32.
See #53323.

[davidbaumwald]

Removing reporter-feedback

[welcher]

This was reviewed in the 6.9 bug scrub today. It seems that we need to get aligned to move this across the finish line in time for beta 1 in a couple of weeks.

[afragen]

Replying to Otto42:

We don't need to change core code for this. It can be handled entirely by the hello.php file that is already there.

Basically, just include the Hello Dolly plugin as an external, much like akismet already is. That way, the new files are there after an update.

Then, replace the contents of the hello PHP file with a small bit of code that does three things. First it checks if the Hello Dolly plugin is there. Second, it rewrites the active plugins option to stop including itself and to include the new hello plugin. Then finally, it simply includes the Hello Dolly plugin directly. This is basically a one-stop shop to the plugin deactivating itself, making itself irrelevant and changing it to including new plugins as part of the normal plugin load system.

Nothing required in core. Nothing needed in updates logic, it's simply a plugin updating its location and then not being loaded anymore. Seems like a very simple process.

@Otto42 any chance you might be able to put the above in code so we can cross the finish line? Please and thank you.

[peterwilsoncc]

With the deactivation problems, I think the best approach for 6.9 would be to revert the existing changes and if Otto's suggested approach works aim to get that in for the beta release.

Patch prepared in SVN by running the following commands:

svn merge -c -60725 .
svn merge -c -60721 .
svn merge -c -60716 .
svn merge -c -60770 .
svn merge -c -60666 .

Trac ticket: https://core.trac.wordpress.org/ticket/53323

[jorbin]

If someone is able to give @Otto42's idea a shot, I would love to see a patch. In the meantime, I've prepared a PR to confirm the revert.

[afragen]

FYI, Matt just brought this up in his WCEH AMA.

​https://www.youtube.com/watch?v=RL-XccK30sY

[peterwilsoncc]

In 61006:

Upgrade/Install: Revert relocation of Hello Dolly plugin.

Reverts Hello Dolly changes moving the plugin to a containing folder. Removes the upgrade_690() function in it's entirety as the upgrade routine is no longer required.

Fully reverted commits: [60666], [60670], [60716], [60725]; partially reverts [60721].

Porps johnbillion, whyisjake, SirLouen, mindctrl, afragen, jorbin, Otto42, swissspidy, welcher, davidbaumwald.
See #53323.

Merged r61006 / ​https://github.com/WordPress/wordpress-develop/commit/f906ea54202b17b450a94d87d0180e762ca83516

Although I forgot to do record only for the reverts, sorry.

[peterwilsoncc]

Moving to the 7.0 milestone for another look with @Otto42's approach suggested in comment #87.

As there were a few conflicts, it wasn't possible to do simple reverts of the earlier commits so it was done manually. I subsequently forgot to do record-only reverse merges sorry.