WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#6262 closed defect (bug) (fixed)

Automatic Plugin Upgrade could break new plugins that require special instructions for upgrading

Reported by: nerrad Owned by:
Milestone: 2.7 Priority: high
Severity: major Version: 2.5
Component: Administration Keywords: dev-feedback 2nd-opinion has-patch
Focuses: Cc:

Description

First off, I love the new "automatic download" and upgrade feature for plugins coming with 2.5. A couple of concerns though:

  1. Does this disable the plugin first and then reactivate it after the download or does the automatic download just overwrite the files? If it's the latter this could cause problems where a plugin being upgraded needs to run the activation hook to bring in new changes.
  2. There doesn't seem to be anyway for plugin authors to indicate special instructions for upgrading from older versions.

Possible solutions:

  1. A good temp solution would be to simply put in a warning for users to check the plugin page on WordPress.org first for any special upgrade instructions.
  2. Introduce a new file type/name that if plugin authors include with their plugin will be executed following the ftp allowing special upgrade tasks to be done. I suggest _pluginname-upgrade.php.
  3. Create a new tag for the readme.txt file in plugin packages (i.e. Special Instructions) so that when the readme.txt file is parsed it will put in an alert and display the special instructions in the plugin list. Plugin authors would then have a way of notifying potential users of possible problems with a simple automatic upgrade.

Attachments (2)

6262.diff (5.6 KB) - added by DD32 7 years ago.
6262.2.diff (5.9 KB) - added by DD32 7 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 @DD327 years ago

  • Keywords dev-feedback 2nd-opinion added

Does this disable the plugin first and then reactivate it after the download or does the automatic download just overwrite the files?

Simply overwrites the files, The same that would happen if the user was to manually upgrade.

No hooks are run to notify the plugin that an upgrade has taken place, This is due to the old plugin being loaded at the time of upgrading.

I asked on #wordpress-dev about it, And it seemed to be agreed on that this just automates the process most users follow anyway, If a plugin needs to do an upgrade proceedure, then it should reconise when its been updated(ie. like wordpress does with its database version); and perform an action then.

There doesn't seem to be anyway for plugin authors to indicate special instructions for upgrading from older versions.

Correct, No extra screens are shown to the blog owner.

Create a new tag for the readme.txt file in plugin packages (i.e. Special Instructions) so that when the readme.txt file is parsed it will put in an alert and display the special instructions in the plugin list. Plugin authors would then have a way of notifying potential users of possible problems with a simple automatic upgrade.

I was thinking of maybe the readme.txt should be parsed, and if a section labled 'upgrading'(or similar) exists, then maybe WordPress could show that afterwards?

Also note, that if a plugin was previously at plugins/joes.php, when the upgrader runes over it, it'll exist at /plugins/joes/joes.php, In that case, The plugin will no longer be activated, and needs to be activated manually. I was thinking of adding a section to the bottom of the upgrade page "This plugin is no longer Activated, Do you wish to re-activate? [Yes] [No]"

If however, it existed at /plugins/joes/joes.php and it gets upggraded to /plugins/joes/joes.php, it'll stay activated.

comment:2 @nerrad7 years ago

I asked on #wordpress-dev about it, And it seemed to be agreed on that this just automates the process most users follow anyway, If a plugin needs to do an upgrade proceedure, then it should reconise when its been updated(ie. like wordpress does with its database version); and perform an action then.

Makes sense, except that this needs to be broadcast to plugin authors because although the usual behaviour is to simply overwrite files there are cases where a new version requires deactivation and reactivation (at least the way a plugin author has been used to coding their plugins). Of course the fix is as simple as instructing a user to deactivate and reactivate a particular plugin but I suspect the support forums will be busy ;)

I was thinking of maybe the readme.txt should be parsed, and if a section labled 'upgrading'(or similar) exists, then maybe WordPress could show that afterwards?

Good idea.

Also note, that if a plugin was previously at plugins/joes.php, when the upgrader runes over it, it'll exist at /plugins/joes/joes.php, In that case, The plugin will no longer be activated, and needs to be activated manually.

This could be a breaker for plugins that don't reference their files properly - again - this needs to be broadcast if there are no changes planned for 2.5 release.

comment:3 follow-up: @markjaquith7 years ago

Any plugin that doesn't reference its files correctly is already broken. Whether it is the plugin upgrade putting it in an unexpected path or merely a fastidious manual installer, it's still a bug with the plugin.

Same with special upgrade routines that trigger on activation hooks... that's bad practice. Activation/deactivation hooks are for initialization/cleanup, not upgrades. The plugin should handle upgrades through some other method.

