diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 1bca354414..e11c20a33c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -622,6 +622,7 @@ phutil_register_library_map(array( 'DiffusionController' => 'applications/diffusion/controller/DiffusionController.php', 'DiffusionCreateCommentConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionCreateCommentConduitAPIMethod.php', 'DiffusionCreateRepositoriesCapability' => 'applications/diffusion/capability/DiffusionCreateRepositoriesCapability.php', + 'DiffusionDaemonLockException' => 'applications/diffusion/exception/DiffusionDaemonLockException.php', 'DiffusionDefaultEditCapability' => 'applications/diffusion/capability/DiffusionDefaultEditCapability.php', 'DiffusionDefaultPushCapability' => 'applications/diffusion/capability/DiffusionDefaultPushCapability.php', 'DiffusionDefaultViewCapability' => 'applications/diffusion/capability/DiffusionDefaultViewCapability.php', @@ -4845,6 +4846,7 @@ phutil_register_library_map(array( 'DiffusionController' => 'PhabricatorController', 'DiffusionCreateCommentConduitAPIMethod' => 'DiffusionConduitAPIMethod', 'DiffusionCreateRepositoriesCapability' => 'PhabricatorPolicyCapability', + 'DiffusionDaemonLockException' => 'Exception', 'DiffusionDefaultEditCapability' => 'PhabricatorPolicyCapability', 'DiffusionDefaultPushCapability' => 'PhabricatorPolicyCapability', 'DiffusionDefaultViewCapability' => 'PhabricatorPolicyCapability', diff --git a/src/applications/almanac/util/AlmanacKeys.php b/src/applications/almanac/util/AlmanacKeys.php index 3f482416df..7ed9098e97 100644 --- a/src/applications/almanac/util/AlmanacKeys.php +++ b/src/applications/almanac/util/AlmanacKeys.php @@ -10,6 +10,14 @@ final class AlmanacKeys extends Phobject { } public static function getDeviceID() { + // While running unit tests, ignore any configured device identity. + try { + PhabricatorTestCase::assertExecutingUnitTests(); + return null; + } catch (Exception $ex) { + // Continue normally. + } + $device_id_path = self::getKeyPath('device.id'); if (Filesystem::pathExists($device_id_path)) { diff --git a/src/applications/diffusion/exception/DiffusionDaemonLockException.php b/src/applications/diffusion/exception/DiffusionDaemonLockException.php new file mode 100644 index 0000000000..f2f5be0018 --- /dev/null +++ b/src/applications/diffusion/exception/DiffusionDaemonLockException.php @@ -0,0 +1,3 @@ +getRepository(); + $lock = $this->newRepositoryLock($repository, 'repo.look', false); + + try { + $lock->lock(); + } catch (PhutilLockException $ex) { + throw new DiffusionDaemonLockException( + pht( + 'Another process is currently discovering repository "%s", '. + 'skipping discovery.', + $repository->getDisplayName())); + } + + try { + $result = $this->discoverCommitsWithLock(); + } catch (Exception $ex) { + $lock->unlock(); + throw $ex; + } + + $lock->unlock(); + + return $result; + } + + private function discoverCommitsWithLock() { + $repository = $this->getRepository(); + $vcs = $repository->getVersionControlSystem(); switch ($vcs) { case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: diff --git a/src/applications/repository/engine/PhabricatorRepositoryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryEngine.php index 42ad650a38..445357e783 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryEngine.php @@ -51,6 +51,27 @@ abstract class PhabricatorRepositoryEngine extends Phobject { return PhabricatorUser::getOmnipotentUser(); } + protected function newRepositoryLock( + PhabricatorRepository $repository, + $lock_key, + $lock_device_only) { + + $lock_parts = array(); + $lock_parts[] = $lock_key; + $lock_parts[] = $repository->getID(); + + if ($lock_device_only) { + $device = AlmanacKeys::getLiveDevice(); + if ($device) { + $lock_parts[] = $device->getID(); + } + } + + $lock_name = implode(':', $lock_parts); + return PhabricatorGlobalLock::newLock($lock_name); + } + + /** * Verify that the "origin" remote exists, and points at the correct URI. * diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php index 621c718577..9e151ecd81 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php @@ -23,6 +23,33 @@ final class PhabricatorRepositoryPullEngine public function pullRepository() { $repository = $this->getRepository(); + + $lock = $this->newRepositoryLock($repository, 'repo.pull', true); + + try { + $lock->lock(); + } catch (PhutilLockException $ex) { + throw new DiffusionDaemonLockException( + pht( + 'Another process is currently updating repository "%s", '. + 'skipping pull.', + $repository->getDisplayName())); + } + + try { + $result = $this->pullRepositoryWithLock(); + } catch (Exception $ex) { + $lock->unlock(); + throw $ex; + } + + $lock->unlock(); + + return $result; + } + + private function pullRepositoryWithLock() { + $repository = $this->getRepository(); $viewer = PhabricatorUser::getOmnipotentUser(); $is_hg = false; diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php index 11ad6a9418..f435861c60 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php @@ -53,60 +53,42 @@ final class PhabricatorRepositoryManagementUpdateWorkflow $repository = head($repos); try { - $lock_name = 'repository.update:'.$repository->getID(); - $lock = PhabricatorGlobalLock::newLock($lock_name); + id(new PhabricatorRepositoryPullEngine()) + ->setRepository($repository) + ->setVerbose($this->getVerbose()) + ->pullRepository(); - try { - $lock->lock(); - } catch (PhutilLockException $ex) { - throw new PhutilProxyException( - pht( - 'Another process is currently holding the update lock for '. - 'repository "%s". Repositories may only be updated by one '. - 'process at a time. This can happen if you are running multiple '. - 'copies of the daemons. This can also happen if you manually '. - 'update a repository while the daemons are also updating it '. - '(in this case, just try again in a few moments).', - $repository->getMonogram()), - $ex); + $no_discovery = $args->getArg('no-discovery'); + if ($no_discovery) { + return 0; } - try { - $no_discovery = $args->getArg('no-discovery'); + // TODO: It would be nice to discover only if we pulled something, but + // this isn't totally trivial. It's slightly more complicated with + // hosted repositories, too. - id(new PhabricatorRepositoryPullEngine()) - ->setRepository($repository) - ->setVerbose($this->getVerbose()) - ->pullRepository(); + $repository->writeStatusMessage( + PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE, + null); - if ($no_discovery) { - $lock->unlock(); - return; - } + $this->discoverRepository($repository); - // TODO: It would be nice to discover only if we pulled something, but - // this isn't totally trivial. It's slightly more complicated with - // hosted repositories, too. + $this->checkIfRepositoryIsFullyImported($repository); - $repository->writeStatusMessage( - PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE, - null); + $this->updateRepositoryRefs($repository); - $this->discoverRepository($repository); + $this->mirrorRepository($repository); - $this->checkIfRepositoryIsFullyImported($repository); - - $this->updateRepositoryRefs($repository); - - $this->mirrorRepository($repository); - - $repository->writeStatusMessage( - PhabricatorRepositoryStatusMessage::TYPE_FETCH, - PhabricatorRepositoryStatusMessage::CODE_OKAY); - } catch (Exception $ex) { - $lock->unlock(); - throw $ex; - } + $repository->writeStatusMessage( + PhabricatorRepositoryStatusMessage::TYPE_FETCH, + PhabricatorRepositoryStatusMessage::CODE_OKAY); + } catch (DiffusionDaemonLockException $ex) { + // If we miss a pull or discover because some other process is already + // doing the work, just bail out. + echo tsprintf( + "%s\n", + $ex->getMessage()); + return 0; } catch (Exception $ex) { $repository->writeStatusMessage( PhabricatorRepositoryStatusMessage::TYPE_FETCH, @@ -118,12 +100,11 @@ final class PhabricatorRepositoryManagementUpdateWorkflow throw $ex; } - $lock->unlock(); - - $console->writeOut( + echo tsprintf( + "%s\n", pht( - 'Updated repository **%s**.', - $repository->getMonogram())."\n"); + 'Updated repository "%s".', + $repository->getDisplayName())); return 0; }