WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 6 months ago

Last modified 6 months ago

#22704 closed task (blessed) (fixed)

Automatic Core Updates

Reported by: pento Owned by: pento
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.5
Component: Upgrade/Install Keywords: has-patch
Focuses: Cc:

Description

It's time to think about automatic updates for WordPress Core. Plugins and Themes are a totally different ball game, so it's probably best to leave them for the moment. Currently, I'm thinking it would be a good idea to release this in stages (some of which may be combined, just spelling them out):

  • SVN updates in trunk installs
  • SVN updates in branch installs
  • Opt-in updates in stable installs
  • Opt-out updates in fresh installs
  • Opt-out updates in all installs
  • Remove option for opting out

I'd like to see SVN updates go into 3.6 early, so we can quickly get a good idea of compatibility issues that we're likely to run into when we get to beta.

Finally, are there any features we should be looking at adding to the upgrader for this? More sanity checking, notifications, other?

Attachments (29)

22704.diff (26.0 KB) - added by dd32 8 months ago.
background-upgrader.php (13.4 KB) - added by dd32 8 months ago.
22704.2.diff (13.0 KB) - added by dd32 7 months ago.
background-upgrader.2.php (11.3 KB) - added by dd32 7 months ago.
A copy of the latest mu-plugin i was using before integrating as the patch
22704.admin_email_filter.diff (2.0 KB) - added by JustinSainton 7 months ago.
22704.admin_email_filter.2.diff (2.0 KB) - added by JustinSainton 7 months ago.
22704_hg_bzr.diff (725 bytes) - added by jamescollins 7 months ago.
22704_dashboard_update_message.diff (1.1 KB) - added by jamescollins 7 months ago.
22704_r25650.diff (2.2 KB) - added by GaryJ 7 months ago.
Refactor of r25650
22704.stats.diff (4.0 KB) - added by dd32 7 months ago.
22704.3.diff (1.7 KB) - added by johnbillion 7 months ago.
22704.4.diff (4.5 KB) - added by dd32 6 months ago.
22704.5.diff (1.3 KB) - added by dd32 6 months ago.
22704.6.diff (2.8 KB) - added by dd32 6 months ago.
22704.whitescreen-rollback.diff (3.2 KB) - added by dd32 6 months ago.
22704.7.diff (11.2 KB) - added by nacin 6 months ago.
22704.8.diff (2.5 KB) - added by dd32 6 months ago.
22704-reactivate-plugins.patch (1.9 KB) - added by kurtpayne 6 months ago.
Reactivate plugins after updating
22704-reactivate-plugins.2.patch.diff (1.9 KB) - added by kurtpayne 6 months ago.
Reactivate plugins after updating ... but only if they were active before updating
words.diff (2.0 KB) - added by jorbin 6 months ago.
22704.9.diff (664 bytes) - added by nacin 6 months ago.
22704-reactivate-plugins.3.patch.diff (875 bytes) - added by kurtpayne 6 months ago.
Reactivate plugins after updating, only during auto-upgrade though
22704.10.diff (2.2 KB) - added by dd32 6 months ago.
22704.11.diff (2.0 KB) - added by dd32 6 months ago.
22704.12.diff (1.9 KB) - added by dd32 6 months ago.
22704.13.diff (1.9 KB) - added by dd32 6 months ago.
22704.14.diff (10.5 KB) - added by nacin 6 months ago.
Changes upgrade() to update() in the automatic upgrader, which is now called the background updater. (It could be useful for more than just 'auto' updates.) The skin remains the Automatic_Upgrader. It kind of makes sense in my head.
22704.emails.diff (1.5 KB) - added by dd32 6 months ago.
22704.15.diff (1.0 KB) - added by dd32 6 months ago.

Download all attachments as: .zip

Change History (191)

comment:1 pento17 months ago

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

comment:2 in reply to: ↑ description ; follow-up: nacin17 months ago

Replying to pento:

Finally, are there any features we should be looking at adding to the upgrader for this? More sanity checking, notifications, other?

Probably three-fold:

  • Sanity checking via file hashing (confirming that files actually got copied over)
  • Sanity checking through some other means, like a parallel install
  • Notifications

I've never really thought heavily about SVN updates. As a developer, having an SVN site update automatically is weird and probably not desirable. If you managed to do a checkout, you can set up a cron to update it.

The first step is probably opt-out (or opt-in, if we are chicken) updates for security and maintenance releases. The next step is probably an update from major-to-major after the .1 drops.

Warning you now, this Trac ticket is likely to get out of hand, quickly. Trac is more about implementation, not planning out potential roadmaps. (Though I applaud your initiative and can't wait to see you build it.) Summaries of the extensive Updates discussion from the community summit should probably be published (if it hasn't been already), and then this should be taken to a P2 thread.

comment:3 in reply to: ↑ 2 bpetty17 months ago

Replying to nacin:

Though I applaud your initiative and can't wait to see you build it.

He has: http://wordpress.org/extend/plugins/automatic-updater/

At Bluehost, we've been researching this for a few months now (and yes, we have tested your plugin a bit with a lot of success - but it certainly has it's limitations right now).

