Make WordPress Core

Opened 21 months ago

Closed 20 months ago

Last modified 16 months ago

#56936 closed enhancement (fixed)

Remove bundled theme files from $_old_files

Reported by: sergeybiryukov's profile SergeyBiryukov Owned by: desrosj's profile desrosj
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch fixed-major
Focuses: Cc:

Description

Per @desrosj in comment:4:ticket:56934:

I'm wondering about including the deleted theme files here. Except 6.0, I don't see any additional wp-content/themes/* entries in the list.

As far as I know, themes are not included with update packages unless it's the first release being included (and they are not intentionally skipped with the CORE_UPGRADE_SKIP_NEW_BUNDLED constant).

...

In my testing, it seems that files within wp-content/themes/* are not flagged during Core upgrades and can probably be left out from this list.

Based on the above, it looks like bundled theme files should be removed from the list and a note should be added to clarify that for future updates.

Change History (13)

This ticket was mentioned in Slack in #core-upgrade-install by costdev. View the logs.


21 months ago

This ticket was mentioned in PR #3632 on WordPress/wordpress-develop by @costdev.


21 months ago
#2

  • Keywords has-patch added

This PR:

  • [x] Adds a note not to include bundled theme files in $_old_files.
  • [x] Removes bundled theme files entries.

Trac ticket: https://core.trac.wordpress.org/ticket/56936

#3 @costdev
21 months ago

  • Keywords needs-testing added

PR 3632 is ready for testing and review.

#4 @desrosj
21 months ago

  • Keywords commit added; needs-testing removed
  • Owner set to desrosj
  • Status changed from new to assigned

I've updated the Dry Run section of the handbook to note that theme files need not be noted in $_old_files.

Last edited 21 months ago by desrosj (previous) (diff)

#5 @desrosj
21 months ago

I did some more testing before making this update and wanted to detail what I found which confirms that these files should be excluded.

Including bundled theme files in the $_old_files list could actually break sites when Core is set to auto-update major versions and the theme with a file included in the list has auto-updates off.

The reason is that all files in the list are deleted when Core updates, regardless of where they are located. So if an earlier version of the theme depends on a file removed in a future version, and that file is listed in $_old_files, it will be deleted and sites with this theme active will potentially break.

This scenario has thankfully not yet happened because the files that were included in this list so far were only license files and SASS files.

#6 @desrosj
21 months ago

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

In 54849:

Upgrade/Install: Remove bundled theme files from $_old_files.

Because themes are updated independently of Core updates, any deleted files from bundled themes should not be included in the $_old_files list.

Any file included in this list is deleted on update, which could cause problems for sites with a given theme active if the removed files were required in earlier versions of that theme and that theme is not updated at the same time.

Props desrosj, costdev, SergeyBiryukov.
Fixes #56936.

#7 follow-up: @desrosj
21 months ago

  • Keywords fixed-major added; commit removed
  • Milestone changed from 6.2 to 6.1.2
  • Resolution fixed deleted
  • Status changed from closed to reopened

I think this is worth consideration for backporting to the 6.1 branch, but don't feel strongly about that.

Backporting to the 6.0 branch is unnecessary.

This ticket was mentioned in Slack in #core-upgrade-install by costdev. View the logs.


20 months ago

#10 in reply to: ↑ 7 @SergeyBiryukov
20 months ago

Replying to desrosj:

I think this is worth consideration for backporting to the 6.1 branch, but don't feel strongly about that.

I don't feel strongly either, but it's a straightforward fix and backporting would not hurt, so might as well do that :)

#11 @desrosj
20 months ago

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

In 54966:

Upgrade/Install: Remove bundled theme files from $_old_files.

Because themes are updated independently of Core updates, any deleted files from bundled themes should not be included in the $_old_files list.

Any file included in this list is deleted on update, which could cause problems for sites with a given theme active if the removed files were required in earlier versions of that theme and that theme is not updated at the same time.

Props desrosj, costdev, SergeyBiryukov.
Merges [54849] to the 6.1 branch.
Fixes #56936.

#12 @SergeyBiryukov
16 months ago

  • Milestone changed from 6.1.2 to 6.2

Moving back to 6.2, as there are no plans for 6.1.2 at this time.

This ticket was mentioned in Slack in #core by sergey. View the logs.


16 months ago

Note: See TracTickets for help on using tickets.