WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

#11495 closed enhancement (fixed)

Automatic upgrade does not use custom directory structure

Reported by: sirzooro Owned by:
Milestone: 3.2 Priority: normal
Severity: normal Version: 2.9
Component: Upgrade/Install Keywords: needs-testing
Focuses: Cc:

Description

When you define WP_CONTENT_DIR, WP_LANG_DIR, WP_PLUGIN_DIR, and/or WPMU_PLUGIN_DIR to some non-standard values and try to automatically upgrade WordPress to new version, new files will end in default locations (wp-content, etc) instead of custom ones.

Attachments (1)

11495.is_callable.diff (916 bytes) - added by duck_ 3 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 dd325 years ago

Can you please give some examples?

Full path to WordPress, Values defined for custom folders, Where do the files end up? What files? All of them, or the temporary files, etc.

It is supposed to use the defines, as you can see by the usage of WP_CONTENT_DIR and WP_PLUGIN_DIR as noted here: http://core.trac.wordpress.org/browser/trunk/wp-admin/includes/class-wp-upgrader.php#L392

WPMU items are completely ignored however.. Thats something that needs updating in WordPress, Supporting the displayment of MU plugins, and alerting for updates for them..

comment:2 sirzooro5 years ago

Plugin upgrade works fine, problem is with core upgrade. Looks that upgrader unpacks downloaded archive only, and does not move files to correct locations.

My defines (added to wp-config.php):

define( 'WP_CONTENT_DIR', ABSPATH . 'my-content' );
define( 'WP_CONTENT_URL', '/my-content' );
define( 'WP_PLUGIN_DIR', ABSPATH . 'my-plugins' );
define( 'WP_PLUGIN_URL', '/my-plugins' );
define( 'PLUGINDIR', 'my-plugins' );

comment:3 hakre5 years ago

Which would be the correct location in your scenario for core files? I do not understand because the defines you posted aren't related to ABSPATH.

comment:4 hakre5 years ago

  • Keywords reporter-feedback added

comment:5 sirzooro5 years ago

  • Keywords reporter-feedback removed

I use following paths (windows box, but it will be the same on unix):

  • main WP dir (ABSPATH): C:\wordpress.local
  • custom wp-content dir: C:\wordpress.local\my-content
  • custom plugins dir: C:\wordpress.local\my-plugins

After I tried to upgrade core automatically, updated plugin and theme files ended within default C:\wordpress.local\wp-content dir. I would expect that they will be moved and overwrite old files in C:\wordpress.local\my-plugins and C:\wordpress.local\my-content\themes.

Translation files (I have upgraded to pl_PL version) also ended in C:\wordpress.local\my-content\languages\ instead of C:\wordpress.local\content\languages\.

comment:6 sirzooro5 years ago

One correction: translation files ended in C:\wordpress.local\wp-content\languages\ instead of C:\wordpress.local\my-content\languages\.

comment:7 follow-ups: dd325 years ago

Ah i see what you mean now. I guess, This means we'll need a rewrite process during the upgrade for the wp-content folders..

comment:8 in reply to: ↑ 7 westi5 years ago

  • Cc westi added

Replying to dd32:

Ah i see what you mean now. I guess, This means we'll need a rewrite process during the upgrade for the wp-content folders..

We probably do.

Although we need to be careful how we do this as some people may have moved the plugins directory away because they don't want the default plugins in it.

We do need to ensure that updated lang files end up in the right place though.

comment:9 dd325 years ago

Although we need to be careful how we do this as some people may have moved the plugins directory away because they don't want the default plugins in it.

This depends on how the Canonical plugins varies for this release IMO, I'm thinking that the default plugins are going to be replaced anyway, And they're not suited to being included in the core-upgrade process at all.

The same goes for themes, The default theme should be upgraded - If it exists, If not, it shouldnt be copied over at all perhaps.

Languages however, are a definite for a path rewrite, Perhaps WordPress should just deal with that branch instead for now?

comment:10 in reply to: ↑ 7 hakre5 years ago

Replying to dd32:

Ah i see what you mean now. I guess, This means we'll need a rewrite process during the upgrade for the wp-content folders..

Can you name file(s) / function(s) where this might be applicable sothat they can be reviewed if they reflect propper const values? I'd like to focus on the concrete bug before doing free-thinking about upcomming features.

comment:11 follow-up: dd325 years ago

I'd like to focus on the concrete bug before doing free-thinking about upcomming features.

The feature IS the bugfix.

WordPress uses ABSPATH for the core upgrade and assumes thats where files live. But as pointed out, Those who move their content direcotry, end up with files in their default location rather than their custom location. End of story. Therefor, The upgrade needs an extra step afterwards, to check if any custom directories are defined, and move/copy/overwrite them with the new files too.