Anyway, our biggest problem has very little to do with how automatic upgrades are implemented (whether it's in core, or in an mu-plugin we provide ourselves). It's that eventually, all installed plugins and themes will break themselves, core, or both upon a major upgrade of WordPress core in an undetectable way; just backwards incompatibility issues. I've run the statistics as best as I can without actually using our customers as test subjects, and I've even written a proposal for one possible solution to this (which also contains some of those stats).

comment:4 markoheijnen17 months ago

I don't understand why WordPress core needs this. Seems like a small percentage will use this and this feels like an item that shows how WordPress is all about feature creeping instead of being a robust platform.

In my opinion there isn't a good way and still have an easy to use system. Maybe when we add a lot of black magic to WordPress but for me that just feels wrong to do so.

comment:5 SergeyBiryukov17 months ago

Related: #15738

Version 0, edited 17 months ago by SergeyBiryukov (next)

comment:6 mikehansenme17 months ago

  • Cc mikehansenme added

From what I have seen(in the past working with clients), most WordPress installs need to be upgraded. Why would we not want to do this automatically, assuming we can provide the right experience? The problem is the experience a user will have when something does go wrong, that is why there needs to be checks. IMO auto upgrades are about security not new features. If we can keep plugins/themes updated as well, I think the amount of hacked sites we see will go down. Replacing the experience of "My site was hacked" with "My site is up to date automatically?". I think we could live with that. Explaining to them how it stayed up to date vs explaining how to fix a hacked install. Will this fix everything? No. But it will help.

comment:7 vanillalounge17 months ago

If I'm geting this right, #18200 (birth of the language pack) is probably somewhat related, too.

comment:8 sennza17 months ago

  • Cc bronson@… added

comment:9 MikeSchinkel17 months ago

  • Cc mike@… added

Replying to mikehansenme:

IMO auto upgrades are about security not new features. If we can keep plugins/themes updated as well, I think the amount of hacked sites we see will go down.

I would tend to agree. Auto update to major versions is something I can see breaking sites, but of course if you can disable auto-update for major versions via a hook then I don't think it will be a problem. If there's significant custom development on a site that might break then the site builder has the skills to add the "no-major-version-update" hook as well.

For plugins I think that would require version numbers to actually matter, and right now there's no well-known advice provided for plugin developers on how to do versioning "right". Maybe WordPress plugins could adoption Semantic Versioning as the best practice and then opt-in via a header line "Auto Update: Yes" for automatic versioning?

comment:10 barrykooij17 months ago

  • Cc wpcore@… added

comment:11 ipstenu-dh17 months ago

  • Cc ipstenu-dh added

comment:12 Ipstenu17 months ago

If we do auto-upgrade, there has to be an opt-out method (an option define(AUTO_UPGRADE, false); at the very least) so we can regression test plugins and hacks. While auto-upgrades, in general, are awesome ideas for the small-fry sites (casual bloggers with limited tech chops and few plugins), it becomes problematic when you introduce plugins like BuddyPress, which often must be upgraded with WP in order to work to it's fullest potential (or work at all, and this is not a dig on BP, it's just that it's very much tied in to core - nature of the beast).

Auto-upgrading minor releases strikes me as a better first-step. Kind of like the hotfix, only with less of a 'You still need to upgrade the plugin' need. That would help security a lot, and make it possible for in a case where we have, say, the need for a fix in v 3.5.3 the day before we want to drop 3.6 to cover both. A silent auto-up of a couple files (now that we have the incremental updates yay!) would be unnoticed by most users, and CYA till they can do the major up. From a support persective, the minor upgrades aren't 100% painless, but they're usually quiet enough that I wouldn't envision massive headaches for this.

I am NOT proposing we support older versions, just putting a use-case out there. I recall we've had cases where we had to upgrade a previous release due to the timing of the major release, and this could help us out there and make it less of a headache.

comment:13 MikeHansenMe17 months ago

I would be ok with 2 options to enable/disable minor and major versions of automatic updates as a starting point. I do think it would help get the ball rolling by reducing concerns some users may have.

comment:14 Japh17 months ago

  • Cc japh@… added

comment:15 DH-Shredder17 months ago

Definitely in agreement that it seems appropriate to start from the minor and security upgrades for a first pass.

From what I've seen, we have generally very low volumes of support (to none) when we run minor upgrades. I'm sure that if these became automated in core, everyone committing would be even more careful about what goes in them.

The biggest security concerns come from plugins, so it'd be good to have a solution that keeps them in mind for the future (perhaps a tag to let plugin authors specify an upgrade as a security update, which was talked about at the summit).

As far as major release auto-upgrades go, I'm a fan of moving forward on them, but with this as a second or third step (likely at least two major releases forward from now).
Plugins and themes need to be, at the very least, taken into account if we want to do them properly. As it's been talked about above, it'll be important to avoid auto-upgrades if it's going to cause a fatal error. This situation is the one we see most often when we auto-upgrade users, and generally speaking it's a problem because the users haven't kept their plugins up to date before the core auto-upgrade takes effect.

comment:16 DH-Shredder17 months ago

  • Cc mike.schroder@… added

comment:17 bpetty15 months ago

  • Cc bpetty added

comment:18 bungeshea10 months ago

  • Cc info@… added

comment:19 naomicbush9 months ago

  • Cc naomicbush added

comment:20 nacin8 months ago

  • Milestone changed from Awaiting Review to 3.7
  • Type changed from feature request to task (blessed)

comment:21 nacin8 months ago

#15738 was marked as a duplicate.

comment:22 sbressler8 months ago

  • Cc sbressler@… added

comment:23 toscho8 months ago

  • Cc info@… added

comment:24 dd328 months ago

In 25219:

Core Updates: Switch to using a new 1.7 version check API which will return extra auto-update packages when requested over SSL. The new API has switched to returning JSON. See #22704

comment:25 dd328 months ago

In 25220:

Core Updates: Ensure that the system supports outgoing HTTPS requests before making an update check over HTTPS. Fixes a typo in [25219]. See #22704

comment:26 follow-up: nofearinc8 months ago

I've been working on some proof of concept based on the Automatic Updater plugin as I've used it before on multiple installs and it's pretty stable and reliable. One important question though - are we implementing the updater for single site installs only or multisites as well? The multisite update could be tricky and dangerous and the plugin does not support it at the moment.

comment:27 alex-ye8 months ago

  • Cc nashwan.doaqan@… added

comment:28 in reply to: ↑ 26 Ipstenu8 months ago

Replying to nofearinc:

The multisite update could be tricky and dangerous and the plugin does not support it at the moment.

I've been using this on a Multisite network for months now, and it works fine. And is totally supported: "If you're working on a WordPress Multisite install, it will properly restrict the options page to your Network Admin."

comment:29 nofearinc8 months ago

Thanks Ipstenu, haven't tried it on my multisites so it's my bad.

comment:30 dd328 months ago

In 25227:

Upgrader: Split the UI "skins" out from the main Upgrader file into a seperate file to reduce the length of the files. See #22704

comment:31 dd328 months ago

In 25228:

Core Auto Updates: Add a initial skin to allow capturing the output from the upgrader without displaying it (such as we need during cron calls).
This has been copied almost verbatim from the automatic-updater plugin with a few style tweaks and additional upgrade-possible check. See #22704

dd328 months ago

comment:32 dd328 months ago

Attachment attachment:22704.diff​ added

Ok, that's a POC that i've been working with. An eagle-eyed person will note that it is basically written from scratch rather than using the Auto Updater plugin (And the author of said plugin agree's with the approach), coming at it from a completely different angle since we have different use-cases in mind.

It's an mu-plugin simply for the ease of testing (Believe me, there's nothing more fun than the code you're working on overwriting itself with the unpatched version!), the class-wp-upgrader.php changes were me removing the first hacks from the class, but i'm not quite happy with the way it's implemented.. but it seems to be on the right track.

The clear_update_cache changes relate to plugin auto-upgrades (which will NOT be enabled by default for 3.7) - basically only one plugin could be updated per pageload, as it would clear the cached updates for the other plugins once it finished.. There's probably a better way of fixing it than avoiding clearing the transients though.

At this point, fast iterations are needed to get it into core, so my aim is to commit as much of this over the next few days while cleaning up and working on other stability, performance, and sanity checks.

If someone else would like to read through it, comment on the direction, and/or give some thoughts to how I've gone about it, it'd be appreciated.

Topics such as making backups and rollbacks and all that fun stuff are acknowledged, if someone wants to head over to #10666 and dive in it'd be great, but there are some significant issues which prevent most of the ideal methods of doing that IMHO at present, so my aim is to make updates as bulletproof as possible, rather than supplying everyone with a Tshirt and first-aid kit - at least for now.

Last edited 8 months ago by dd32 (previous) (diff)

comment:33 dd328 months ago

In 25272:

Upgrader: Make clearing the Plugin and Theme update caches optional during install and upgrade proceedures. See #22704

dd328 months ago

comment:34 Denis-de-Bernardy7 months ago

+1 on the remarks about it needing a means to be disabled.

In particular, it should be disabled by default if WP detects a .git or .svn repo somewhere in the WP root folder.

Auto-updates should only ever occur for sites setup without source control, and even then methinks WP should be very careful.

dd327 months ago

comment:35 dd327 months ago

Attachment attachment:22704.2.diff​ added

Warning: Patch completely untested.

Alright, So this needs some feedback from others now. Instead of just dropping the Background_Upgrader class into core as-is, I've tried to integrate it as a patch above.

It doesn't really fit into any kind of structure which core currently has, since the upgrader is built with the notion of "Perform this action now" from an admin page, not, "Here's a bunch of moving parts which work together".

If anyone has any implementation suggestions, or things to change, I'd love to hear them.

dd327 months ago

A copy of the latest mu-plugin i was using before integrating as the patch

comment:36 dd327 months ago

Something that is not implemented in either of those attachments, is that we should only upgrade them if the data is delivered over a HTTPS connection.

The WordPress.org core version check API only returns automatic update offerings when SSL is used, The plugin/theme update checker also returns SSL resources, but, the automatic upgrader doesn't verify that.

comment:37 dd327 months ago

In 25421:

WordPress Core Automatic Updates: Add the first slice of Automatic Upgrades, This is presently disabled, and requires a filter to enable ( 'auto_upgrade_core' ). See #22704

comment:38 dd327 months ago

$when_to_update = apply_filters( 'auto_upgrade_when_to_upgrade', $when_to_update );
if ( ! apply_filters( 'allow_dev_auto_core_updates', $upgrade_dev ) )
return apply_filters( 'allow_minor_auto_core_updates', $upgrade_minor );
return apply_filters( 'allow_major_auto_core_updates', $upgrade_major );
return apply_filters( 'auto_upgrader_disabled', false );
if ( ! apply_filters( 'auto_upgrade_ignore_checkout_status', false ) ) {
if ( ! apply_filters( 'auto_upgrade_' . $type, $upgrade, $item ) )

All of those filters need a once-over for naming, the upgrader already uses upgrader_pre_* for it's filters, makes sense to use something similar here.
The difference between upgrade/upgrader/update/updates is a worth while distinction to keep in mind, an update is a available update, upgrader is the thing that does the upgrade, which is why the existing filters are a bit here and there.

comment:39 dd327 months ago

In 25422:

WordPress Core Automatic Updates: Pass the Filesystem path as $context to request_filesystem_credentials(). See #22704

comment:40 follow-up: pavelevap7 months ago

What about localized versions? When there is only English version available, there should be update of English version? And when localized version available later, then there will be another update?

comment:41 in reply to: ↑ 40 dd327 months ago

Replying to pavelevap:

What about localized versions? When there is only English version available, there should be update of English version? And when localized version available later, then there will be another update?

Localised versions are a bit of a pain here.. but don't worry, we've not completely forgotten them. The plan is mostly to just use English versions for all automatic updates - which, will be only point releases to start with, so the manually-translated files should not need altering. Language Packs (#18200) should take care of updating the language files for each release.

In the event that the above doesn't plan out, we'll have to re-think our approach, but we'd have several options, including, having the API return localised autoupdate packages, or, simply not returning autoupdate offerings for non-english sites (instead, relying on the existing manual updates which will still work). Both are things we can look into in the event Language packs don't quite make it.

comment:42 nacin7 months ago

[25421] introduces a closure, which produces a syntax error in 5.2. I'm going to have to check our post-commit hook, as the commit should have been rejected...

comment:43 dd327 months ago

In 25447:

WordPress Core Automatic Updates: Remove an accidental closure which isn't supported in PHP 5.2. See #22704

comment:44 dd327 months ago

In 25466:

WordPress Core Automatic Updates: Switch to a twicedaily cronjob to match the update check cron jobs, this removes the ability for update checks to continuously re-queue a upgrade job. See #22704

comment:45 dd327 months ago

In 25467:

WordPress Core Automatic Updates: Switch from using a transient for locking the upgrade process, to using a site option. See #22704

comment:46 dd327 months ago

In 25468:

WordPress Core Automatic Updates: Remove some debug. See #22704

comment:47 dd327 months ago

In 25495:

Upgrader: Fix the order of arguements passed to wp_parse_args() in the Upgrader, introduced with [25272]. See #22704

comment:48 dd327 months ago

In 25496:

WordPress Core Automatic Updates: Add a post-upgrade summary email to the WordPress install's admin email address. See #22704

comment:49 follow-up: JustinSainton7 months ago

Wondering if it might be nice to have a filter around the send_email() function? For me, at least, I don't care to be notified every time WP automatically updates (iOS7 won't email me, Chrome doesn't email me, etc.)

Something like -

if ( apply_filters( 'auto_upgrade_email_notice', true, $core_update, $theme_updates, $plugin_updates ) ) 
    self::send_email();

Thoughts?

comment:50 in reply to: ↑ 49 DrewAPicture7 months ago

Replying to JustinSainton:

Something like -

if ( apply_filters( 'auto_upgrade_email_notice', true, $core_update, $theme_updates, $plugin_updates ) ) 
    self::send_email();

Thoughts?

First, I agree a filter here would be nice.

On the name, personally, I'm kind of partial to filter hooks that enable or disable something to use that in the filter hook name.

Maybe enable_auto_upgrade_email? Or the reverse, disable_auto_upgrade_email default to false, but then you have to test the double negative which is a little weird.

comment:51 follow-up: JustinSainton7 months ago

Yeah, I suppose I'd prefer enable to disable...would be nice to use __return_false to disable, and semantically, add_filter( 'enable_auto_upgrade_email', '__return_false' ) makes more sense to me. If that's the general consensus, happy to provide a patch.

comment:52 in reply to: ↑ 51 DrewAPicture7 months ago

Replying to JustinSainton:

Yeah, I suppose I'd prefer enable to disable...would be nice to use __return_false to disable, and semantically, add_filter( 'enable_auto_upgrade_email', '__return_false' ) makes more sense to me. If that's the general consensus, happy to provide a patch.

Please do, plus hook docs ;)

comment:53 follow-up: JustinSainton7 months ago

First pass at inline filter documentation. Be gentle, @DrewAPicture.

comment:54 in reply to: ↑ 53 DrewAPicture7 months ago

Replying to JustinSainton:

First pass at inline filter documentation. Be gentle, @DrewAPicture.

Looks pretty good for a first inline docs patch!

Just a few things:

  • The short description should probably mention 'email' somewhere. I suggest something like "Filter whether to email an update summary to the site admin." Whatever you make it, it needs a period at the end.
  • Add "Default true." to the end of the first @param line.
  • The $core_update line @param is a little confusing. False on what failing? And "otherwise the core update offering" is pretty vague. So maybe, "An array of core update data, false otherwise."

Other than that stuff, it looks great.

comment:55 JustinSainton7 months ago

Doneskies. FWIW, the $core_update line came straight from the return param for find_core_auto_update(). I don't disagree with your criticisms though.

comment:56 follow-up: dd327 months ago

Wondering if it might be nice to have a filter around the send_email() function? For me, at least, I don't care to be notified every time WP automatically updates (iOS7 won't email me, Chrome doesn't email me, etc.)

That's fair enough, but they're also far more common things, right now, I'm thinking we should force everyone to receive it.. filter it in your mail client if you absolutely hate it, and we can add the filter before we hit release.

The simple reason is that right now, the emails are the only way you can possibly know something has failed with the update, if you're running trunk right now, and you have it setup to receive updates, that means you're testing out the functionality and need to be able to report if something breaks.

comment:57 in reply to: ↑ 56 JustinSainton7 months ago

Replying to dd32:

The simple reason is that right now, the emails are the only way you can possibly know something has failed with the update, if you're running trunk right now, and you have it setup to receive updates, that means you're testing out the functionality and need to be able to report if something breaks.

Yep, completely agree - and adding this filter doesn't change any of that - by default, everyone will still get those updates. If you opt-out of them, whether the filter is available now or once 3.7 id out, you opt out knowing that you won't be emailed if something goes awry.

comment:58 dd327 months ago

In 25542:

WordPress Core Automatic Updates: Add a filter as to whether or not to send the site administrator a summary of executed updates. Props JustinSainton. See #22704

comment:59 dd327 months ago

In 25543:

Add a missing . in the Hook documentation from r25542 See #22704

comment:60 dd327 months ago

  • Keywords dev-feedback removed

comment:61 JustinSainton7 months ago

Crap, didn't realize my .diff file had extra stuff in it. Ugh, my bad.

comment:62 dd327 months ago

In 25568:

Language Packs: Integrate Language Packs into the Auto-upgrader. See #18200 See #22704

comment:63 dd327 months ago

In 25598:

Automatic Core Updates: Enable Core Automatic Updates for Security releases, Development nightly releases, and, Language packs. See #22704
For more information on Automatic Core Updates, See #22704 and http://wp.me/p2AvED-1Lo

comment:64 josephscott7 months ago

Minor thing: AUTOMATIC_UPDATER_DISABLED. At one point I thought we'd decided to not use negative options.

I would have expected something like define( 'AUTOMATIC_UPDATER', false ); to turn off updates.

comment:65 dd327 months ago

Minor thing: AUTOMATIC_UPDATER_DISABLED.

This particular constant was inherited from the Automatic Updater plugin, I see no reason why we shouldn't rename it though.

comment:66 dd327 months ago

In 25599:

Automatic Updates: Correct a typo of a constant, s/DISABLE_FILE_MODS/DISALLOW_FILE_MODS/. See #22704

jamescollins7 months ago

comment:67 follow-up: jamescollins7 months ago

22704_hg_bzr.diff also disables auto updates for hg/bzr working copies (in addition to the already disabled svn and git).

As per this suggestion from @mikeschinkel.

Not quite sure of the potential performance impact of adding an additional 2 file_exists() calls.

comment:68 in reply to: ↑ 67 MikeSchinkel7 months ago

Replying to jamescollins:

22704_hg_bzr.diff also disables auto updates for hg/bzr working copies (in addition to the already disabled svn and git).

+1!

comment:69 dd327 months ago

No issues with disabling it for hg/bzr, git/svn were just chosen as that's what's primarily used by more core developers.

comment:70 nacin7 months ago

In 25633:

Changes to automatic background updates in preparation for Beta 1.

  • Show a notice for beta testers on update-core.php explaining the status of their install. Three possibilities: auto updates are enabled, auto updates are disabled because the install doesn't support SSL HTTP requests, and auto updates are disabled because it is a VCS checkout.
  • Improve the output of the email, for maximum debugging potential. Failures are clearly labeled and the email leads testers to the support forums and Trac.
  • Try to create wp-content/languages in the upgrader if it doesn't exist. Our mkdir isn't recursive, so trying to create wp-content/languages/plugins could fail.
  • Abstract out version control checkout determination into a public method. The filter is now auto_upgrade_is_vcs_checkout, still subject to change.

see #22704.

comment:71 kpdesign7 months ago

Just updated via SVN to r25633, getting the following notices on update-core.php:

Strict Standards: Non-static method WP_Automatic_Upgrader::is_vcs_checkout() should not be called statically in C:\xampp\htdocs\wordpress-svn\src\wp-admin\includes\class-wp-upgrader.php on line 1494

Strict Standards: Non-static method WP_Automatic_Upgrader::is_vcs_checkout() should not be called statically in C:\xampp\htdocs\wordpress-svn\src\wp-admin\update-core.php on line 168

These are directly above the pink box with the following message:

BETA TESTERS: This install uses version control (SVN or Git) and thus will not receive auto updates. Try a separate install of WordPress with the Beta Tester plugin set to bleeding edge.

comment:72 follow-up: wonderboymusic7 months ago

Looks like this class should be a Singleton - I loathe classes where every method is static.

comment:73 follow-up: greenshady7 months ago

I can open a separate ticket if needed.

Here's the warnings I received when updating to the latest. This was shown during the update process. This is also on a many-years-old install, so it's not fresh and clean.

Warning: md5_file(/html/blog/wp-content/themes/twentyfourteen/languages/twentyfourteen.pot) [function.md5-file]: failed to open stream: No such file or directory in /html/blog/wp-admin/includes/class-wp-upgrader.php on line 1359

Warning: md5_file(/html/blog/wp-content/themes/twentyfourteen/languages/twentyfourteen.pot) [function.md5-file]: failed to open stream: No such file or directory in /html/blog/wp-admin/includes/update-core.php on line 701

I suspect this has something to do with not actually having the Twenty Fourteen theme installed.

comment:74 nacin7 months ago

In 25635:

Fix strict notice. see #22704.

comment:75 in reply to: ↑ 73 nacin7 months ago

Replying to greenshady:

I can open a separate ticket if needed.

This is actually due to the work in #18201. If you could open a new ticket and just cross-reference it with #18201, that'd be perfect.

comment:76 in reply to: ↑ 72 ; follow-ups: nacin7 months ago

Replying to wonderboymusic:

Looks like this class should be a Singleton - I loathe classes where every method is static.

Yeah, we're really just using it as a namespace. Singletons are also something to be loathed. Suggestions welcome.

comment:77 in reply to: ↑ 76 rmccue7 months ago

Replying to nacin:

Replying to wonderboymusic:

Looks like this class should be a Singleton - I loathe classes where every method is static.

Yeah, we're really just using it as a namespace. Singletons are also something to be loathed. Suggestions welcome.

Singletons and static classes both suck, but at least $this obeys inheritance. Lesser of two evils IMO. (PHP 5.3 has Late Static Bindings though. ;))

comment:78 in reply to: ↑ 76 MikeSchinkel7 months ago

Replying to nacin:

Replying to wonderboymusic:

Looks like this class should be a Singleton - I loathe classes where every method is static.

Yeah, we're really just using it as a namespace. Singletons are also something to be loathed. Suggestions welcome.

Both Singletons and classes with all static methods have their use-cases. Why not consider using a bit of both? Here's an example pattern that illustrates:

Last edited 7 months ago by MikeSchinkel (previous) (diff)

comment:79 follow-up: wonderboymusic7 months ago

Your example would still produce strict errors/warnings if you used $this in the methods hooked with __CLASS__

class Burrito {
    static $instance;

    private function __construct() {
        add_action( 'init', array( $this, 'add_onions' ) );
    }

    public static function get_instance() {
        if ( ! self::$instance instanceof Burrito )
            self::$instance = new self; 

        return self::$instance;
    }

    function add_onions() {
        // now I can use $this with no warnings
    }
}

// Burrito::get_instance() called somewhere

comment:80 in reply to: ↑ 79 MikeSchinkel7 months ago

Replying to wonderboymusic:

Your example would still produce strict errors/warnings if you used $this in the methods hooked with __CLASS__

Ah yes, thanks. Serves me right for not doing exhaustive testing before posting. I've revised the example to be what I had intended:

And the problem with just using a single class like your Burrito is unfortunately PHP does not allow same-named methods to exist both as static and as instance methods so you either get the ability to do something like Burrito::add_salsa() or you want to use singletons have to do something like one of the following:

global $burrito; 
$burrito::add_salsa();
// or
$GLOBALS['burrito']::add_salsa();
// or
Burrito:: get_instance()->add_salsa();

I would think none of those are "clean" enough for a core API.

Also, if you are going to use your class for both hooking actions and/or filters and as an instance class in it's own right then you get into messy business because adding hooks in the __construct() method is no longer appropriate. At that point you are relegated to using static methods for your hooks (which I think you were trying to avoid?) or you are going to have to concoct a really elaborate code pattern to allow instances for hooks and for, ahem, instances.

As a side note, I've always disliked instantiation inside get_instance() if the class is always going to be loaded, i.e. if it hooks WordPress actions or filters. Here's how I might rewrite if I were ignoring the other issues I surfaced:

class Burrito {
    /* 
      * Private because we don't need to allow direct acccess. 
      * Underscore to indicate it's not public
      */
    private static $_instance;  

    static function on_load() {
       /* 
        * Call like this and you know your hooks are hooked.
        */
       self::$_instance = new self; 
    }

    private function __construct() {
       add_action( 'init', array( $this, 'add_onions' ) );
    }

    static function get_instance() {
       /* 
        * No need to check for instantiation _every_ access.
        */
       return self::$_instance;
    }

    function add_onions() {
       // smelly business, really.
    }
}
Burrito::on_load();
Last edited 7 months ago by MikeSchinkel (previous) (diff)

comment:81 dd327 months ago

In 25647:

Automatic Updates: Skip doing the sanity checking MD5 before deciding if we should use a partial build or not, This change means we always use a partial build for automatic updates. See #22704

comment:82 dd327 months ago

In 25648:

MD5 file verification: Prevent md5_file() warnings when files don't exist, additionally, don't verify wp-content files as they can be updated separately, as well as WP_CONTENT_DIR being set elsewhere. See #22704 See #18201

comment:83 dd327 months ago

In 25649:

Automatic Updates: Add a rollback functionality upon installation failure, the rollback package will be available for partial-updates for automatic updates and be similar to our existing partial builds (but in reverse).
A further iteration of this is to also detect whitescreens (fatals) after a auto update, and trigger the rollback for that too.
See #22704

comment:84 dd327 months ago

In 25649:

Todo:

  • The Automatic updater needs to check for whitescreens and trigger it then
  • The Automatic updater needs to detect failures, and not try to update to the new version twicedaily when it whitescreens/fails

comment:85 dd327 months ago

In 25650:

Automatic Updates: Disable Automatic updates for Mercurial(HG) and Bazaar(bzr) version control systems in addition to Git and SVN. Props jamescollins. See #22704

comment:86 dd327 months ago

In 25651:

Automatic Updates: Remove some debug from [25649] See #22704

comment:87 jamescollins7 months ago

22704_dashboard_update_message.diff simplifies the Beta Testers message now that we look for 4 version control systems as of [25650].

Rather than list all VCS types, I was thinking a simpler generic message would be best, particularly given that the auto_upgrade_is_vcs_checkout filter can now be used.

GaryJ7 months ago

Refactor of r25650

comment:88 GaryJ7 months ago

22704_r25650 is a small refactor of r25650, reducing duplicate code (multiple file_exists() calls) by moving the VCS directory names into an array which is then looped through. Also updates the function short description to indicate that more than just Git and SVN are now checked.

May want that '/' added via a trailingslashit() call instead of an appended string.

comment:89 Ipstenu7 months ago

I got the update for 3.7-beta1-25639-20130929 twice (midnight and noon today). Isn't that supposed to check if I'm on THAT version and only update if I needs updates?

comment:90 knutsp7 months ago

When using "Beta Tester" plugin with "Nightlies" I have always seen the the message that says there is a new version of WordPress available, even if I just updated, and no matter how many times I update. No way to make it say "You have the latest development version" or something like that.

The "Automatic Updater" plugin, however, seemed to detect that I have the latest available version and has only done upgrades when there is really a later one to update to. I have had the "deactivate emails" option unchecked, so I have been noticed when the actual updates have been performed.

Now, with core "Background Updates", it runs an upgrade twice daily, no matter if the latest is already installed, and sends an email about it.

"Automatic Updater" plugin is still active on my development test site, but it now (correctly) stays away from (minor) core updates.

comment:91 dd327 months ago

I got the update for 3.7-beta1-25639-20130929 twice (midnight and noon today). Isn't that supposed to check if I'm on THAT version and only update if I needs updates?

Now, with core "Background Updates", it runs an upgrade twice daily, no matter if the latest is already installed, and sends an email about it.

Yeah, We've got a limitiation of the API at present, we can fix it (We'll bump $wp_version for every nightly eventually) but for now, nightly users get twice daily updates - call it stress testing, we're running updates twice as often, twice the testing!
We'll get the API fixed up at some point, just not immediately while we've got more important tasks pending.

dd327 months ago

comment:93 johnbillion7 months ago

The WP_Automatic_upgrader class has a strange negatively named method, upgrader_disabled(), and an associated filter, auto_upgrader_disabled. To avoid double negatives (the auto upgrader is not disabled), this method and its filter should be changed to upgrader_enabled() and auto_upgrader_enabled respectively. Patch upcoming.

johnbillion7 months ago

comment:94 johnbillion7 months ago

  • Keywords has-patch added

22704.3.diff inverts the naming logic for upgrader_disabled() and auto_upgrader_disabled.

comment:95 follow-up: nacin7 months ago

In 25700:

Be as sure as possible that WordPress is not under version control when deciding if we should do automatic updates.

see #22704.

comment:96 in reply to: ↑ 95 ; follow-up: Denis-de-Bernardy7 months ago

Replying to nacin:

In 25700:

Be as sure as possible that WordPress is not under version control when deciding if we should do automatic updates.

see #22704.

This fails to detect WP when it's installed using e.g. Mark Jacquith's WP-Skeleton:

/index.php
/wp <-- abs path is here

At the very least, we should manage that setup, since rolling out a site with WP in the main folder hardly is best practice for customer sites.

In my own site's template, detecting the above would still fail since I've the same as above even further up, in /www alongside a /bin folder. I presume that is common enough as well to at least consider detecting it.

More generally and ideally, you'd scan for .git as low in the folder hierarchy as file permissions allow you to to make the same check work with more exotic use-cases.

comment:97 in reply to: ↑ 96 jeremyfelt7 months ago

Replying to Denis-de-Bernardy:

More generally and ideally, you'd scan for .git as low in the folder hierarchy as file permissions allow you to to make the same check work with more exotic use-cases.

This is what [25700] addresses. It walks up both ABSPATH and the passed context if necessary to look for any VCS directories.

Previously is_vcs_check() was returning false when the VCS files were 2 directories up.

comment:98 follow-up: GregLone6 months ago

Hi,

These are the warnings I get when I visit wp-admin/update-core.php:

Warning: is_dir(): open_basedir restriction in effect. File(/volume1/.svn) is not within the allowed path(s): ( ...foo... ) in /volume1/web/foobar/wp-admin/includes/class-wp-upgrader.php on line 1510 
Warning: is_dir(): open_basedir restriction in effect. File(/volume1/.git) is not within the allowed path(s): ( ...foo... ) in /volume1/web/foobar/wp-admin/includes/class-wp-upgrader.php on line 1510 
Warning: is_dir(): open_basedir restriction in effect. File(/volume1/.hg) is not within the allowed path(s): ( ...foo... ) in /volume1/web/foobar/wp-admin/includes/class-wp-upgrader.php on line 1510 
Warning: is_dir(): open_basedir restriction in effect. File(/volume1/.bzr) is not within the allowed path(s): ( ...foo... ) in /volume1/web/foobar/wp-admin/includes/class-wp-upgrader.php on line 1510

They come from the method WP_Automatic_Upgrader::is_vcs_checkout().

Of course, prefixing is_dir() with a @ will hide the problem.

Server: NAS Synology DS412+
php: 5.3.27
WP: 3.7-beta1-25639

Cheers.
Greg

dd326 months ago

comment:99 dd326 months ago

In 25750:

In the event that an Automatic Upgrade fails, send a failure status on the next API request to indicate that it failed, and if the rollback was successful.
See #22704

comment:100 dd326 months ago

In 25751:

Correct a variable typo in [25750]. See #22704

dd326 months ago

comment:101 dd326 months ago

In 25752:

After a upgrade fails, and we successfully rollback to a previous version, Don't send the version we rolled back to to the API, just that it was successful. See [25750]. See #22704

dd326 months ago

comment:102 nacin6 months ago

In 25755:

Auto updates: For follow-up API call when an update fails, send back the version attempted. see #22704.

comment:103 in reply to: ↑ 98 GaryJ6 months ago

Replying to GregLone:

These are the warnings I get when I visit wp-admin/update-core.php:

Confirming similar on 3.7-beta2 as well.

Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/vhosts/example.com/.svn) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/vhosts/.svn) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/.svn) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/.svn) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/vhosts/example.com/.git) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/vhosts/.git) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/.git) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/.git) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/vhosts/example.com/.hg) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/vhosts/.hg) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/.hg) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/.hg) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/vhosts/example.com/.bzr) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/vhosts/.bzr) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/www/.bzr) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510
Warning: is_dir() [function.is-dir]: open_basedir restriction in effect. File(/var/.bzr) is not within the allowed path(s): (/var/www/vhosts/example.com/httpdocs:/tmp) in /var/www/vhosts/example.com/httpdocs/wp-admin/includes/class-wp-upgrader.php on line 1510

