[wp-trac] [WordPress Trac] #54166: Updating errors due to PHP timeouts

[WordPress Trac]

#54166: Updating errors due to PHP timeouts
-------------------------------------------------+-------------------------
 Reporter:  aristath                             |       Owner:
                                                 |  SergeyBiryukov
     Type:  defect (bug)                         |      Status:  reopened
 Priority:  normal                               |   Milestone:  5.9
Component:  Upgrade/Install                      |     Version:  trunk
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch early dev-feedback needs-  |     Focuses:
  testing                                        |
-------------------------------------------------+-------------------------

Comment (by SergeyBiryukov):

 Replying to [comment:24 afragen]:
 > > Yeah, now that I think of it, I also have some concerns with the
 function:
 > > * It does not use the `WP_Filesystem_*` methods, so the `rename()`
 call will only work if direct PHP file access is available.
 > > * The fallback to `copy_dir()` causes inconsistent behavior:
 > >  * If the `rename()` call worked, the source directory is successfully
 renamed and does not need any further actions.
 > >  * If the `rename()` call failed, the source directory is left behind
 and has to be cleaned separately.
 > >
 > > I wonder [51899] is still needed after [51902], or after getting to
 the bottom of the issue from comment:16.
 >
 > I think I have addressed the above concerns in a new
 [https://github.com/WordPress/wordpress-develop/pull/1747| PR#1747]

 Thanks for the PR! I think it addresses the first concern, but not the
 second. After the fallback to `copy_dir()`, I think the source directory
 should be removed. Currently, in that scenario, `move_dir()` is just an
 alias for `copy_dir()`, as it performs the copying part, but does not
 clean up the source directory like the `move_dir()` name suggests, unless
 I'm missing something.

 Addressing that would make the `clear_working` parameter of
 `WP_Upgrader::install_package()` a bit redundant, as the working directory
 would always be cleared regardless of its value. However, it is already
 redundant in case the `rename()` call worked. If `move_dir()` stays, the
 `clear_working` parameter would probably have to be deprecated.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/54166#comment:25>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform