Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14031187
D15903.id.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
9 KB
Referenced Files
None
Subscribers
None
D15903.id.diff
View Options
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
Details
Attached
Mime Type
text/plain
Expires
Sun, Nov 10, 9:45 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6737720
Default Alt Text
D15903.id.diff (9 KB)
Attached To
Mode
D15903: Make repository daemon locks more granular and forgiving
Attached
Detach File
Event Timeline
Log In to Comment