I know my host has PHP safe_mode enabled :-/

comment:104 markoheijnen6 months ago

#25572 was marked as a duplicate.

nacin6 months ago

comment:105 nacin6 months ago

In 25763:

Refine error codes throughout the upgrader so we can better detect at what stage updates fail.

see #22704.

comment:106 follow-up: TobiasBg6 months ago

Minor typo: The last two error codes in [25763] have a double underscore in them.

comment:107 in reply to: ↑ 106 dd326 months ago

Replying to TobiasBg:

Minor typo: The last two error codes in [25763] have a double underscore in them.

That's deliberate as they're in the _copy_dir function.

comment:108 TobiasBg6 months ago

Yes, makes sense. Didn't see that in the plain changeset. Sorry for the false alarm!

comment:109 nacin6 months ago

In 25770:

Add explicit link to the installation being updated in debugging emails. see #22704.

comment:110 nacin6 months ago

In 25772:

More specific error codes in the core upgrader when copying language, plugin, and theme files. see #22704.

comment:111 nacin6 months ago

In 25777:

Auto updates: Only attempt a roll back when we've hit a critical error code.

Specifically, this means when we've failed in copying files.

see #22704.

comment:112 nacin6 months ago

In 25778:

Account for the possible failure of disk_free_space() in update_core().

