diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0c6ea3d867..e7f9b09d86 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1300,6 +1300,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryManagementWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementWorkflow.php', 'PhabricatorRepositoryMercurialCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/PhabricatorRepositoryMercurialCommitChangeParserWorker.php', 'PhabricatorRepositoryMercurialCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php', + 'PhabricatorRepositoryPullEngine' => 'applications/repository/engine/PhabricatorRepositoryPullEngine.php', 'PhabricatorRepositoryPullLocalDaemon' => 'applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php', 'PhabricatorRepositoryPullLocalDaemonTestCase' => 'applications/repository/daemon/__tests__/PhabricatorRepositoryPullLocalDaemonTestCase.php', 'PhabricatorRepositoryQuery' => 'applications/repository/query/PhabricatorRepositoryQuery.php', @@ -1487,6 +1488,8 @@ phutil_register_library_map(array( 'PhabricatorWorkerTaskDetailController' => 'applications/daemon/controller/PhabricatorWorkerTaskDetailController.php', 'PhabricatorWorkerTaskUpdateController' => 'applications/daemon/controller/PhabricatorWorkerTaskUpdateController.php', 'PhabricatorWorkerTestCase' => 'infrastructure/daemon/workers/__tests__/PhabricatorWorkerTestCase.php', + 'PhabricatorWorkingCopyPullTestCase' => 'applications/repository/engine/__tests__/PhabricatorWorkingCopyPullTestCase.php', + 'PhabricatorWorkingCopyTestCase' => 'applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php', 'PhabricatorWorkpanelView' => 'view/layout/PhabricatorWorkpanelView.php', 'PhabricatorXHPASTViewController' => 'applications/phpast/controller/PhabricatorXHPASTViewController.php', 'PhabricatorXHPASTViewDAO' => 'applications/phpast/storage/PhabricatorXHPASTViewDAO.php', @@ -3194,6 +3197,8 @@ phutil_register_library_map(array( 'PhabricatorWorkerTaskDetailController' => 'PhabricatorDaemonController', 'PhabricatorWorkerTaskUpdateController' => 'PhabricatorDaemonController', 'PhabricatorWorkerTestCase' => 'PhabricatorTestCase', + 'PhabricatorWorkingCopyPullTestCase' => 'PhabricatorWorkingCopyTestCase', + 'PhabricatorWorkingCopyTestCase' => 'PhabricatorTestCase', 'PhabricatorWorkpanelView' => 'AphrontView', 'PhabricatorXHPASTViewController' => 'PhabricatorController', 'PhabricatorXHPASTViewDAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php index b6e0085d9c..ca8739f700 100644 --- a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php +++ b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php @@ -120,7 +120,9 @@ final class PhabricatorRepositoryPullLocalDaemon $callsign = $repository->getCallsign(); $this->log("Updating repository '{$callsign}'."); - $this->pullRepository($repository); + id(new PhabricatorRepositoryPullEngine()) + ->setRepository($repository) + ->pullRepository(); if (!$no_discovery) { // TODO: It would be nice to discover only if we pulled something, @@ -175,54 +177,6 @@ final class PhabricatorRepositoryPullLocalDaemon } } - - /** - * @task pull - */ - public function pullRepository(PhabricatorRepository $repository) { - $vcs = $repository->getVersionControlSystem(); - - $is_svn = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_SVN); - $is_git = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT); - $is_hg = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL); - - if ($is_svn) { - return; - } - - $callsign = $repository->getCallsign(); - - if (!$is_git && !$is_hg) { - throw new Exception( - "Unknown VCS '{$vcs}' for repository '{$callsign}'!"); - } - - $local_path = $repository->getDetail('local-path'); - if (!$local_path) { - throw new Exception( - "No local path is available for repository '{$callsign}'."); - } - - if (!Filesystem::pathExists($local_path)) { - $dirname = dirname($local_path); - if (!Filesystem::pathExists($dirname)) { - Filesystem::createDirectory($dirname, 0755, $recursive = true); - } - - if ($is_git) { - return $this->executeGitCreate($repository, $local_path); - } else if ($is_hg) { - return $this->executeHgCreate($repository, $local_path); - } - } else { - if ($is_git) { - return $this->executeGitUpdate($repository, $local_path); - } else if ($is_hg) { - return $this->executeHgUpdate($repository, $local_path); - } - } - } - public function discoverRepository(PhabricatorRepository $repository) { $vcs = $repository->getVersionControlSystem(); switch ($vcs) { @@ -468,121 +422,6 @@ final class PhabricatorRepositoryPullLocalDaemon /* -( Git Implementation )------------------------------------------------- */ - private function canWrite($path) { - $default_path = - PhabricatorEnv::getEnvConfig('repository.default-local-path'); - return Filesystem::isDescendant($path, $default_path); - } - - /** - * @task git - */ - private function executeGitCreate( - PhabricatorRepository $repository, - $path) { - - $repository->execxRemoteCommand( - 'clone --origin origin %s %s', - $repository->getRemoteURI(), - rtrim($path, '/')); - } - - - /** - * @task git - */ - private function executeGitUpdate( - PhabricatorRepository $repository, - $path) { - - // Run a bunch of sanity checks to detect people checking out repositories - // inside other repositories, making empty directories, pointing the local - // path at some random file or path, etc. - - list($err, $stdout) = $repository->execLocalCommand( - 'rev-parse --show-toplevel'); - $msg = ''; - - if ($err) { - - // Try to raise a more tailored error message in the more common case - // of the user creating an empty directory. (We could try to remove it, - // but might not be able to, and it's much simpler to raise a good - // message than try to navigate those waters.) - if (is_dir($path)) { - $files = Filesystem::listDirectory($path, $include_hidden = true); - if (!$files) { - $msg = - "Expected to find a git repository at '{$path}', but there ". - "is an empty directory there. Remove the directory: the daemon ". - "will run 'git clone' for you."; - } - } else { - $msg = - "Expected to find a git repository at '{$path}', but there is ". - "a non-repository directory (with other stuff in it) there. Move or ". - "remove this directory (or reconfigure the repository to use a ". - "different directory), and then either clone a repository yourself ". - "or let the daemon do it."; - } - } else { - $repo_path = rtrim($stdout, "\n"); - - if (empty($repo_path)) { - $err = true; - $msg = - "Expected to find a git repository at '{$path}', but ". - "there was no result from `git rev-parse --show-toplevel`. ". - "Something is misconfigured or broken. The git repository ". - "may be inside a '.git/' directory."; - } else if (!Filesystem::pathsAreEquivalent($repo_path, $path)) { - $err = true; - $msg = - "Expected to find repo at '{$path}', but the actual ". - "git repository root for this directory is '{$repo_path}'. ". - "Something is misconfigured. The repository's 'Local Path' should ". - "be set to some place where the daemon can check out a working ". - "copy, and should not be inside another git repository."; - } - } - - if ($err && $this->canWrite($path)) { - phlog("{$path} failed sanity check; recloning. ({$msg})"); - Filesystem::remove($path); - $this->executeGitCreate($repository, $path); - } else if ($err) { - throw new Exception($msg); - } - - $retry = false; - do { - // This is a local command, but needs credentials. - $future = $repository->getRemoteCommandFuture('fetch --all --prune'); - $future->setCWD($path); - list($err, $stdout, $stderr) = $future->resolve(); - - if ($err && !$retry && $this->canWrite($path)) { - $retry = true; - // Fix remote origin url if it doesn't match our configuration - $origin_url = - $repository->execLocalCommand('config --get remote.origin.url'); - $remote_uri = $repository->getDetail('remote-uri'); - if ($origin_url != $remote_uri) { - $repository->execLocalCommand('remote set-url origin %s', - $remote_uri); - } - } else if ($err) { - throw new Exception( - "git fetch failed with error #{$err}:\n". - "stdout:{$stdout}\n\n". - "stderr:{$stderr}\n"); - } else { - $retry = false; - } - } while ($retry); - } - - /** * @task git */ @@ -787,60 +626,6 @@ final class PhabricatorRepositoryPullLocalDaemon /* -( Mercurial Implementation )------------------------------------------- */ - /** - * @task hg - */ - private function executeHgCreate( - PhabricatorRepository $repository, - $path) { - - $repository->execxRemoteCommand( - 'clone %s %s', - $repository->getRemoteURI(), - rtrim($path, '/')); - } - - - /** - * @task hg - */ - private function executeHgUpdate( - PhabricatorRepository $repository, - $path) { - - // This is a local command, but needs credentials. - $future = $repository->getRemoteCommandFuture('pull -u'); - $future->setCWD($path); - - try { - $future->resolvex(); - } catch (CommandException $ex) { - $err = $ex->getError(); - $stdout = $ex->getStdOut(); - - // NOTE: Between versions 2.1 and 2.1.1, Mercurial changed the behavior - // of "hg pull" to return 1 in case of a successful pull with no changes. - // This behavior has been reverted, but users who updated between Feb 1, - // 2012 and Mar 1, 2012 will have the erroring version. Do a dumb test - // against stdout to check for this possibility. - // See: https://github.com/facebook/phabricator/issues/101/ - - // NOTE: Mercurial has translated versions, which translate this error - // string. In a translated version, the string will be something else, - // like "aucun changement trouve". There didn't seem to be an easy way - // to handle this (there are hard ways but this is not a common problem - // and only creates log spam, not application failures). Assume English. - - // TODO: Remove this once we're far enough in the future that deployment - // of 2.1 is exceedingly rare? - if ($err == 1 && preg_match('/no changes found/', $stdout)) { - return; - } else { - throw $ex; - } - } - } - private function executeHgDiscover(PhabricatorRepository $repository) { // NOTE: "--debug" gives us 40-character hashes. list($stdout) = $repository->execxLocalCommand('--debug branches'); diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php new file mode 100644 index 0000000000..d295cc7346 --- /dev/null +++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php @@ -0,0 +1,267 @@ +repository = $repository; + return $this; + } + + private function getRepository() { + return $this->repository; + } + + public function pullRepository() { + $repository = $this->getRepository(); + + if (!$repository) { + throw new Exception("Call setRepository() before pullRepository()!"); + } + + $is_hg = false; + $is_git = false; + + $vcs = $repository->getVersionControlSystem(); + switch ($vcs) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + // We never pull a local copy of Subversion repositories. + return; + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + $is_git = true; + break; + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + $is_hg = true; + break; + default: + throw new Exception("Unsupported VCS '{$vcs}'!"); + } + + $callsign = $repository->getCallsign(); + $local_path = $repository->getLocalPath(); + if ($local_path === null) { + throw new Exception( + "No local path is configured for repository '{$callsign}'."); + } + + $dirname = dirname($local_path); + if (!Filesystem::pathExists($dirname)) { + Filesystem::createDirectory($dirname, 0755, $recursive = true); + } + + if (!Filesystem::pathExists($local_path)) { + if ($is_git) { + $this->executeGitCreate(); + } else { + $this->executeMercurialCreate(); + } + } else { + if ($is_git) { + $this->executeGitUpdate(); + } else { + $this->executeMercurialUpdate(); + } + } + + return $this; + } + + +/* -( Pulling Git Working Copies )----------------------------------------- */ + + + /** + * @task git + */ + private function executeGitCreate() { + $repository = $this->getRepository(); + + $repository->execxRemoteCommand( + 'clone --origin origin %s %s', + $repository->getRemoteURI(), + rtrim($repository->getLocalPath(), '/')); + } + + + /** + * @task git + */ + private function executeGitUpdate() { + $repository = $this->getRepository(); + + list($err, $stdout) = $repository->execLocalCommand( + 'rev-parse --show-toplevel'); + + $message = null; + $path = $repository->getLocalPath(); + if ($err) { + // Try to raise a more tailored error message in the more common case + // of the user creating an empty directory. (We could try to remove it, + // but might not be able to, and it's much simpler to raise a good + // message than try to navigate those waters.) + if (is_dir($path)) { + $files = Filesystem::listDirectory($path, $include_hidden = true); + if (!$files) { + $message = + "Expected to find a git repository at '{$path}', but there ". + "is an empty directory there. Remove the directory: the daemon ". + "will run 'git clone' for you."; + } else { + $message = + "Expected to find a git repository at '{$path}', but there is ". + "a non-repository directory (with other stuff in it) there. Move ". + "or remove this directory (or reconfigure the repository to use a ". + "different directory), and then either clone a repository ". + "yourself or let the daemon do it."; + } + } else if (is_file($path)) { + $message = + "Expected to find a git repository at '{$path}', but there is a ". + "file there instead. Remove it and let the daemon clone a ". + "repository for you."; + } else { + $message = + "Expected to find a git repository at '{$path}', but did not."; + } + } else { + $repo_path = rtrim($stdout, "\n"); + + if (empty($repo_path)) { + $err = true; + $msg = + "Expected to find a git repository at '{$path}', but ". + "there was no result from `git rev-parse --show-toplevel`. ". + "Something is misconfigured or broken. The git repository ". + "may be inside a '.git/' directory."; + } else if (!Filesystem::pathsAreEquivalent($repo_path, $path)) { + $err = true; + $msg = + "Expected to find repo at '{$path}', but the actual ". + "git repository root for this directory is '{$repo_path}'. ". + "Something is misconfigured. The repository's 'Local Path' should ". + "be set to some place where the daemon can check out a working ". + "copy, and should not be inside another git repository."; + } + } + + if ($err && $this->canDestroyWorkingCopy($path)) { + phlog("Repository working copy at '{$path}' failed sanity check; ". + "destroying and re-cloning. {$message}"); + Filesystem::remove($path); + $this->executeGitCreate(); + } else if ($err) { + throw new Exception($message); + } + + $retry = false; + do { + // This is a local command, but needs credentials. + $future = $repository->getRemoteCommandFuture('fetch --all --prune'); + $future->setCWD($path); + list($err, $stdout, $stderr) = $future->resolve(); + + if ($err && !$retry && $this->canDestroyWorkingCopy($path)) { + $retry = true; + // Fix remote origin url if it doesn't match our configuration + $origin_url = $repository->execLocalCommand( + 'config --get remote.origin.url'); + $remote_uri = $repository->getDetail('remote-uri'); + if ($origin_url != $remote_uri) { + $repository->execLocalCommand( + 'remote set-url origin %s', + $remote_uri); + } + } else if ($err) { + throw new Exception( + "git fetch failed with error #{$err}:\n". + "stdout:{$stdout}\n\n". + "stderr:{$stderr}\n"); + } else { + $retry = false; + } + } while ($retry); + } + + +/* -( Pulling Mercurial Working Copies )----------------------------------- */ + + + /** + * @task hg + */ + private function executeMercurialCreate() { + $repository = $this->getRepository(); + + $repository->execxRemoteCommand( + 'clone %s %s', + $repository->getRemoteURI(), + rtrim($repository->getLocalPath(), '/')); + } + + + /** + * @task hg + */ + private function executeMercurialUpdate() { + $repository = $this->getRepository(); + $path = $repository->getLocalPath(); + + // This is a local command, but needs credentials. + $future = $repository->getRemoteCommandFuture('pull -u'); + $future->setCWD($path); + + try { + $future->resolvex(); + } catch (CommandException $ex) { + $err = $ex->getError(); + $stdout = $ex->getStdOut(); + + // NOTE: Between versions 2.1 and 2.1.1, Mercurial changed the behavior + // of "hg pull" to return 1 in case of a successful pull with no changes. + // This behavior has been reverted, but users who updated between Feb 1, + // 2012 and Mar 1, 2012 will have the erroring version. Do a dumb test + // against stdout to check for this possibility. + // See: https://github.com/facebook/phabricator/issues/101/ + + // NOTE: Mercurial has translated versions, which translate this error + // string. In a translated version, the string will be something else, + // like "aucun changement trouve". There didn't seem to be an easy way + // to handle this (there are hard ways but this is not a common problem + // and only creates log spam, not application failures). Assume English. + + // TODO: Remove this once we're far enough in the future that deployment + // of 2.1 is exceedingly rare? + if ($err == 1 && preg_match('/no changes found/', $stdout)) { + return; + } else { + throw $ex; + } + } + } + + +/* -( Internals )---------------------------------------------------------- */ + + + private function canDestroyWorkingCopy($path) { + $default_path = PhabricatorEnv::getEnvConfig( + 'repository.default-local-path'); + return Filesystem::isDescendant($path, $default_path); + } + +} diff --git a/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyPullTestCase.php b/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyPullTestCase.php new file mode 100644 index 0000000000..0ab91d2b48 --- /dev/null +++ b/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyPullTestCase.php @@ -0,0 +1,32 @@ +buildPulledRepository('GT'); + + $this->assertEqual( + true, + Filesystem::pathExists($repo->getLocalPath().'/.git')); + } + + public function testHgPullBasic() { + $repo = $this->buildPulledRepository('HT'); + + $this->assertEqual( + true, + Filesystem::pathExists($repo->getLocalPath().'/.hg')); + } + + public function testSVNPullBasic() { + $repo = $this->buildPulledRepository('ST'); + + // We don't pull local clones for SVN, so we don't expect there to be + // a working copy. + $this->assertEqual( + false, + Filesystem::pathExists($repo->getLocalPath())); + } + +} diff --git a/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php b/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php new file mode 100644 index 0000000000..e8da90dde1 --- /dev/null +++ b/src/applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php @@ -0,0 +1,95 @@ + true, + ); + } + + protected function buildBareRepository($callsign) { + if (isset($this->repos[$callsign])) { + return $this->repos[$callsign]; + } + + $data_dir = dirname(__FILE__).'/data/'; + + $types = array( + 'svn' => PhabricatorRepositoryType::REPOSITORY_TYPE_SVN, + 'hg' => PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL, + 'git' => PhabricatorRepositoryType::REPOSITORY_TYPE_GIT, + ); + + $hits = array(); + foreach ($types as $type => $const) { + $path = $data_dir.$callsign.'.'.$type.'.tgz'; + if (Filesystem::pathExists($path)) { + $hits[$const] = $path; + } + } + + if (!$hits) { + throw new Exception( + "No test data for callsign '{$callsign}'. Expected an archive ". + "like '{$callsign}.git.tgz' in '{$data_dir}'."); + } + + if (count($hits) > 1) { + throw new Exception( + "Expected exactly one archive matching callsign '{$callsign}', ". + "found too many: ".implode(', ', $hits)); + } + + $path = head($hits); + $vcs_type = head_key($hits); + + $dir = PhutilDirectoryFixture::newFromArchive($path); + $local = new TempFile('.ignore'); + + $repo = id(new PhabricatorRepository()) + ->setCallsign($callsign) + ->setName(pht('Test Repo "%s"', $callsign)) + ->setVersionControlSystem($vcs_type) + ->setDetail('local-path', dirname($local).'/'.$callsign) + ->setDetail('remote-uri', 'file://'.$dir->getPath().'/'); + + $this->didConstructRepository($repo); + + $repo->save(); + $repo->makeEphemeral(); + + // Keep the disk resources around until we exit. + $this->dirs[] = $dir; + $this->dirs[] = $local; + + $this->repos[$callsign] = $repo; + + return $repo; + } + + protected function didConstructRepository(PhabricatorRepository $repository) { + return; + } + + protected function buildPulledRepository($callsign) { + $repository = $this->buildBareRepository($callsign); + + if (isset($this->pulled[$callsign])) { + return $repository; + } + + id(new PhabricatorRepositoryPullEngine()) + ->setRepository($repository) + ->pullRepository(); + + $this->pulled[$callsign] = true; + + return $repository; + } + +} diff --git a/src/applications/repository/engine/__tests__/data/GT.git.tgz b/src/applications/repository/engine/__tests__/data/GT.git.tgz new file mode 100644 index 0000000000..b53e9a2d1c Binary files /dev/null and b/src/applications/repository/engine/__tests__/data/GT.git.tgz differ diff --git a/src/applications/repository/engine/__tests__/data/HT.hg.tgz b/src/applications/repository/engine/__tests__/data/HT.hg.tgz new file mode 100644 index 0000000000..7b170be199 Binary files /dev/null and b/src/applications/repository/engine/__tests__/data/HT.hg.tgz differ diff --git a/src/applications/repository/engine/__tests__/data/ST.svn.tgz b/src/applications/repository/engine/__tests__/data/ST.svn.tgz new file mode 100644 index 0000000000..c7899faeb1 Binary files /dev/null and b/src/applications/repository/engine/__tests__/data/ST.svn.tgz differ diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php index ec7eab7c94..c2088d7fd3 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php @@ -10,10 +10,6 @@ final class PhabricatorRepositoryManagementPullWorkflow ->setSynopsis('Pull __repository__, named by callsign or PHID.') ->setArguments( array( - array( - 'name' => 'verbose', - 'help' => 'Show additional debugging information.', - ), array( 'name' => 'repos', 'wildcard' => true, @@ -34,9 +30,9 @@ final class PhabricatorRepositoryManagementPullWorkflow foreach ($repos as $repo) { $console->writeOut("Pulling '%s'...\n", $repo->getCallsign()); - $daemon = new PhabricatorRepositoryPullLocalDaemon(array()); - $daemon->setVerbose($args->getArg('verbose')); - $daemon->pullRepository($repo); + id(new PhabricatorRepositoryPullEngine()) + ->setRepository($repo) + ->pullRepository(); } $console->writeOut("Done.\n");