Using "update automatically" can destroy a theme in a Windows environment

[blurback]

If there are any locked files or files in use, Update Automatically destroys the theme.

I've experienced this three different ways:

1. The machine is really slow to delete the original theme dir, when the update is moved into the themes dir, an error occurs: "Could not create directory". The theme is now gone.
2. If there are any locked files in the outdated theme (e.g. SVN), all the files except the locked one will be deleted. Upgrader bails and you're left with a broken/empty theme.
3. If any of the files are in use, all files inside the theme will be deleted but the file system will throw an error. The updater bails with an empty theme directory.

Steps to duplicate:

1. On a Win hosted install, open the twentyten theme directory
2. Edit stylesheet, change version to 1.0
3. Open screenshot.png in paint
4. In the wp-admin, upgrade the theme
5. Sad times (screenshot attached)

Proposed solution is to try to rename the old theme first. Provides two benefits:

1. Have a backup of original theme if something breaks mid-way
2. Never left with a broken site

Note: all the more reasons why one should not use Windows :/ but I have no choice.

Screenshot of directory and output from upgrader

In most windows environments, renaming the folder moves the locks with it, which is what makes this work.. In other windows environments, locks are lost when moving the folder, which allows this to work, In some windows environments, locks prevent the parent directory from being renamed, which allows this to work.

All in all, this is probably the best option, an extension of this would be if there's an error during copying the new theme in place, to delete the partial and rename the old one back into place.

Note, the patch is incomplete as it uses rename directly, which we can't do on many systems, instead, $wp_filesystem->move() (Which is a single fs request compared to copy_dir() which makes many fs requests.)

Leaving as has-patch for now given it's minor details we can look closer at when implementing.

Replying to dd32:

if there's an error during copying the new theme in place, to delete the partial and rename the old one back into place.

Agree.

Note, the patch is incomplete as it uses rename directly, which we can't do on many systems, instead, $wp_filesystem->move()

I did first try $wp_filesystem->move() but encountered an error. If I remember correctly, the problem was that $wp_filesystem->move() first tries @rename and if that fails it tries to copy (which is not a recursive function and errors).

I used @rename directly because I took it from the first half of wp's move function. As for systems, I tested on a unix system and win system without issue.

I used @rename directly because I took it from the first half of wp's move function.

On some systems which use FTP (and there are windows systems which require that), rename() directly will cause it to fail every time.

It'll need some testing regardless, but I'm fairly confident a solution similar to the patch can be used. We've got a set of IIS test VM's that we'll hopefully be able to test this under.