see #22704, #25576.

comment:113 dd326 months ago

In 25779:

Remove PHP4 compat code from the ZipArchive unzip handler, and pass the failure reason into the WP_Error return.

See #22704

comment:114 nacin6 months ago

In 25780:

Parse absolute paths out of error data. see #22704.

dd326 months ago

comment:115 follow-up: kurtpayne6 months ago

  • Cc kurtpayne added

When using the 'auto_upgrade_plugin' filter, plugins are not being re-activated after upgrade. It looks like they are being explicitly deactivated:

add_filter('upgrader_pre_install', array($this, 'deactivate_plugin_before_upgrade'), 10, 2);
$this->run(....);
remove_filter('upgrader_pre_install', array($this, 'deactivate_plugin_before_upgrade'));

kurtpayne6 months ago

Reactivate plugins after updating

comment:116 nacin6 months ago

In 25781:

Automatic updates: Include error data in the follow-up API request.

props dd32.
see #22704.

kurtpayne6 months ago

Reactivate plugins after updating ... but only if they were active before updating

jorbin6 months ago

comment:117 follow-up: jorbin6 months ago

The above patch is a start on some messaging that we can use on wp-admin/update-core.php These are just words right now, the patch does nothing but give us a starting point for discussion of the language. Feedback (and pointing out of spelling/grammar mistakes) strongly welcome.

