Page MenuHomePhabricator

D15903.diff
No OneTemporary

D15903.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -622,6 +622,7 @@
'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 @@
'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
--- a/src/applications/almanac/util/AlmanacKeys.php
+++ b/src/applications/almanac/util/AlmanacKeys.php
@@ -10,6 +10,14 @@
}
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
--- /dev/null
+++ b/src/applications/diffusion/exception/DiffusionDaemonLockException.php
@@ -0,0 +1,3 @@
+<?php
+
+final class DiffusionDaemonLockException extends Exception {}
diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
--- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
@@ -37,6 +37,33 @@
public function discoverCommits() {
$repository = $this->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
--- a/src/applications/repository/engine/PhabricatorRepositoryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryEngine.php
@@ -51,6 +51,27 @@
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
--- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
@@ -23,6 +23,33 @@
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
--- a/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementUpdateWorkflow.php
@@ -53,60 +53,42 @@
$repository = head($repos);
try {
- $lock_name = 'repository.update:'.$repository->getID();
- $lock = PhabricatorGlobalLock::newLock($lock_name);
-
- 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);
- }
-
- try {
- $no_discovery = $args->getArg('no-discovery');
-
- id(new PhabricatorRepositoryPullEngine())
- ->setRepository($repository)
- ->setVerbose($this->getVerbose())
- ->pullRepository();
+ id(new PhabricatorRepositoryPullEngine())
+ ->setRepository($repository)
+ ->setVerbose($this->getVerbose())
+ ->pullRepository();
- if ($no_discovery) {
- $lock->unlock();
- return;
- }
+ $no_discovery = $args->getArg('no-discovery');
+ if ($no_discovery) {
+ return 0;
+ }
- // 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.
+ // 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.
- $repository->writeStatusMessage(
- PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE,
- null);
+ $repository->writeStatusMessage(
+ PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE,
+ null);
- $this->discoverRepository($repository);
+ $this->discoverRepository($repository);
- $this->checkIfRepositoryIsFullyImported($repository);
+ $this->checkIfRepositoryIsFullyImported($repository);
- $this->updateRepositoryRefs($repository);
+ $this->updateRepositoryRefs($repository);
- $this->mirrorRepository($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 @@
throw $ex;
}
- $lock->unlock();
-
- $console->writeOut(
+ echo tsprintf(
+ "%s\n",
pht(
- 'Updated repository **%s**.',
- $repository->getMonogram())."\n");
+ 'Updated repository "%s".',
+ $repository->getDisplayName()));
return 0;
}

File Metadata

Mime Type
text/plain
Expires
Fri, Nov 15, 9:04 PM (3 d, 11 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6737720
Default Alt Text
D15903.diff (9 KB)

Event Timeline