comment:12 in reply to: ↑ 11 hakre5 years ago

Replying to dd32:

I'd like to focus on the concrete bug before doing free-thinking about upcomming features.

The feature IS the bugfix.

WordPress uses ABSPATH for the core upgrade and assumes thats where files live. But as pointed out, Those who move their content direcotry, end up with files in their default location rather than their custom location. End of story. Therefor, The upgrade needs an extra step afterwards, to check if any custom directories are defined, and move/copy/overwrite them with the new files too.

sounds complicated to me. why not let core upgrade use the correct location firsthand?

comment:13 dd325 years ago

sounds complicated to me. why not let core upgrade use the correct location firsthand?

Because that IS The correct location.

The WordPress package has to be unpacked to ABSPATH.

However, The package contains files whose path is set to wp-content/

In the event that the content directory is moved, One of 2 things are needed:

  1. Less Complicated: Copy the files to the new location afterwards
  2. More Complicated: Intrduce some Directory rewriting in the copying process to redirect wp-content/ to WP_CONTENT_DIR and wp-content/plugins to WP_PLUGIN_DIR, etc

The core upgrade IS using the correct location, Its just that the paths may vary within, and as such, more complicated approaches are needed.

comment:14 sirzooro5 years ago

Related: #11642. When it gets fixed and commited, you may need to add extra step - remove empty wp-content dir after upgrade when it is not used at all.

comment:15 nacin4 years ago

  • Milestone changed from 3.0 to Future Release
  • Type changed from defect (bug) to enhancement

As has been mentioned before, wp-content should probably be removed from the core upgrade all together, and copy over only what we need -- i.e., the current default theme if the directory does not exist.

We can then rely on the theme/plugin updaters for Akismet et al.

That would require some tweaks in the auto upgrade code, though not in this direction.

comment:16 dd323 years ago

  • Milestone changed from Future Release to 3.2

comment:17 dd323 years ago

(In [17576]) Be a party-pooper; No more Akismet Dancing upon upgrade; Respect custom WP_CONTENT_DIR for bundled plugins/theme installation; Respect custom WP_CONTENT_DIR/WP_LANG_DIR for Language files when upgrading; Standardise WP_Filesystem path method returns (They're trailing slash'd). Adds an exclusion list to copy_dir() as well as WP_Filesystem_Base::wp_lang_dir(). See #14484 See #11495

comment:18 dd323 years ago

(In [17577]) Typo in r17576, Twentyten isn't new to 3.2. See #14484 See #11495

comment:19 dd323 years ago

  • Keywords needs-testing added; needs-patch removed

[17576] should've largely fixed this, the only custom path not being respected here was WP_LANG_DIR. wp-content will not longer be created either.

I believe that fixes this ticket, however, I'm leaving this open for now pending more testing of the changes.

To test: You'll need to take a 3.2 install, modify it's version to indicate 3.1, set a custom WP_CONTENT_DIR and/or WP_LANG_DIR, and perform a automatic re-installation from Dashboard->Update, the result should be that wp-content doesn't exist, and that the language files have been updated

comment:20 duck_3 years ago

This is going to fatal error on upgrades from a version less than 3.2 to a package containing a language directory. This is because the upgrade will be using update-core.php from 3.2, but $wp_filesystem->wp_lang_dir() is not defined in 3.1 and below.

Might have to check is_callable and leave the problem for updates to 3.2, but fixed from 3.2 and beyond.

duck_3 years ago

comment:21 dd323 years ago

I noticed that last night unfortunately.

The alternative is to switch ->wp_lang_dir() to ->find_folder(WP_LANG_DIR); since it's just a wrapper.

The function will be useful for the language packs which is why I added it instead of just calling directly.

The other issue is that on <3.1 WP_LANG_DIR will be defined as wp-includes/languages unless wp-content/languages exists, which will cause problems for that code for upgrades to a zip which includes language files, yet wp-content/languages doesnt exist on the system - a very edge case

Whilst we're at it, might as well add a FTP_LANG_DIR override constant like the rest. (To skip path searching)

comment:22 dd323 years ago

(In [17578]) Back-compat for upgrades with WP_LANG_DIR. See #11495

comment:23 dd323 years ago

Going with is_callable would protect against the fatal, but given we're not copying wp-content, those who do NOT use a custom WP_CONTENT_DIR / WP_LANG_DIR would've missed out on language files during the update.

comment:24 dd323 years ago

(In [17579]) Add an extra FTP_LANG_DIR override constant to short-circuit WP_Filesystem_Base::find_folder(WP_LANG_DIR);. See #11495

comment:25 dd323 years ago

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

Closing as fixed; Please open a new ticket for any new issues.

Note: See TracTickets for help on using tickets.