comment:118 nacin6 months ago

In 25782:

Automatic updates: An error code containing 'do_rollback' can be used to trigger a rollback.

This could enable the triggering of a rollback in update_core() in wp-admin/includes/update-core.php.

see #22704.

comment:119 in reply to: ↑ 117 kpdesign6 months ago

Replying to jorbin:

The above patch is a start on some messaging that we can use on wp-admin/update-core.php These are just words right now, the patch does nothing but give us a starting point for discussion of the language. Feedback (and pointing out of spelling/grammar mistakes) strongly welcome.

The only things I see are some missing commas and one spelling error (s/automattically/automatically) in the messages:

<strong>IMPORTANT:</strong> This install uses version control (SVN or Git), and thus will not receive auto updates. You are responsible for keeping this install updated.

(2nd message okay)

Your install of WordPress is up to date, and able to automatically update for all security releases. You are still responsible for keeping your plugins and themes updated.

Your install of WordPress is up to date, but we are unable to automatically update this site. For help with updates, visit the <a href="http://codex.wordpress.org/Updating_WordPress">Updating WordPress</a> Codex page.

Your install of WordPress is out of date, and we are unable to automatically update your site. For help with updates, visit the <a href="http://codex.wordpress.org/Updating_WordPress">Updating WordPress</a> Codex page.

