revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-01-08 22:01:03 +01:00
Code Issues Releases Wiki Activity

Make repository pulls testable

Browse source 
Summary:
Ref T2784. This moves us toward being able to test the background and Conduit pipelines for repositories. In particular:

  - Separate the logic for pulling repositories (`git pull`, `hg pull`) out of `PhabricatorRepositoryPullLocalDaemon` and put it in `PhabricatorRepositoryPullEngine`. This allows repositories to be pulled directly without invoking the daemons.
  - Add tests for the engine, including a future-looking base test case.
  - Add basic `PhutilDirectoryFixture`-based repositories.

Next steps:

  # Do the same for repo discovery.
  # Then we can start writing tests against specific Conduit methods.

Test Plan: Ran unit tests. Ran `bin/repository pull` on SVN, Hg and Git repositories. Ran `bin/phd debug pulllocal`.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, nh

Maniphest Tasks: T2784

Differential Revision: https://secure.phabricator.com/D5904
This commit is contained in:
epriestley 2013-05-12 19:05:52 -07:00
parent 98e2ad4ebc
commit 105dd1899c
9 changed files with 405 additions and 225 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
src/__phutil_library_map__.php
View file

@ -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',

221
src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
View file

@ -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');

267
src/applications/repository/engine/PhabricatorRepositoryPullEngine.php Normal file
View file

@ -0,0 +1,267 @@
<?php
/**
* Manages execution of `git pull` and `hg pull` commands for
* @{class:PhabricatorRepository} objects. Used by
* @{class:PhabricatorRepositoryPullLocalDaemon}.
*
* @task pull Pulling Working Copies
* @task git Pulling Git Working Copies
* @task hg Pulling Mercurial Working Copies
* @task internal Internals
*/
final class PhabricatorRepositoryPullEngine {
private $repository;
/* -( Pulling Working Copies )--------------------------------------------- */
public function setRepository(PhabricatorRepository $repository) {
$this->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);
}
}

32
src/applications/repository/engine/__tests__/PhabricatorWorkingCopyPullTestCase.php Normal file
View file

@ -0,0 +1,32 @@
<?php
final class PhabricatorWorkingCopyPullTestCase
extends PhabricatorWorkingCopyTestCase {
public function testGitPullBasic() {
$repo = $this->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()));
}
}

95
src/applications/repository/engine/__tests__/PhabricatorWorkingCopyTestCase.php Normal file
View file

@ -0,0 +1,95 @@
<?php
abstract class PhabricatorWorkingCopyTestCase extends PhabricatorTestCase {
private $dirs = array();
private $repos = array();
private $pulled = array();
protected function getPhabricatorTestCaseConfiguration() {
return array(
self::PHABRICATOR_TESTCONFIG_BUILD_STORAGE_FIXTURES => 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;
}
}

BIN
src/applications/repository/engine/__tests__/data/GT.git.tgz Normal file
View file

Binary file not shown.

BIN
src/applications/repository/engine/__tests__/data/HT.hg.tgz Normal file
View file

Binary file not shown.

BIN
src/applications/repository/engine/__tests__/data/ST.svn.tgz Normal file
View file

Binary file not shown.

10
src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php
View file

@ -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");