Skip to content

Commit

Permalink
MDL-39442 Do not use native rename() when moving folders
Browse files Browse the repository at this point in the history
The native rename() function does not support moving folders
cross-device. See https://bugs.php.net/bug.php?id=54097 for details. So
instead of trying to move the whole tree, the new installer's method
moves files recursively one by one.

This is consistent with what mdeploy.php already does.
  • Loading branch information
mudrd8mz committed May 2, 2013
1 parent cf5a329 commit ed70c74
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 2 deletions.
46 changes: 46 additions & 0 deletions admin/tool/installaddon/classes/installer.php
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,52 @@ public function download_file($source, $target) {
}
}

/**
* Moves the given source into a new location recursively
*
* This is cross-device safe implementation to be used instead of the native rename() function.
* See https://bugs.php.net/bug.php?id=54097 for more details.
*
* @param string $source full path to the existing directory
* @param string $target full path to the new location of the directory
*/
public function move_directory($source, $target) {

if (file_exists($target)) {
throw new tool_installaddon_installer_exception('err_folder_already_exists', array('path' => $target));
}

if (is_dir($source)) {
$handle = opendir($source);
} else {
throw new tool_installaddon_installer_exception('err_no_such_folder', array('path' => $source));
}

make_writable_directory($target);

while ($filename = readdir($handle)) {
$sourcepath = $source.'/'.$filename;
$targetpath = $target.'/'.$filename;

if ($filename === '.' or $filename === '..') {
continue;
}

if (is_dir($sourcepath)) {
$this->move_directory($sourcepath, $targetpath);

} else {
rename($sourcepath, $targetpath);
}
}

closedir($handle);

rmdir($source);

clearstatcache();
}

//// End of external API ///////////////////////////////////////////////////

/**
Expand Down
2 changes: 1 addition & 1 deletion admin/tool/installaddon/deploy.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,6 @@
get_string('invaliddata', 'core_error'));
}

rename($zipcontentpath.'/'.$pluginname, $plugintypepath.'/'.$pluginname);
$installer->move_directory($zipcontentpath.'/'.$pluginname, $plugintypepath.'/'.$pluginname);
fulldelete($CFG->tempdir.'/tool_installaddon/'.$jobid);
redirect(new moodle_url('/admin'));
14 changes: 14 additions & 0 deletions admin/tool/installaddon/tests/installer_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,20 @@ public function test_decode_remote_request() {
)));
$this->assertSame(false, $installer->testable_decode_remote_request($request));
}

public function test_move_directory() {
$jobid = md5(rand().uniqid('test_', true));
$jobroot = make_temp_directory('tool_installaddon/'.$jobid);
$contentsdir = make_temp_directory('tool_installaddon/'.$jobid.'/contents/sub/folder');
file_put_contents($contentsdir.'/readme.txt', 'Hello world!');

$installer = tool_installaddon_installer::instance();
$installer->move_directory($jobroot.'/contents', $jobroot.'/moved');

$this->assertFalse(is_dir($jobroot.'/contents'));
$this->assertTrue(is_file($jobroot.'/moved/sub/folder/readme.txt'));
$this->assertSame('Hello world!', file_get_contents($jobroot.'/moved/sub/folder/readme.txt'));
}
}


Expand Down
2 changes: 1 addition & 1 deletion admin/tool/installaddon/version.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@
$plugin->component = 'tool_installaddon';
$plugin->version = 2013031400;
$plugin->requires = 2013031400;
$plugin->maturity = MATURITY_BETA;
$plugin->maturity = MATURITY_STABLE;

0 comments on commit ed70c74

Please sign in to comment.