And the @TODO: s/fuctions/functions.

Last edited 6 months ago by kpdesign (previous) (diff)

comment:120 nacin6 months ago

In 25783:

Hide auto updates from update-core.php directly in get_core_updates(). see #22704.

comment:121 nacin6 months ago

In 25784:

Hide language-specific warnings/labels on update-core.php when we are dealing with a point release partial build.

see #22704, #18200.

comment:122 dd326 months ago

In 25787:

Fix a misspelling of the filter we're checking for, s/fs_method/filesystem_method/.
My bad. See #22704, [25781].

comment:123 in reply to: ↑ 115 ; follow-up: dd326 months ago

Replying to kurtpayne:

When using the 'auto_upgrade_plugin' filter, plugins are not being re-activated after upgrade. It looks like they are being explicitly deactivated:

add_filter('upgrader_pre_install', array($this, 'deactivate_plugin_before_upgrade'), 10, 2);
$this->run(....);
remove_filter('upgrader_pre_install', array($this, 'deactivate_plugin_before_upgrade'));

So the cause here is that we're using singular updates in the automatic updater, which differ from their bulk upgrader counterparts in one specific case - Singular plugin updates uses a plugin sandboxing iframe to only re-activate the plugin if it doesn't fatal, bulk upgrades don't do this.

