Make WordPress Core

Opened 2 years ago

Closed 22 months ago

#45468 closed defect (bug) (fixed)

Language_Pack_Upgrader should delete existing language files

Reported by: ocean90 Owned by: ocean90
Milestone: 5.1 Priority: normal
Severity: normal Version: 5.0
Component: Upgrade/Install Keywords: has-patch
Focuses: Cc:

Description (last modified by swissspidy)

Related: #45103, #45467

Translations in JSON format are stored in a file named <locale>-<hash-of-file-path>.json. This means if the file is removed or renamed the hash will change too and the old file is no longer used. To avoid filling up the disk with unnecessary files the upgrader should remove all existing files first.

Attachments (2)

45468.diff (4.0 KB) - added by ocean90 22 months ago.
45468.2.diff (4.3 KB) - added by ocean90 22 months ago.

Download all attachments as: .zip

Change History (10)

#1 @pento
23 months ago

  • Milestone changed from 5.0.1 to 5.0.2

#2 @pento
23 months ago

  • Milestone changed from 5.0.2 to 5.0.3

#3 @audrasjb
22 months ago

  • Milestone changed from 5.0.3 to 5.1

5.0.3 is going to be released in a couple of weeks. We are currently sorting the remaining tickets in the milestone. It doesn't appear that ticket can be handled in the next couple of days (still needs a patch). Let's address it in 5.1 which is coming in February. Feel free to change/ask to change the milestone if you think the issue can be quickly resolved.

#4 @swissspidy
22 months ago

  • Description modified (diff)

22 months ago

#5 @ocean90
22 months ago

  • Keywords has-patch added; needs-patch removed

#6 @dd32
22 months ago

Hey @ocean90,

Just reviewing 45468.diff
$remote_destination is a path on the "remote" filesystem, as seen by FTP or others, as a result glob( $remote_destination ) won't work as expected.

You can just use glob( WP_LANG_DIR ) directly i instead to find out what files exist, but you'll have to iterate over the results to make them all relative to $remote_destination before acting upon them.

The other option is to use $wp_filesystem->dirlist( $remote_destination ) and filter through the listings directly there, but there's no real benefit in doing so (other than that's used elsewhere, for example in the base class clear_destination() ).

That's the only thing I noticed in reading the patch, but haven't applied it for testing.

22 months ago

#7 @ocean90
22 months ago

Thanks for the feedback @dd32! Added 45468.2.diff to use WP_LANG_DIR for the glob() call.

#8 @ocean90
22 months ago

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

In 44676:

I18N: Enable clear_destination in upgrader for updating/installing language packs.

Introduces Language_Pack_Upgrader::clear_destination() to clear existing translations before installing new translations. Ensures that unused translations in JSON format are cleaned up.

Props dd32, swissspidy, ocean90.
Fixes #45468.

Note: See TracTickets for help on using tickets.