Neither of these problems are unique to the auto-upgrader. These are problems that people could easily have run into with manual installation/upgrade as well.

DD32, why do we need to ask if a plugin should be re-activated after being moved? Can't we assume that?

comment:4 in reply to: ↑ 3 @nerrad7 years ago

Replying to markjaquith:

Any plugin that doesn't reference its files correctly is already broken. Whether it is the plugin upgrade putting it in an unexpected path or merely a fastidious manual installer, it's still a bug with the plugin.

Same with special upgrade routines that trigger on activation hooks... that's bad practice. Activation/deactivation hooks are for initialization/cleanup, not upgrades. The plugin should handle upgrades through some other method.

Neither of these problems are unique to the auto-upgrader. These are problems that people could easily have run into with manual installation/upgrade as well.

Mark, you make valid points and I don't disagree with them. However, the automatic upgrader "forces" these standards. At the very least if there is a "proper" way to be coding plugins THEN there must be 'clear documentation' for plugin authors regarding checking their work into the dev.wp-plugins.org repository.

Not only that but the Plugin documentation on the WordPress Codex needs a major overhaul before releasing the automatic upgrade feature to highlight standards plugin authors should follow.

As for activation/deactivation hooks somebody needs to take a look at this linky on the codex because it sure doesn't seem to describe the "correct way" to use the 'register activation hook' as you've described Mark.

I sure appreciate your input, at the very least it's given me some direction for reexamining how I setup my plugins :) IMHO I think this feature needs to be thought through some more and pushed back to 2.6 so more docs can put up outlining the standards this forces.

Especially when the codex has this as a correct way to use the ‘register_activation_hook’.

comment:5 @DD327 years ago

Allright.

I've written up some changes, Now plugins are deactivated during the upgrade process(Just before files are modified), And reactivation is attempted after the upgrade proceedure completes.

The activation hooks run during activation, so plugins can hook upgrade functionality, however, interactive upgrade wizard or something wouldnt be encouraged as it runs within a small iframe(170px high).

Deactivation hooks are NOT run, as the plugin isnt being killed off, and certain plugins are known to hook the deactivation to redirect the user elsewhere to remove all settings.. It also prevents some plugins from clearing their cached files, And since the plugin isnt being actually deactivated, it makes sense to me.

I'll post a patch tomorrow after some attention has been given to another patch thats needed first.

(So yes, "DD32, why do we need to ask if a plugin should be re-activated after being moved? Can't we assume that?", your right, if it was previously activated, might as well go ahead and activate it again.

comment:6 @DD327 years ago

Deactivation hooks are NOT run

However, the plugin is removed from the active plugins list, otherwise a guest hitting the wordpress page half way through the process may get a nasty php error due to incomplete files.

comment:7 @nerrad7 years ago

sounds good - so I take it hopes are to get this in before 2.5 is released?

@DD327 years ago

comment:8 @DD327 years ago

  • Keywords has-patch added

Added a patch.

so I take it hopes are to get this in before 2.5 is released?

If the lead devs think its a good idea(which i believe they do), then yeah.

Havnt done anything on the readme.txt idea, At that point the plugins been upgraded anyway, the user cant go back, any special plugin needs can be taken care of on the activation hook.

comment:9 @nerrad7 years ago

Yeah don't think anything needs to be done with the readme.txt with the changes. Thanks for the attention :)

comment:10 @ryan7 years ago

DD32, looks good. I think we should add the is_plugin_active() function. Just have one question. What is this line for:

apply_filters('deactivate_plugin', __('Deactivating the plugin'));

There's no lvalue, so the filter doesn't do anything.

comment:11 @DD327 years ago

I think we should add the is_plugin_active() function

I'm with you there. I nearly created it too.

There's no lvalue, so the filter doesn't do anything.

That should've been apply_filters('update_feedback'.. same as the rest of the feedback lines

comment:12 @DD327 years ago

You can either go ahead and make the modifications, Or i'll modify it later.

@DD327 years ago

comment:13 @ryan7 years ago

(In [7465]) Deactivate plugins during plugin update. Props DD32. see #6262

comment:14 @DD327 years ago

  • Milestone changed from 2.9 to 2.7
  • Resolution set to fixed
  • Status changed from new to closed

WordPress 2.7 allows users to view the information from the readme file before upgrading, This seems to solve one of the points above. Theres another ticket somewhere requesting filters on the deletion steps too.. So that'll take care of the other concerns.

I'm closing as fixed in 2.7

Note: See TracTickets for help on using tickets.