The problem is that that sandboxing mode requires the user's browser to hit an iframe which re-enables the plugin: http://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-upgrader-skins.php#L122

Honestly, Since we don't support plugin sandboxing correctly anymore, deactivating the plugin doesn't make 100% sense at all, and we should use Maintenance mode instead. If we can patch *just* the automatic updater to reactivate the plugin that would suffice for now and we can iterate on it later.

comment:124 nacin6 months ago

In 25793:

Use FS_CHMOD_FILE rather than an explicit 0644 in copy_dir() and _copy_dir().

This occurs when we can't copy a file. We chmod it and try again.

see #22704.

comment:125 nacin6 months ago

In 25794:

Core Updates: Verify that all files we're about to copy are actually writable, when using the direct transport.

Once we begin to copy core files, all bets are off. This allows us to fail early when we'll otherwise need to stop midway through due to permissions issues, which is a particularly bad problem when only some files have permissions issues.

see #22704.

comment:126 nacin6 months ago

In 25797:

Only add error data to the files_not_writable WP_Error if the install already has [25775] and [25796] applied.

see #22704.

comment:127 nacin6 months ago

In 25798:

Avoid our pre-flight writable checks if get_core_checksums() doesn't exist yet.

Thus, it will not work for 3.6 => 3.7, only 3.7+.

see #22704.

comment:128 nacin6 months ago

In 25799:

Remove accidental debug cruft in [25780]. see #22704.

comment:129 nacin6 months ago

In 25800:

Use correct variable. see #22704.

comment:130 bungeshea6 months ago

  • Cc info@… removed

nacin6 months ago

comment:131 GregLone6 months ago

  • Cc greg@… added

comment:132 GregLone6 months ago

  • Cc greg@… removed

comment:133 nacin6 months ago

In 25801:

Significantly simplify get_core_checksums(), as the caching and chunking was causing too much grief.

Make sure we only do our pre-flight is_writable check when the file exists.

see #18201. see #22704.

comment:134 nacin6 months ago

In 25805:

Avoid numerous potential PHP warnings when dealing with the pre-r25801 get_core_checksums() return value.

Warnings included current(), filestat(), and md5_file().

see #18201. see #22704.

comment:135 dd326 months ago

In 25806:

Language Packs: Many many fixes such as:

  • Add a "Update Translations" stand-alone button to the updates page
  • Shift Language feedback to before update process completion action links & limit the verbosity of output (name + success/errors)
  • Simplify/combine the language update descriptive string to only include a plugin/theme name
  • Properly handle cache clearing after language updates to prevent langs being repeditively updated
  • Display a "All items up to date" string when there's nothing to do
  • Reduce the 'Connection Information' from a <h2> to a <h3> to remove duplicate h2's and screen icons from update screens
  • Fix the Direct filesystem method not being used for Language updates because WP_LANG_DIR doesn't exist (check it's parent for writable instead)

See #18200, #22704

kurtpayne6 months ago

Reactivate plugins after updating, only during auto-upgrade though

comment:136 in reply to: ↑ 123 ; follow-up: kurtpayne6 months ago

Replying to dd32:

If we can patch *just* the automatic updater to reactivate the plugin that would suffice for now and we can iterate on it later.

22704-reactivate-plugins.3.patch.diff should address this.

comment:137 nacin6 months ago

In 25815:

Merge the should_auto_update() and can_auto_update() methods. see #22704.

comment:138 nacin6 months ago

In 25816:

Update the banner for update-core.php that tells users they are set up for security updates to happen in the background.

"This site is set up to install security updates of WordPress automatically. Cool!"

Checkmark is a placeholder; we'll tinker with this further today.

see #22704.

comment:139 dd326 months ago

In 25817:

When a plugin enables Background Plugin updates, don't deactivate the plugin during update as we require a browser to reactivate it afterwards. See #22704

comment:140 follow-up: nacin6 months ago

[25816] will only display if your $wp_version in wp-includes/version.php is set to 3.6.1. It looks like this:

http://cl.ly/image/0A2H3o0b2C3e/Screen%20Shot%202013-10-16%20at%203.43.10%20PM.png

Feedback welcome.

I don't intend to stick with the unicode checkmark. I was thinking a checkmark icon, maybe designed by empireoflight? We could remove the yellow update nag, then reduce the margins between this line of text and "You have the latest version of WordPress." It looked better in my head:

http://cl.ly/image/29431G3c183V/Screen%20Shot%202013-10-16%20at%203.46.59%20PM.png

(Yes, obviously, the whole updates experience needs a UI refresh...)

comment:141 toscho6 months ago

Do we need a check mark? There is no way to uncheck it. :)

comment:142 in reply to: ↑ 136 dd326 months ago

Replying to kurtpayne:

Replying to dd32:

If we can patch *just* the automatic updater to reactivate the plugin that would suffice for now and we can iterate on it later.

