From 638b28399e0969967b527cb931b205a81f7b4049 Mon Sep 17 00:00:00 2001 From: Thomas Keller Date: Sun, 5 Jun 2011 02:37:47 +0200 Subject: [PATCH] Fix issues 695 and 697. Still, public keys aren't added to monotone databases for some weird reason, so issue 696 is still open. --- NEWS.mdtext | 7 + src/IDF/Plugin/SyncMonotone.php | 218 ++++++++++++++++++++++---------- 2 files changed, 157 insertions(+), 68 deletions(-) diff --git a/NEWS.mdtext b/NEWS.mdtext index 703336e..80830f3 100644 --- a/NEWS.mdtext +++ b/NEWS.mdtext @@ -20,9 +20,16 @@ - The usher section in the forge administration no longer displays a bogus server enty in case no monotone server is configured in the connected usher instance +- Prevent a timeout from popping up when Usher is restarted (issue 695) +- The SyncMonotone plugin now cleans up partial artifacts it created during the addition of + a new project or monotone key, in case an error popped up in the middle (issue 697) +- Better error detection and reporting in the SyncMonotone plugin + ATTENTION: This needs Pluf 46b7f251 or newer! ## Documentation +- The documentation on the setup of the monotone plugin has been improved. + ## Translations # InDefero 1.1.2 - Thu May 26 07:42:25 2011 UTC diff --git a/src/IDF/Plugin/SyncMonotone.php b/src/IDF/Plugin/SyncMonotone.php index 57f4035..19ea163 100644 --- a/src/IDF/Plugin/SyncMonotone.php +++ b/src/IDF/Plugin/SyncMonotone.php @@ -27,6 +27,29 @@ */ class IDF_Plugin_SyncMonotone { + private $old_err_rep = 0; + + public function __construct() + { + $this->old_err_rep = error_reporting(0); + } + + public function __destruct() + { + error_reporting($this->old_err_rep); + } + + private function _diagnoseProblem($msg) + { + $system_err = error_get_last(); + if (!empty($system_err)) { + $msg .= ': '.$system_err['message']; + } + + error_reporting($this->old_err_rep); + throw new IDF_Scm_Exception($msg); + } + /** * Entry point of the plugin. */ @@ -80,24 +103,33 @@ class IDF_Plugin_SyncMonotone return; } + // This guard cleans up on any kind of error, and here is how it works: + // As long as the guard is not committed, it keeps a reference to + // the given project. When the guard is destroyed and the reference + // is still present, it deletes the object. The deletion indirectly + // also calls into this plugin again, as the project delete hook + // will be called, that removes any changes we've made during the + // process. + $projectGuard = new IDF_Plugin_SyncMonotone_ModelGuard($project); + $projecttempl = Pluf::f('mtn_repositories', false); if ($projecttempl === false) { - throw new IDF_Scm_Exception( - __('"mtn_repositories" must be defined in your configuration file.') + $this->_diagnoseProblem( + __('"mtn_repositories" must be defined in your configuration file') ); } $usher_config = Pluf::f('mtn_usher_conf', false); if (!$usher_config || !is_writable($usher_config)) { - throw new IDF_Scm_Exception( - __('"mtn_usher_conf" does not exist or is not writable.') + $this->_diagnoseProblem( + __('"mtn_usher_conf" does not exist or is not writable') ); } $mtnpostpush = realpath(dirname(__FILE__) . '/../../../scripts/mtn-post-push'); if (!file_exists($mtnpostpush)) { - throw new IDF_Scm_Exception(sprintf( - __('Could not find mtn-post-push script "%s".'), $mtnpostpush + $this->_diagnoseProblem(sprintf( + __('Could not find mtn-post-push script "%s"'), $mtnpostpush )); } @@ -131,8 +163,8 @@ class IDF_Plugin_SyncMonotone } foreach ($confdir_contents as $content) { if (!file_exists($confdir.$content)) { - throw new IDF_Scm_Exception(sprintf( - __('The configuration file %s is missing.'), $content + $this->_diagnoseProblem(sprintf( + __('The configuration file "%s" is missing'), $content )); } } @@ -140,14 +172,15 @@ class IDF_Plugin_SyncMonotone $shortname = $project->shortname; $projectpath = sprintf($projecttempl, $shortname); if (file_exists($projectpath)) { - throw new IDF_Scm_Exception(sprintf( - __('The project path %s already exists.'), $projectpath + $this->_diagnoseProblem(sprintf( + __('The project path "%s" already exists'), $projectpath )); } - if (!mkdir($projectpath)) { - throw new IDF_Scm_Exception(sprintf( - __('The project path %s could not be created.'), $projectpath + if (!@mkdir($projectpath)) { + $this->_diagnoseProblem(sprintf( + __('The project path "%s" could not be created'), + $projectpath )); } @@ -182,9 +215,10 @@ class IDF_Plugin_SyncMonotone // $keydir = Pluf::f('tmp_folder').'/mtn-client-keys'; if (!file_exists($keydir)) { - if (!mkdir($keydir)) { - throw new IDF_Scm_Exception(sprintf( - __('The key directory %s could not be created.'), $keydir + if (!@mkdir($keydir)) { + $this->_diagnoseProblem(sprintf( + __('The key directory "%s" could not be created'), + $keydir )); } } @@ -201,7 +235,7 @@ class IDF_Plugin_SyncMonotone $parsed_keyinfo = IDF_Scm_Monotone_BasicIO::parse($keyinfo); } catch (Exception $e) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('Could not parse key information: %s'), $e->getMessage() )); } @@ -238,18 +272,20 @@ class IDF_Plugin_SyncMonotone foreach ($confdir_contents as $content) { $filepath = $projectpath.'/'.$content; if (substr($content, -1) == '/') { - if (!mkdir($filepath)) { - throw new IDF_Scm_Exception(sprintf( - __('Could not create configuration directory "%s"'), $filepath + if (!@mkdir($filepath)) { + $this->_diagnoseProblem(sprintf( + __('Could not create configuration directory "%s"'), + $filepath )); } continue; } if (substr($content, -3) != '.in') { - if (!symlink($confdir.$content, $filepath)) { + if (!@symlink($confdir.$content, $filepath)) { IDF_Scm_Exception(sprintf( - __('Could not create symlink "%s"'), $filepath + __('Could not create symlink for configuration file "%s"'), + $filepath )); } continue; @@ -264,9 +300,10 @@ class IDF_Plugin_SyncMonotone // remove the .in $filepath = substr($filepath, 0, -3); - if (file_put_contents($filepath, $filecontents, LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( - __('Could not write configuration file "%s"'), $filepath + if (@file_put_contents($filepath, $filecontents, LOCK_EX) === false) { + $this->_diagnoseProblem(sprintf( + __('Could not write configuration file "%s"'), + $filepath )); } } @@ -280,7 +317,7 @@ class IDF_Plugin_SyncMonotone $parsed_config = IDF_Scm_Monotone_BasicIO::parse($usher_rc); } catch (Exception $e) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('Could not parse usher configuration in "%s": %s'), $usher_config, $e->getMessage() )); @@ -291,7 +328,7 @@ class IDF_Plugin_SyncMonotone foreach ($stanzas as $stanza_line) { if ($stanza_line['key'] == 'server' && $stanza_line['values'][0] == $shortname) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('usher configuration already contains a server '. 'entry named "%s"'), $shortname @@ -315,9 +352,10 @@ class IDF_Plugin_SyncMonotone // FIXME: more sanity - what happens on failing writes? we do not // have a backup copy of usher.conf around... - if (file_put_contents($usher_config, $usher_rc, LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( - __('Could not write usher configuration file "%s"'), $usher_config + if (@file_put_contents($usher_config, $usher_rc, LOCK_EX) === false) { + $this->_diagnoseProblem(sprintf( + __('Could not write usher configuration file "%s"'), + $usher_config )); } @@ -325,6 +363,9 @@ class IDF_Plugin_SyncMonotone // step 6) reload usher to pick up the new configuration // IDF_Scm_Monotone_Usher::reload(); + + // commit the guard, so the newly created project is not deleted + $projectGuard->commit(); } /** @@ -361,9 +402,10 @@ class IDF_Plugin_SyncMonotone $write_permissions = implode("\n", $key_ids); $rcfile = $projectpath.'/write-permissions'; - if (file_put_contents($rcfile, $write_permissions, LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( - __('Could not write write-permissions file "%s"'), $rcfile + if (@file_put_contents($rcfile, $write_permissions, LOCK_EX) === false) { + $this->_diagnoseProblem(sprintf( + __('Could not write write-permissions file "%s"'), + $rcfile )); } @@ -382,11 +424,13 @@ class IDF_Plugin_SyncMonotone array('key' => 'allow', 'values' => array('*')), ); } + $read_permissions = IDF_Scm_Monotone_BasicIO::compile(array($stanza)); $rcfile = $projectpath.'/read-permissions'; - if (file_put_contents($rcfile, $read_permissions, LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( - __('Could not write read-permissions file "%s"'), $rcfile + if (@file_put_contents($rcfile, $read_permissions, LOCK_EX) === false) { + $this->_diagnoseProblem(sprintf( + __('Could not write read-permissions file "%s"'), + $rcfile )); } @@ -401,7 +445,7 @@ class IDF_Plugin_SyncMonotone $serverRestartRequired = false; if ($project->private && file_exists($projectfile) && is_link($projectfile)) { - if (!unlink($projectfile)) { + if (!@unlink($projectfile)) { IDF_Scm_Exception(sprintf( __('Could not remove symlink "%s"'), $projectfile )); @@ -409,8 +453,8 @@ class IDF_Plugin_SyncMonotone $serverRestartRequired = true; } else if (!$project->private && !file_exists($projectfile)) { - if (!symlink($templatefile, $projectfile)) { - throw new IDF_Scm_Exception(sprintf( + if (!@symlink($templatefile, $projectfile)) { + $this->_diagnoseProblem(sprintf( __('Could not create symlink "%s"'), $projectfile )); } @@ -422,6 +466,9 @@ class IDF_Plugin_SyncMonotone // seems to be ignored when the server should be started // again immediately afterwards IDF_Scm_Monotone_Usher::killServer($project->shortname); + // give usher some time to cool down, otherwise it might hang + // (see https://code.monotone.ca/p/contrib/issues/175/) + sleep(2); IDF_Scm_Monotone_Usher::startServer($project->shortname); } } @@ -443,8 +490,8 @@ class IDF_Plugin_SyncMonotone $usher_config = Pluf::f('mtn_usher_conf', false); if (!$usher_config || !is_writable($usher_config)) { - throw new IDF_Scm_Exception( - __('"mtn_usher_conf" does not exist or is not writable.') + $this->_diagnoseProblem( + __('"mtn_usher_conf" does not exist or is not writable') ); } @@ -453,16 +500,16 @@ class IDF_Plugin_SyncMonotone $projecttempl = Pluf::f('mtn_repositories', false); if ($projecttempl === false) { - throw new IDF_Scm_Exception( - __('"mtn_repositories" must be defined in your configuration file.') + $this->_diagnoseProblem( + __('"mtn_repositories" must be defined in your configuration file') ); } $projectpath = sprintf($projecttempl, $shortname); if (file_exists($projectpath)) { if (!self::_delete_recursive($projectpath)) { - throw new IDF_Scm_Exception(sprintf( - __('One or more paths underknees %s could not be deleted.'), $projectpath + $this->_diagnoseProblem(sprintf( + __('One or more paths underneath %s could not be deleted'), $projectpath )); } } @@ -473,8 +520,9 @@ class IDF_Plugin_SyncMonotone if ($keyname && $keyhash && file_exists($keydir .'/'. $keyname . '.' . $keyhash)) { if (!@unlink($keydir .'/'. $keyname . '.' . $keyhash)) { - throw new IDF_Scm_Exception(sprintf( - __('Could not delete client private key %s'), $keyname + $this->_diagnoseProblem(sprintf( + __('Could not delete client private key "%s"'), + $keyname )); } } @@ -485,7 +533,7 @@ class IDF_Plugin_SyncMonotone $parsed_config = IDF_Scm_Monotone_BasicIO::parse($usher_rc); } catch (Exception $e) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('Could not parse usher configuration in "%s": %s'), $usher_config, $e->getMessage() )); @@ -505,9 +553,10 @@ class IDF_Plugin_SyncMonotone // FIXME: more sanity - what happens on failing writes? we do not // have a backup copy of usher.conf around... - if (file_put_contents($usher_config, $usher_rc, LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( - __('Could not write usher configuration file "%s"'), $usher_config + if (@file_put_contents($usher_config, $usher_rc, LOCK_EX) === false) { + $this->_diagnoseProblem(sprintf( + __('Could not write usher configuration file "%s"'), + $usher_config )); } @@ -528,6 +577,8 @@ class IDF_Plugin_SyncMonotone return; } + $keyGuard = new IDF_Plugin_SyncMonotone_ModelGuard($key); + foreach (Pluf::factory('IDF_Project')->getList() as $project) { $conf = new IDF_Conf(); $conf->setProject($project); @@ -556,7 +607,7 @@ class IDF_Plugin_SyncMonotone $parsed_read_perms = IDF_Scm_Monotone_BasicIO::parse($read_perms); } catch (Exception $e) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('Could not parse read-permissions for project "%s": %s'), $shortname, $e->getMessage() )); @@ -598,10 +649,11 @@ class IDF_Plugin_SyncMonotone $read_perms = IDF_Scm_Monotone_BasicIO::compile($parsed_read_perms); - if (file_put_contents($projectpath.'/read-permissions', + if (@file_put_contents($projectpath.'/read-permissions', $read_perms, LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( - __('Could not write read-permissions for project "%s"'), $shortname + $this->_diagnoseProblem(sprintf( + __('Could not write read-permissions for project "%s"'), + $shortname )); } } @@ -611,9 +663,9 @@ class IDF_Plugin_SyncMonotone if (!in_array('*', $lines) && !in_array($mtn_key_id, $lines)) { $lines[] = $mtn_key_id; } - if (file_put_contents($projectpath.'/write-permissions', + if (@file_put_contents($projectpath.'/write-permissions', implode("\n", $lines) . "\n", LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('Could not write write-permissions file for project "%s"'), $shortname )); @@ -623,6 +675,8 @@ class IDF_Plugin_SyncMonotone $stdio = $mtn->getStdio(); $stdio->exec(array('put_public_key', $key->content)); } + + $key->commit(); } /** @@ -672,7 +726,7 @@ class IDF_Plugin_SyncMonotone $parsed_read_perms = IDF_Scm_Monotone_BasicIO::parse($read_perms); } catch (Exception $e) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('Could not parse read-permissions for project "%s": %s'), $shortname, $e->getMessage() )); @@ -693,10 +747,11 @@ class IDF_Plugin_SyncMonotone $read_perms = IDF_Scm_Monotone_BasicIO::compile($parsed_read_perms); - if (file_put_contents($projectpath.'/read-permissions', + if (@file_put_contents($projectpath.'/read-permissions', $read_perms, LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( - __('Could not write read-permissions for project "%s"'), $shortname + $this->_diagnoseProblem(sprintf( + __('Could not write read-permissions for project "%s"'), + $shortname )); } } @@ -711,9 +766,9 @@ class IDF_Plugin_SyncMonotone continue; } } - if (file_put_contents($projectpath.'/write-permissions', - implode("\n", $lines) . "\n", LOCK_EX) === false) { - throw new IDF_Scm_Exception(sprintf( + if (@file_put_contents($projectpath.'/write-permissions', + implode("\n", $lines) . "\n", LOCK_EX) === false) { + $this->_diagnoseProblem(sprintf( __('Could not write write-permissions file for project "%s"'), $shortname )); @@ -779,14 +834,14 @@ class IDF_Plugin_SyncMonotone { $projecttempl = Pluf::f('mtn_repositories', false); if ($projecttempl === false) { - throw new IDF_Scm_Exception( + $this->_diagnoseProblem( __('"mtn_repositories" must be defined in your configuration file.') ); } $projectpath = sprintf($projecttempl, $project->shortname); if (!file_exists($projectpath)) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('The project path %s does not exists.'), $projectpath )); } @@ -804,7 +859,7 @@ class IDF_Plugin_SyncMonotone $output = $return = null; exec($fullcmd, $output, $return); if ($return != 0) { - throw new IDF_Scm_Exception(sprintf( + $this->_diagnoseProblem(sprintf( __('The command "%s" could not be executed.'), $cmd )); } @@ -823,8 +878,35 @@ class IDF_Plugin_SyncMonotone foreach ($scan as $subpath) { $status |= self::_delete_recursive($subpath); } - $status |= rmdir($path); + $status |= @rmdir($path); return $status; } } } + +/** + * A simple helper class that deletes the model instance if + * it is not committed + */ +class IDF_Plugin_SyncMonotone_ModelGuard +{ + private $model; + + public function __construct(Pluf_Model $m) + { + $this->model = $m; + } + + public function __destruct() + { + if ($this->model == null) + return; + $this->model->delete(); + } + + public function commit() + { + $this->model = null; + } +} +