From 9d29bb7e4bf242260d3c400e9de2e887a994765b Mon Sep 17 00:00:00 2001 From: Michael M Slusarz Date: Fri, 23 Jan 2009 10:44:01 -0700 Subject: [PATCH] Clean up some Vcs code. Clean up how we handle instantiation of the File object. Remove all PEAR_Error code - use Exceptions instead. Move valid revision determination into base class. --- framework/Vcs/lib/Horde/Vcs.php | 64 ++++++----- framework/Vcs/lib/Horde/Vcs/Cvs.php | 210 +++++++++++++----------------------- framework/Vcs/lib/Horde/Vcs/Git.php | 73 ++++++------- framework/Vcs/lib/Horde/Vcs/Rcs.php | 77 +++++-------- framework/Vcs/lib/Horde/Vcs/Svn.php | 141 +++++++----------------- 5 files changed, 203 insertions(+), 362 deletions(-) diff --git a/framework/Vcs/lib/Horde/Vcs.php b/framework/Vcs/lib/Horde/Vcs.php index cdc34e368..fdf3747cc 100644 --- a/framework/Vcs/lib/Horde/Vcs.php +++ b/framework/Vcs/lib/Horde/Vcs.php @@ -82,6 +82,11 @@ class Horde_Vcs protected $_branches = false; /** + * @var integer + */ + protected $_cacheVersion = 3; + + /** * Attempts to return a concrete Horde_Vcs instance based on $driver. * * @param mixed $driver The type of concrete Horde_Vcs subclass to return. @@ -99,7 +104,7 @@ class Horde_Vcs return new $class($params); } - return PEAR::raiseError($class . ' not found.'); + throw new Horde_Vcs_Exception($class . ' not found.'); } /** @@ -137,7 +142,7 @@ class Horde_Vcs } /** - * Return the source root for this repository, with no trailing / + * Return the source root for this repository, with no trailing /. * * @return string Source root for this repository. */ @@ -147,17 +152,16 @@ class Horde_Vcs } /** - * Validation function to ensure that a revision number is of the right + * Validation function to ensure that a revision string is of the right * form. * - * @param mixed $rev The purported revision number. + * @param mixed $rev The purported revision string. * - * @return boolean True if it is a revision number. + * @return boolean True if a valid revision string. */ public function isValidRevision($rev) { - $rev_ob = $this->getRevisionObject(); - return $rev_ob->valid($rev); + return true; } /** @@ -300,12 +304,33 @@ class Horde_Vcs * $opts: * 'cache' - (boolean) * 'quicklog' - (boolean) + * 'branch' - (string) */ public function getFileObject($filename, $opts = array()) { $class = 'Horde_Vcs_File_' . $this->_driver; - $vc_file = new $class($this, $filename, $opts); - return $vc_file->getFileObject(); + + /* The version of the cached data. Increment this whenever the + * internal storage format changes, such that we must + * invalidate prior cached data. */ + sort($opts); + $cacheId = implode('|', array($class, $this->sourceroot(), $filename, serialize($opts), $this->_cacheVersion)); + + $ctime = time() - filemtime($filename); + if (!empty($opts['cache']) && + $opts['cache']->exists($cacheId, $ctime)) { + $fileOb = unserialize($opts['cache']->get($cacheId, $ctime)); + $fileOb->setRepository($this); + } else { + $fileOb = new $class($this, $filename, $opts); + $fileOb->applySort(self::SORT_AGE); + + if (!empty($opts['cache'])) { + $opts['cache']->set($cacheId, serialize($fileOb)); + } + } + + return $fileOb; } public function getAnnotateObject($filename) @@ -758,10 +783,6 @@ class Horde_Vcs_File public $branches = array(); public $revob; - public function getFileObject() - { - } - /** * TODO */ @@ -808,7 +829,7 @@ class Horde_Vcs_File public function queryRevision() { if (!isset($this->revs[0])) { - return PEAR::raiseError('No revisions'); + throw new Horde_Vcs_Exception('No revisions'); } return $this->revs[0]; } @@ -846,7 +867,7 @@ class Horde_Vcs_File public function queryLastLog() { if (!isset($this->revs[0]) || !isset($this->logs[$this->revs[0]])) { - return PEAR::raiseError('No revisions'); + throw new Horde_Vcs_Exception('No revisions'); } return $this->logs[$this->revs[0]]; } @@ -1074,19 +1095,6 @@ abstract class Horde_Vcs_Patchset class Horde_Vcs_Revision { /** - * Validation function to ensure that a revision string is of the right - * form. - * - * @param mixed $rev The purported revision string. - * - * @return boolean True if it is a valid revision string. - */ - public function valid($rev) - { - return true; - } - - /** * Given a revision string, remove a given number of portions from * it. For example, if we remove 2 portions of 1.2.3.4, we are * left with 1.2. diff --git a/framework/Vcs/lib/Horde/Vcs/Cvs.php b/framework/Vcs/lib/Horde/Vcs/Cvs.php index 81e17940f..752fedc01 100644 --- a/framework/Vcs/lib/Horde/Vcs/Cvs.php +++ b/framework/Vcs/lib/Horde/Vcs/Cvs.php @@ -60,7 +60,15 @@ class Horde_Vcs_Cvs extends Horde_Vcs_Rcs if (substr($filename, 0, 1) != '/') { $filename = '/' . $filename; } - return parent::getFileObject($this->sourceroot() . $filename, $opts); + + $filename = $this->sourceroot() . $filename; + + /* Assume file is in the Attic if it doesn't exist. */ + $fname = $filename . ',v'; + if (!@is_file($fname)) { + $fname = dirname($filename) . '/Attic/' . basename($filename) . ',v'; + } + return parent::getFileObject($fname, $opts); } /** @@ -113,9 +121,7 @@ class Horde_Vcs_Annotate_Cvs extends Horde_Vcs_Annotate */ public function doAnnotate($rev) { - if (is_a($this->_file, 'PEAR_Error') || - is_a($this->_rep, 'PEAR_Error') || - !$this->_rep->isValidRevision($rev)) { + if (!$this->_rep->isValidRevision($rev)) { return false; } @@ -166,7 +172,7 @@ class Horde_Vcs_Annotate_Cvs extends Horde_Vcs_Annotate } if (!$line) { - return PEAR::raiseError('Unable to annotate; server said: ' . $line); + throw new Horde_Vcs_Exception('Unable to annotate; server said: ' . $line); } $lineno = 1; @@ -207,25 +213,16 @@ class Horde_Vcs_Checkout_Cvs extends Horde_Vcs_Checkout * to checkout. * @param string $rev Revision number to check out. * - * @return resource|object Either a PEAR_Error object, or a stream - * pointer to the head of the checkout + * @return resource A stream pointer to the head of the checkout. */ public function get($rep, $fullname, $rev) { if (!$rep->isValidRevision($rev)) { - return PEAR::raiseError('Invalid revision number'); - } - - if (VC_WINDOWS) { - $mode = 'rb'; - $q_name = '"' . escapeshellcmd(str_replace('\\', '/', $fullname)) . '"'; - } else { - $mode = 'r'; - $q_name = escapeshellarg($fullname); + throw new Horde_Vcs_Exception('Invalid revision number'); } - if (!($RCS = popen($rep->getPath('co') . " -p$rev $q_name 2>&1", $mode))) { - return PEAR::raiseError('Couldn\'t perform checkout of the requested file'); + if (!($RCS = popen($rep->getPath('co') . " -p$rev " . escapeshellarg($fullname) . " 2>&1", VC_WINDOWS ? 'rb' : 'r'))) { + throw new Horde_Vcs_Exception('Couldn\'t perform checkout of the requested file'); } /* First line from co should be of the form : @@ -235,7 +232,7 @@ class Horde_Vcs_Checkout_Cvs extends Horde_Vcs_Checkout $co = fgets($RCS, 1024); if (!preg_match('/^([\S ]+),v\s+-->\s+st(andar)?d ?out(put)?\s*$/', $co, $regs) || ($regs[1] != $fullname)) { - return PEAR::raiseError('Unexpected output from checkout: ' . $co); + throw new Horde_Vcs_Exception('Unexpected output from checkout: ' . $co); } /* Next line from co is of the form: @@ -278,11 +275,6 @@ class Horde_Vcs_Diff_Cvs extends Horde_Vcs_Diff public function get($rep, $file, $rev1, $rev2, $type = 'context', $num = 3, $ws = true) { - /* Make sure that the file parameter is valid. */ - if (is_a($file, 'PEAR_Error')) { - return false; - } - /* Check that the revision numbers are valid. */ $rev1 = $rep->isValidRevision($rev1) ? $rev1 : '1.1'; $rev2 = $rep->isValidRevision($rev1) ? $rev2 : '1.1'; @@ -362,19 +354,19 @@ class Horde_Vcs_Directory_Cvs extends Horde_Vcs_Directory * retrieve a list of all the objects in there. It then populates * the file/directory stack and makes it available for retrieval. * - * @return boolean|object PEAR_Error object on an error, true on success. + * @return boolean True on success. */ public function browseDir($cache = null, $quicklog = true, $showattic = false) { /* Make sure we are trying to list a directory */ if (!@is_dir($this->_dirName)) { - return PEAR::raiseError('Unable to find directory ' . $this->_dirName); + throw new Horde_Vcs_Exception('Unable to find directory ' . $this->_dirName); } /* Open the directory for reading its contents */ if (!($DIR = @opendir($this->_dirName))) { - return PEAR::raiseError(empty($php_errormsg) ? 'Permission denied' : $php_errormsg); + throw new Horde_Vcs_Exception(empty($php_errormsg) ? 'Permission denied' : $php_errormsg); } /* Create two arrays - one of all the files, and the other of @@ -392,12 +384,7 @@ class Horde_Vcs_Directory_Cvs extends Horde_Vcs_Directory } } elseif (@is_file($path) && (substr($name, -2) == ',v')) { /* Spawn a new file object to represent this file. */ - $fl = $this->_rep->getFileObject(substr($path, strlen($this->_rep->sourceroot()), -2), array('cache' => $cache, 'quicklog' => $quicklog)); - if (is_a($fl, 'PEAR_Error')) { - return $fl; - } else { - $this->_files[] = $fl; - } + $this->_files[] = $this->_rep->getFileObject(substr($path, strlen($this->_rep->sourceroot()), -2), array('cache' => $cache, 'quicklog' => $quicklog)); } } @@ -406,11 +393,11 @@ class Horde_Vcs_Directory_Cvs extends Horde_Vcs_Directory /* If we want to merge the attic, add it in here. */ if ($showattic) { - $atticDir = new Horde_Vcs_Directory_Cvs($this->_rep, $this->_moduleName . '/Attic', $this); - if (!is_a($atticDir->browseDir($cache, $quicklog), 'PEAR_Error')) { + try { + $atticDir = new Horde_Vcs_Directory_Cvs($this->_rep, $this->_moduleName . '/Attic', $this); $this->_atticFiles = $atticDir->queryFileList(); $this->_mergedFiles = array_merge($this->_files, $this->_atticFiles); - } + } catch (Horde_Vcs_Exception $e) {} } return true; @@ -446,109 +433,26 @@ class Horde_Vcs_File_Cvs extends Horde_Vcs_File $this->name = basename($fl); $this->dir = dirname($fl); $this->filename = $fl; - $this->cache = empty($opts['cache']) ? null : $opts['cache']; $this->quicklog = !empty($opts['quicklog']); if (!empty($opts['branch'])) { $this->_branch = $opts['branch']; } - } - - public function &getFileObject() - { - /* Assume file is in the Attic if it doesn't exist. */ - $filename = $this->filename . ',v'; - if (!@is_file($filename)) { - $filename = dirname($this->filename) . '/Attic/' . basename($this->filename) . ',v'; - } - - /* The version of the cached data. Increment this whenever the - * internal storage format changes, such that we must - * invalidate prior cached data. */ - $cacheVersion = 2; - $cacheId = $this->rep->sourceroot() . '_n' . $filename . '_f' . (int)$this->quicklog . '_v' . $cacheVersion; - - $ctime = time() - filemtime($filename); - if ($this->cache && - $this->cache->exists($cacheId, $ctime)) { - $fileOb = unserialize($this->cache->get($cacheId, $ctime)); - $fileOb->setRepository($this->rep); - } else { - if (is_a(($result = $this->getBrowseInfo()), 'PEAR_Error')) { - return $result; - } - $this->applySort(Horde_Vcs::SORT_AGE); - - if ($this->cache) { - $this->cache->set($cacheId, serialize($this)); - } - - $fileOb = $this; - } - - return $fileOb; - } - - /** - * If this file is present in an Attic directory, this indicates it. - * - * @return boolean True if file is in the Attic, and false otherwise - */ - public function isDeleted() - { - return (substr($this->dir, -5) == 'Attic'); - } - - /** - * Returns name of the current file without the repository - * extensions (usually ,v) - * - * @return string Filename without repository extension - */ - public function queryName() - { - return preg_replace('/,v$/', '', $this->name); - } - - public function queryPreviousRevision($rev) - { - $ob = $this->rep->getRevisionObject(); - return $ob->prev($rev); - } - - /** - * Return the HEAD (most recent) revision number for this file. - * - * @return string HEAD revision number - */ - public function queryHead() - { - return $this->head; - } - /** - * Populate the object with information about the revisions logs and dates - * of the file. - * - * @return boolean|object PEAR_Error object on error, or true on success - */ - public function getBrowseInfo() - { /* Check that we are actually in the filesystem. */ - $file = $this->queryFullPath() . ',v'; + $file = $this->queryFullPath(); if (!is_file($file)) { - return PEAR::raiseError('File Not Found: ' . $file); + throw new Horde_Vcs_Exception('File Not Found: ' . $file); } /* Call the RCS rlog command to retrieve the file * information. */ $flag = $this->quicklog ? ' -r ' : ' '; - $q_file = VC_WINDOWS ? '"' . escapeshellcmd($file) . '"' : escapeshellarg($file); - $cmd = $this->rep->getPath('rlog') . $flag . $q_file; + $cmd = $this->rep->getPath('rlog') . $flag . escapeshellarg($file); exec($cmd, $return_array, $retval); if ($retval) { - return PEAR::raiseError('Failed to spawn rlog to retrieve file log information for ' . $file); + throw new Horde_Vcs_Exception('Failed to spawn rlog to retrieve file log information for ' . $file); } $accum = $revsym = $symrev = array(); @@ -613,7 +517,46 @@ class Horde_Vcs_File_Cvs extends Horde_Vcs_File } } - return true; + } + + /** + * If this file is present in an Attic directory, this indicates it. + * + * @return boolean True if file is in the Attic, and false otherwise + */ + public function isDeleted() + { + return (substr($this->dir, -5) == 'Attic'); + } + + /** + * Returns name of the current file without the repository + * extensions (usually ,v) + * + * @return string Filename without repository extension + */ + public function queryName() + { + return preg_replace('/,v$/', '', $this->name); + } + + /** + * TODO + */ + public function queryPreviousRevision($rev) + { + $ob = $this->rep->getRevisionObject(); + return $ob->prev($rev); + } + + /** + * Return the HEAD (most recent) revision number for this file. + * + * @return string HEAD revision number + */ + public function queryHead() + { + return $this->head; } /** @@ -647,12 +590,8 @@ class Horde_Vcs_File_Cvs extends Horde_Vcs_File public function toBranch($rev) { /* Check if we have a valid revision number */ - $rev_ob = $this->rep->getRevisionObject(); - if (!$rev_ob->valid($rev)) { - return false; - } - - if (($end = strrpos($rev, '.')) === false) { + if (!$this->rep->isValidRevision($rev) || + (($end = strrpos($rev, '.')) === false)) { return false; } @@ -836,28 +775,25 @@ class Horde_Vcs_Patchset_Cvs extends Horde_Vcs_Patchset * Populate the object with information about the patchsets that * this file is involved in. * - * @return boolean|object PEAR_Error object on error, or true on success. + * @return boolean True on success. */ public function getPatchsets() { /* Check that we are actually in the filesystem. */ if (!is_file($this->getFullPath() . ',v')) { - return PEAR::raiseError('File Not Found'); + throw new Horde_Vcs_Exception('File Not Found'); } /* Call cvsps to retrieve all patchsets for this file. */ - $q_root = $this->_rep->sourceroot(); - $q_root = VC_WINDOWS ? '"' . escapeshellcmd($q_root) . '"' : escapeshellarg($q_root); - $cvsps_home = $this->_rep->getPath('cvsps_home'); $HOME = !empty($cvsps_home) ? 'HOME=' . $cvsps_home . ' ' : ''; - $cmd = $HOME . $this->_rep->getPath('cvsps') . ' -u --cvs-direct --root ' . $q_root . ' -f ' . escapeshellarg($this->_name) . ' ' . escapeshellarg($this->_dir); + $cmd = $HOME . $this->_rep->getPath('cvsps') . ' -u --cvs-direct --root ' . escapeshellarg($this->_rep->sourceroot) . ' -f ' . escapeshellarg($this->_name) . ' ' . escapeshellarg($this->_dir); exec($cmd, $return_array, $retval); if ($retval) { - return PEAR::raiseError('Failed to spawn cvsps to retrieve patchset information'); + throw new Horde_Vcs_Exception('Failed to spawn cvsps to retrieve patchset information'); } $this->_patchsets = array(); diff --git a/framework/Vcs/lib/Horde/Vcs/Git.php b/framework/Vcs/lib/Horde/Vcs/Git.php index b86b4cafd..aeef64f26 100644 --- a/framework/Vcs/lib/Horde/Vcs/Git.php +++ b/framework/Vcs/lib/Horde/Vcs/Git.php @@ -36,9 +36,18 @@ class Horde_Vcs_Git extends Horde_Vcs /** * TODO */ + public function isValidRevision($rev) + { + return $rev && preg_match('/^[a-f0-9]+$/i', $rev); + } + + /** + * TODO + */ public function isFile($where) { - $command = $this->_rep->getCommand() . ' ls-tree master ' . escapeshellarg($where) . ' 2>&1'; + $where = str_replace($this->sourceroot() . '/', '', $where); + $command = $this->getCommand() . ' ls-tree master ' . escapeshellarg($where) . ' 2>&1'; $entry = array(); exec($command, $entry); $data = explode(' ', $entry[0]); @@ -70,14 +79,6 @@ class Horde_Vcs_Git extends Horde_Vcs */ class Horde_Vcs_Annotate_Git extends Horde_Vcs_Annotate { - public function __construct($rep, $file) - { - if (is_a($file, 'PEAR_Error')) { - throw new Horde_Vcs_Exception($file->getMessage()); - } - parent::__construct($rep, $file); - } - /** * TODO */ @@ -154,8 +155,7 @@ class Horde_Vcs_Checkout_Git extends Horde_Vcs_Checkout * to checkout * @param string $rev Revision number to check out * - * @return resource|object Either a PEAR_Error object, or a stream - * pointer to the head of the checkout. + * @return resource A stream pointer to the head of the checkout. */ function get($rep, $file, $rev) { @@ -207,11 +207,6 @@ class Horde_Vcs_Diff_Git extends Horde_Vcs_Diff public function get($rep, $file, $rev1, $rev2, $type = 'context', $num = 3, $ws = true) { - /* Make sure that the file parameter is valid */ - if (is_a($file, 'PEAR_Error')) { - return false; - } - /* Check that the revision numbers are valid */ $rev1 = $rep->isValidRevision($rev1) ? $rev1 : 0; $rev2 = $rep->isValidRevision($rev1) ? $rev2 : 0; @@ -281,7 +276,7 @@ class Horde_Vcs_Directory_Git extends Horde_Vcs_Directory * retrieve a list of all the objects in there. It then populates * the file/directory stack and makes it available for retrieval. * - * @return PEAR_Error object on an error, 1 on success. + * @return integer 1 on success. */ public function browseDir($cache = null, $quicklog = true, $showattic = false) @@ -339,6 +334,7 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File { /* @TODO */ protected $_branch; + public $cache; /** * Create a repository file object, and give it information about @@ -359,13 +355,7 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File if (!empty($opts['branch'])) { $this->_branch = $opts['branch']; } - } - /** - * TODO - */ - public function getFileObject() - { // Get the list of revisions that touch this path $this->revs = $this->_getRevList($this->_branch); @@ -387,22 +377,6 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File $line = explode(' ', trim($val), 2); $this->branches[substr($line[1], strrpos($line[1], '/') + 1)] = $line[0]; } - - return $this; - } - - protected function _getRevList($branch) - { - $cmd = $this->rep->getCommand() . ' rev-list ' . (empty($branch) ? '--branches' : $branch) . ' -- ' . escapeshellarg($this->fullname) . ' 2>&1'; - - $revisions = shell_exec($cmd); - if (substr($revisions, 5) == 'fatal') { - throw new Horde_Vcs_Exception($revisions); - } elseif (!strlen($revisions)) { - throw new Horde_Vcs_Exception('No revisions found'); - } - - return explode("\n", trim($revisions)); } /** @@ -443,6 +417,23 @@ class Horde_Vcs_File_Git extends Horde_Vcs_File return $revs; } + /** + * TODO + */ + protected function _getRevList($branch) + { + $cmd = $this->rep->getCommand() . ' rev-list ' . (empty($branch) ? '--branches' : $branch) . ' -- ' . escapeshellarg($this->fullname) . ' 2>&1'; + + $revisions = shell_exec($cmd); + if (substr($revisions, 5) == 'fatal') { + throw new Horde_Vcs_Exception($revisions); + } elseif (!strlen($revisions)) { + throw new Horde_Vcs_Exception('No revisions found'); + } + + return explode("\n", trim($revisions)); + } + } /** @@ -673,10 +664,6 @@ class Horde_Vcs_Revision_Git extends Horde_Vcs_Revision * * @return boolean True if it is a revision number. */ - public function valid($rev) - { - return preg_match('/^[a-f0-9]+$/i', $rev); - } /** * Returns an abbreviated form of the revision, for display. diff --git a/framework/Vcs/lib/Horde/Vcs/Rcs.php b/framework/Vcs/lib/Horde/Vcs/Rcs.php index 8ee3e5da9..4a36283d1 100644 --- a/framework/Vcs/lib/Horde/Vcs/Rcs.php +++ b/framework/Vcs/lib/Horde/Vcs/Rcs.php @@ -11,6 +11,14 @@ class Horde_Vcs_Rcs extends Horde_Vcs { /** + * TODO + */ + public function isValidRevision($rev) + { + return $rev && preg_match('/^[\d\.]+$/', $rev); + } + + /** * Checks an RCS file in with a specified change log. * * @param string $filepath Location of file to check in. @@ -18,8 +26,7 @@ class Horde_Vcs_Rcs extends Horde_Vcs * @param string $user The user name to use for the check in. * @param boolean $newBinary Does the change involve binary data? * - * @return string|object The new revision number on success, or a - * PEAR_Error object on failure. + * @return string The new revision number on success. */ public function ci($filepath, $message, $user = null, $newBinary = false) { @@ -29,10 +36,8 @@ class Horde_Vcs_Rcs extends Horde_Vcs putenv('LOGNAME=guest'); } - $Q = VC_WINDOWS ? '"' : "'" ; - - $ci_cmd = $this->getPath('ci') . ' ' . $Q . $filepath . $Q.' 2>&1'; - $rcs_cmd = $this->getPath('rcs') . ' -i -kb ' . $Q . $filepath . $Q.' 2>&1'; + $ci_cmd = $this->getPath('ci') . ' ' . escapeshellarg($filepath) . ' 2>&1'; + $rcs_cmd = $this->getPath('rcs') . ' -i -kb ' . escapeshellarg($filepath) . ' 2>&1'; $output = ''; $message_lines = explode("\n", $message); @@ -63,14 +68,14 @@ class Horde_Vcs_Rcs extends Horde_Vcs fclose($pipes[1]); proc_close($process); } else { - return PEAR::raiseError('Failed to open pipe in ci()'); + throw new Horde_Vcs_Exception('Failed to open pipe in ci()'); } if ($newBinary) { exec($ci_cmd . ' 2>&1', $return_array, $retval); if ($retval) { - return PEAR::raiseError("Unable to spawn ci on $filepath from ci()"); + throw new Horde_Vcs_Exception("Unable to spawn ci on $filepath from ci()"); } else { foreach ($return_array as $line) { $output .= $line; @@ -95,9 +100,9 @@ class Horde_Vcs_Rcs extends Horde_Vcs unlock($filepath); $temp_pos = strpos($output, 'file is unchanged'); if ($temp_pos !== false) { - return PEAR::raiseError('Check-in Failure: ' . basename($filepath) . ' has not been modified'); + throw new Horde_Vcs_Exception('Check-in Failure: ' . basename($filepath) . ' has not been modified'); } else { - return PEAR::raiseError("Failed to checkin $filepath, $ci_cmd, $output"); + throw new Horde_Vcs_Exception("Failed to checkin $filepath, $ci_cmd, $output"); } } } @@ -108,21 +113,16 @@ class Horde_Vcs_Rcs extends Horde_Vcs * @param string $filepath Location of file. * @param string &$locked_by Returns the username holding the lock. * - * @return boolean|object True on success, or a PEAR_Error on failure. + * @return boolean True on success. */ public function isLocked($filepath, &$locked_by) { - $rlog_cmd = $this->getPath('rlog'); - $rlog_flag = ' -L '; - - $Q = VC_WINDOWS ? '"' : "'"; - - $cmd = $rlog_cmd . $rlog_flag . $Q . $filepath . $Q; + $cmd = $this->getPath('rlog') . ' -L ' . escapeshellarg($filepath); - exec($cmd.' 2>&1', $return_array, $retval); + exec($cmd . ' 2>&1', $return_array, $retval); if ($retval) { - return PEAR::raiseError("Unable to spawn rlog on $filepath from isLocked()"); + throw new Horde_Vcs_Exception("Unable to spawn rlog on $filepath from isLocked()"); } else { $output = ''; @@ -140,7 +140,7 @@ class Horde_Vcs_Rcs extends Horde_Vcs } elseif (strlen($output) == 0) { return false; } else { - return PEAR::raiseError('Failure running rlog in isLocked()'); + throw new Horde_Vcs_Exception('Failure running rlog in isLocked()'); } } } @@ -151,7 +151,7 @@ class Horde_Vcs_Rcs extends Horde_Vcs * @param string $filepath Location of file. * @param string $user User name to lock the file with * - * @return boolean|object True on success, or a PEAR_Error on failure. + * @return boolean True on success. */ public function lock($filepath, $user = null) { @@ -162,15 +162,11 @@ class Horde_Vcs_Rcs extends Horde_Vcs putenv('LOGNAME=guest'); } - $rcs_cmd = $this->getPath('rcs'); - $rcs_flag = ' -l '; - - $Q = VC_WINDOWS ? '"' : "'" ; - $cmd = $rcs_cmd . $rcs_flag . $Q . $filepath . $Q; - exec($cmd.' 2>&1', $return_array, $retval); + $cmd = $this->getPath('rcs') . ' -l ' . escapeshellarg($filepath); + exec($cmd . ' 2>&1', $return_array, $retval); if ($retval) { - return PEAR::raiseError('Failed to spawn rcs ("' . $cmd . '") on "' . $filepath . '" (returned ' . $retval . ')'); + throw new Horde_Vcs_Exception('Failed to spawn rcs ("' . $cmd . '") on "' . $filepath . '" (returned ' . $retval . ')'); } else { $output = ''; foreach ($return_array as $line) { @@ -181,7 +177,7 @@ class Horde_Vcs_Rcs extends Horde_Vcs if ($locked_pos !== false) { return true; } else { - return PEAR::raiseError('Failed to lock "' . $filepath . '" (Ran "' . $cmd . '", got return code ' . $retval . ', output: ' . $output . ')'); + throw new Horde_Vcs_Exception('Failed to lock "' . $filepath . '" (Ran "' . $cmd . '", got return code ' . $retval . ', output: ' . $output . ')'); } } } @@ -192,7 +188,7 @@ class Horde_Vcs_Rcs extends Horde_Vcs * @param string $filepath Location of file. * @param string $user User name to unlock the file with * - * @return boolean|object True on success, or a PEAR_Error on failure. + * @return boolean True on success. */ public function unlock($filepath, $user = null) { @@ -203,15 +199,11 @@ class Horde_Vcs_Rcs extends Horde_Vcs putenv('LOGNAME=guest'); } - $rcs_cmd = $this->getPath('rcs'); - $rcs_flag = ' -u '; - - $Q = VC_WINDOWS ? '"' : "'" ; - $cmd = $rcs_cmd . $rcs_flag . $Q . $filepath . $Q; + $cmd = $this->getPath('rcs') . ' -u ' . escapeshellarg($filepath); exec($cmd . ' 2>&1', $return_array, $retval); if ($retval) { - return PEAR::raiseError('Failed to spawn rcs ("' . $cmd . '") on "' . $filepath . '" (returned ' . $retval . ')'); + throw new Horde_Vcs_Exception('Failed to spawn rcs ("' . $cmd . '") on "' . $filepath . '" (returned ' . $retval . ')'); } else { $output = ''; @@ -235,19 +227,6 @@ class Horde_Vcs_Rcs extends Horde_Vcs class Horde_Vcs_Revision_Rcs extends Horde_Vcs_Revision { /** - * Validation function to ensure that a revision number is of the right - * form. - * - * @param mixed $rev The purported revision number. - * - * @return boolean True if it is a revision number. - */ - public function valid($rev) - { - return $rev && preg_match('/^[\d\.]+$/', $rev); - } - - /** * Given a revision number, remove a given number of portions from * it. For example, if we remove 2 portions of 1.2.3.4, we are * left with 1.2. diff --git a/framework/Vcs/lib/Horde/Vcs/Svn.php b/framework/Vcs/lib/Horde/Vcs/Svn.php index 2bd91becf..31c846f81 100644 --- a/framework/Vcs/lib/Horde/Vcs/Svn.php +++ b/framework/Vcs/lib/Horde/Vcs/Svn.php @@ -77,6 +77,11 @@ class Horde_Vcs_Svn extends Horde_Vcs return $command; } + + public function isValidRevision($rev) + { + return $rev && is_numeric($rev); + } } /** @@ -94,15 +99,14 @@ class Horde_Vcs_Annotate_Svn extends Horde_Vcs_Annotate */ public function doAnnotate($rev) { - if (is_a($this->_file, 'PEAR_Error') || - !$this->_rep->isValidRevision($rev)) { + if (!$this->_rep->isValidRevision($rev)) { return false; } $command = $this->_rep->getCommand() . ' annotate -r 1:' . $rev . ' ' . escapeshellarg($this->_file->queryFullPath()) . ' 2>&1'; $pipe = popen($command, 'r'); if (!$pipe) { - return PEAR::raiseError('Failed to execute svn annotate: ' . $command); + throw new Horde_Vcs_Exception('Failed to execute svn annotate: ' . $command); } $lines = array(); @@ -141,23 +145,22 @@ class Horde_Vcs_Checkout_Svn extends Horde_Vcs_Checkout * Function which returns a file pointing to the head of the requested * revision of a file. * - * @param Horde_Vcs $rep A repository object + * @param Horde_Vcs $rep A repository object * @param string $fullname Fully qualified pathname of the desired file * to checkout * @param string $rev Revision number to check out * - * @return resource|object Either a PEAR_Error object, or a stream - * pointer to the head of the checkout. + * @return resource A stream pointer to the head of the checkout. */ public function get($rep, $fullname, $rev) { if (!$rep->isValidRevision($rev)) { - return PEAR::raiseError('Invalid revision number'); + throw new Horde_Vcs_Exception('Invalid revision number'); } return ($RCS = popen($rep->getCommand() . ' cat -r ' . $rev . ' ' . escapeshellarg($fullname) . ' 2>&1', VC_WINDOWS ? 'rb' : 'r')) ? $RCS - : PEAR::raiseError('Couldn\'t perform checkout of the requested file'); + : throw new Horde_Vcs_Exception('Couldn\'t perform checkout of the requested file'); } } @@ -177,12 +180,12 @@ class Horde_Vcs_Diff_Svn extends Horde_Vcs_Diff * * @param Horde_Vcs $rep A repository object. * @param Horde_Vcs_File $file The desired file. - * @param string $rev1 Original revision number to compare from. - * @param string $rev2 New revision number to compare against. - * @param string $type The type of diff (e.g. 'unified'). - * @param integer $num Number of lines to be used in context and - * unified diffs. - * @param boolean $ws Show whitespace in the diff? + * @param string $rev1 Original revision number to compare from. + * @param string $rev2 New revision number to compare against. + * @param string $type The type of diff (e.g. 'unified'). + * @param integer $num Number of lines to be used in context and + * unified diffs. + * @param boolean $ws Show whitespace in the diff? * * @return string|boolean False on failure, or a string containing the * diff on success. @@ -190,11 +193,6 @@ class Horde_Vcs_Diff_Svn extends Horde_Vcs_Diff public function get($rep, $file, $rev1, $rev2, $type = 'context', $num = 3, $ws = true) { - /* Make sure that the file parameter is valid */ - if (is_a($file, 'PEAR_Error')) { - return false; - } - /* Check that the revision numbers are valid */ $rev1 = $rep->isValidRevision($rev1) ? $rev1 : 0; $rev2 = $rep->isValidRevision($rev1) ? $rev2 : 0; @@ -249,7 +247,7 @@ class Horde_Vcs_Directory_Svn extends Horde_Vcs_Directory * retrieve a list of all the objects in there. It then populates * the file/directory stack and makes it available for retrieval. * - * @return PEAR_Error object on an error, 1 on success. + * @return integer 1 on success. */ public function browseDir($cache = null, $quicklog = true, $showattic = false) @@ -258,7 +256,7 @@ class Horde_Vcs_Directory_Svn extends Horde_Vcs_Directory $dir = popen($cmd, 'r'); if (!$dir) { - return PEAR::raiseError('Failed to execute svn ls: ' . $cmd); + throw new Horde_Vcs_Exception('Failed to execute svn ls: ' . $cmd); } /* Create two arrays - one of all the files, and the other of @@ -275,19 +273,14 @@ class Horde_Vcs_Directory_Svn extends Horde_Vcs_Directory } elseif (substr($line, -1) == '/') { $this->_dirs[] = substr($line, 0, -1); } else { - $fl = $this->_rep->getFileObject($this->queryDir() . "/$line", array('cache' => $cache, 'quicklog' => $quicklog)); - if (is_a($fl, 'PEAR_Error')) { - return $fl; - } else { - $this->_files[] = $fl; - } + $this->_files[] = $this->_rep->getFileObject($this->queryDir() . "/$line", array('cache' => $cache, 'quicklog' => $quicklog)); } } pclose($dir); return $errors - ? PEAR::raiseError(implode("\n", $errors)) + ? throw new Horde_Vcs_Exception(implode("\n", $errors)) : true; } @@ -317,75 +310,22 @@ class Horde_Vcs_File_Svn extends Horde_Vcs_File { $this->name = basename($fl); $this->dir = dirname($fl); $this->filename = $fl; - $this->cache = empty($opts['cache']) ? null : $opts['cache']; $this->quicklog = !empty($opts['quicklog']); - } - - public function getFileObject() - { - /* The version of the cached data. Increment this whenever the - * internal storage format changes, such that we must - * invalidate prior cached data. */ - $cacheVersion = 2; - $cacheId = $this->rep->sourceroot() . '_n' . $this->filename . '_f' . (int)$this->quicklog . '_v' . $cacheVersion; - - if ($this->cache && - // The file is cached for one hour no matter what, because - // there is no way to determine with Subversion the time - // the file last changed. - $this->cache->exists($cacheId, 3600)) { - $fileOb = unserialize($this->cache->get($cacheId, 3600)); - $fileOb->setRepository($rep); - } else { - $fileOb = new Horde_Vcs_File_Svn($this->rep, $this->filename, $this->cache, $this->quicklog); - if (is_a(($result = $fileOb->getBrowseInfo()), 'PEAR_Error')) { - return $result; - } - $fileOb->applySort(Horde_Vcs::SORT_AGE); - - if ($this->cache) { - $this->cache->set($cacheId, serialize($fileOb)); - } - } - - return $fileOb; - } - /** - * Returns name of the current file without the repository - * extensions (usually ,v) - * - * @return Filename without repository extension - */ - function queryName() - { - return preg_replace('/,v$/', '', $this->name); - } - - /** - * Populate the object with information about the revisions logs - * and dates of the file. - * - * @return mixed boolean True on success, - * PEAR_Error On error. - */ - function getBrowseInfo() - { /* This doesn't work; need to find another way to simply * request the most recent revision: * * $flag = $this->quicklog ? '-r HEAD ' : ''; */ $flag = ''; - $Q = VC_WINDOWS ? '"' : "'"; - $cmd = $this->rep->getCommand() . ' log -v ' . $flag . $Q . str_replace($Q, '\\' . $Q, $this->queryFullPath()) . $Q . ' 2>&1'; + $cmd = $this->rep->getCommand() . ' log -v ' . $flag . escapeshellarg($this->queryFullPath()) . ' 2>&1'; $pipe = popen($cmd, 'r'); if (!$pipe) { - return PEAR::raiseError('Failed to execute svn log: ' . $cmd); + throw new Horde_Vcs_Exception('Failed to execute svn log: ' . $cmd); } $header = fgets($pipe); if (!strspn($header, '-')) { - return PEAR::raiseError('Error executing svn log: ' . $header); + throw new Horde_Vcs_Exception('Error executing svn log: ' . $header); } while (!feof($pipe)) { @@ -403,7 +343,17 @@ class Horde_Vcs_File_Svn extends Horde_Vcs_File { } pclose($pipe); - return true; + } + + /** + * Returns name of the current file without the repository + * extensions (usually ,v) + * + * @return Filename without repository extension + */ + function queryName() + { + return preg_replace('/,v$/', '', $this->name); } } @@ -472,14 +422,11 @@ class Horde_Vcs_Patchset_Svn extends Horde_Vcs_Patchset * Populate the object with information about the patchsets that * this file is involved in. * - * @return mixed PEAR_Error object on error, or true on success. + * @return boolean True on success. */ function getPatchsets() { $fileOb = new Horde_Vcs_File_Svn($this->_rep, $this->_file); - if (is_a(($result = $fileOb->getBrowseInfo()), 'PEAR_Error')) { - return $result; - } $this->_patchsets = array(); foreach ($fileOb->logs as $rev => $log) { @@ -515,19 +462,3 @@ class Horde_Vcs_Patchset_Svn extends Horde_Vcs_Patchset } } - -class Horde_Vcs_Revision_Svn extends Horde_Vcs_Revision_Rcs -{ - /** - * Validation function to ensure that a revision number is of the right - * form. - * - * @param mixed $rev The purported revision number. - * - * @return boolean True if it is a revision number. - */ - public function valid($rev) - { - return is_numeric($rev); - } -} -- 2.11.0