22704-reactivate-plugins.3.patch.diff should address this.

Missed this comment, but since Maintenance mode is already used, I just went with [25817] to not-break plugins.

comment:143 in reply to: ↑ 140 kpdesign6 months ago

Replying to nacin:

(Yes, obviously, the whole updates experience needs a UI refresh...)

I would suggest keeping the yellow update banner for now, as it's what users are accustomed to, and refresh the update UI in 3.8 with the admin refresh (MP6).

comment:144 melchoyce6 months ago

  • Cc melissachoyce@… added

comment:145 follow-up: nacin6 months ago

In 25820:

"Future security updates will be applied automatically." see #22704.

comment:146 nacin6 months ago

In 25823:

Make WP_Automatic_Upgrader a proper object that gets instantiated. Renames nearly all of its methods.

Also renames wp_auto_updates_maybe_update() to wp_maybe_auto_update().

see #22704.

comment:147 in reply to: ↑ 145 ; follow-up: rmccue6 months ago

Replying to nacin:

In 25820:

"Future security updates will be applied automatically." see #22704.

This seems a little dry compared to the old "Cool!" :(

comment:148 nacin6 months ago

In [25825]: Remove the old wp_auto_updates_maybe_update cron event. Schedule the new wp_maybe_auto_update event at 7 a.m. and 7 p.m. in the site's timezone.


Talked with jorbin and dd32 about this over dinner and such. If we're going to do 12-hour checks, we should try to avoid doing them mid-day as much as possible. 6pm is a little too early in the day, 8am is a little too late in the day. So here's some clever logic to try to schedule it at 7 and 7.

comment:149 in reply to: ↑ 147 nacin6 months ago

Replying to rmccue:

Replying to nacin:

In 25820:

"Future security updates will be applied automatically." see #22704.

This seems a little dry compared to the old "Cool!" :(

Yeah. So, for background, there was quite a long conversation in IRC.

I ended up talking with jenmylo for a long while about what we should do here. A yellow update didn't make sense, but nothing else did, really, either. In the end, simple made the most sense. You may note that [25820] simplifies things even further, by hiding a string and shortening another. She'll come by to explain some of our reasoning a bit more.

We can maybe be more flashy on the about page.

dd326 months ago

dd326 months ago

dd326 months ago

dd326 months ago

comment:150 dd326 months ago

In 25828:

For Background updates, ensure that only one update process runs at the same time by using the options table as a lock.
This prevents multiple cron spawns and/or long-running updates from causing multiple update processes to spin up.

This also fixes a case where the upgrader might kick in for ( ! is_main_network()
! is_main_site() ) in mulisite installs.

See #22704

nacin6 months ago

Changes upgrade() to update() in the automatic upgrader, which is now called the background updater. (It could be useful for more than just 'auto' updates.) The skin remains the Automatic_Upgrader. It kind of makes sense in my head.

comment:151 dd326 months ago

In 25831:

Silence PHP warnings from disk_free_space(). disk_free_space() will produce a warning in error conditions in addition to returning false, this includes a case where the bytes free is greater than PHP_INT_MAX (which is a error condition we don't need to check).
See #25576, #22704

comment:152 jenmylo6 months ago

  • Using background colors for alerts is an indication that something has just happened. We should not ever use an alert styling for a persistent message.
  • There was a lot of extra text. We pared it down for readability and to reduce confusion.
  • I know we are all really excited about automatic updates, but information isn't meant to be flashy. Only, "Oh no you're about to break something really horribly!" should ever be flashy in the dashboard.

dd326 months ago

comment:153 nacin6 months ago

In 25833:

Check if background core updates are supported on about.php.

see #25603, #22704.

comment:154 nacin6 months ago

In 25834:

Remove redundant code. see #25603, #22704, [25833].

comment:155 nacin6 months ago

"Cool!" came back in #25603, on the About page.

dd326 months ago

comment:156 nacin6 months ago

In 25835:

In automatic background updates, standardize on 'update'.

New, final filter names:

  • auto_update_{$type} (plugin, theme, core, language)
  • automatic_updates_is_vcs_checkout
  • automatic_updates_disabled

New class name is WP_Automatic_Updater. Method names include update() and should_update().

see #22704.

comment:157 follow-up: jeremyfelt6 months ago

Hate leaving this here as I haven't fully figured it out yet. That said...

Somehow after an automatic update upgraded the db version 25824, the cached option for db_version on my site was not cleared. This caused a loop of sorts when I went to wp-admin/.

wp-admin/admin.php detected that the cached option was different than expected and redirected to wp-admin/upgrade.php. Because upgrade.php defines WP_INSTALLING as true, get_option( 'db_version' ); performs a non cached query and sees that the upgrade was successful, which displays this message. Repeat.

I dug through briefly to see why the cache wasn't cleared when that option was updated during an automatic upgrade, but didn't find anything immediately. I'll pick it up and/or be prepared to eat crow in the morning. :)

Edit: Very likely this was due to a misfire in Memcache. So mostly ignore the above.

Last edited 6 months ago by jeremyfelt (previous) (diff)

comment:158 in reply to: ↑ 157 nacin6 months ago

Replying to jeremyfelt:

Hate leaving this here as I haven't fully figured it out yet. That said...
<snip>
Edit: Very likely this was due to a misfire in Memcache. So mostly ignore the above.

See also this IRC conversation.

comment:159 nacin6 months ago

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

In [25841]: Notify administrators of successful, failed, and pending core updates. Blocks future background updates after critical failures, but allow retries in certain situations. See #10787 for a lot more on that.


Okay! This ticket is DONE. Amazing work, everyone.

Please open new tickets for any future issues (or enhancements).

comment:160 nacin6 months ago

In 25851:

Commit [25823] should not have renamed AUTOMATIC_UPDATER_DISABLED to AUTOMATIC_UPDATES_DISABLED.

The proper constant is AUTOMATIC_UPDATER_DISABLED. Keep the filter in line with that too.

see #22704.

comment:161 follow-up: xiong.chiamiov6 months ago

So... how do I disable this? Define AUTOMATIC_UPDATER_DISABLED to something truthy?

I expected to see something in wp-config-sample.php, or a prominent mention on this ticket, or (even better) instructions on the release announcement. The only reason I can guess at AUTOMATIC_UPDATER_DISABLED is because the last commit here changes it.

comment:162 in reply to: ↑ 161 kpdesign6 months ago

Replying to xiong.chiamiov:

So... how do I disable this? Define AUTOMATIC_UPDATER_DISABLED to something truthy?

I expected to see something in wp-config-sample.php, or a prominent mention on this ticket, or (even better) instructions on the release announcement. The only reason I can guess at AUTOMATIC_UPDATER_DISABLED is because the last commit here changes it.

Please read @nacin's post on the Make/Core blog: http://make.wordpress.org/core/2013/10/25/the-definitive-guide-to-disabling-auto-updates-in-wordpress-3-7/

Note: See TracTickets for help on using tickets.