Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13195086
D15902.id38301.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
16 KB
Referenced Files
None
Subscribers
None
D15902.id38301.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
@@ -665,6 +665,7 @@
'DiffusionLintController' => 'applications/diffusion/controller/DiffusionLintController.php',
'DiffusionLintCountQuery' => 'applications/diffusion/query/DiffusionLintCountQuery.php',
'DiffusionLintSaveRunner' => 'applications/diffusion/DiffusionLintSaveRunner.php',
+ 'DiffusionLocalRepositoryFilter' => 'applications/diffusion/data/DiffusionLocalRepositoryFilter.php',
'DiffusionLookSoonConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionLookSoonConduitAPIMethod.php',
'DiffusionLowLevelCommitFieldsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php',
'DiffusionLowLevelCommitQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelCommitQuery.php',
@@ -4890,6 +4891,7 @@
'DiffusionLintController' => 'DiffusionController',
'DiffusionLintCountQuery' => 'PhabricatorQuery',
'DiffusionLintSaveRunner' => 'Phobject',
+ 'DiffusionLocalRepositoryFilter' => 'Phobject',
'DiffusionLookSoonConduitAPIMethod' => 'DiffusionConduitAPIMethod',
'DiffusionLowLevelCommitFieldsQuery' => 'DiffusionLowLevelQuery',
'DiffusionLowLevelCommitQuery' => 'DiffusionLowLevelQuery',
diff --git a/src/applications/diffusion/data/DiffusionLocalRepositoryFilter.php b/src/applications/diffusion/data/DiffusionLocalRepositoryFilter.php
new file mode 100644
--- /dev/null
+++ b/src/applications/diffusion/data/DiffusionLocalRepositoryFilter.php
@@ -0,0 +1,184 @@
+<?php
+
+/**
+ * Filter a list of repositories, removing repositories not local to the
+ * current device.
+ */
+final class DiffusionLocalRepositoryFilter extends Phobject {
+
+ private $viewer;
+ private $device;
+ private $repositories;
+ private $rejectionReasons;
+
+ public function setViewer(PhabricatorUser $viewer) {
+ $this->viewer = $viewer;
+ return $this;
+ }
+
+ public function getViewer() {
+ return $this->viewer;
+ }
+
+ public function setDevice(AlmanacDevice $device = null) {
+ $this->device = $device;
+ return $this;
+ }
+
+ public function getDevice() {
+ return $this->device;
+ }
+
+ public function setRepositories(array $repositories) {
+ $this->repositories = $repositories;
+ return $this;
+ }
+
+ public function getRepositories() {
+ return $this->repositories;
+ }
+
+ public function setRejectionReasons($rejection_reasons) {
+ $this->rejectionReasons = $rejection_reasons;
+ return $this;
+ }
+
+ public function getRejectionReasons() {
+ return $this->rejectionReasons;
+ }
+
+ public function execute() {
+ $repositories = $this->getRepositories();
+ $device = $this->getDevice();
+ $viewer = $this->getViewer();
+
+ $reasons = array();
+
+ $service_phids = array();
+ foreach ($repositories as $key => $repository) {
+ $service_phid = $repository->getAlmanacServicePHID();
+
+ // If the repository is bound to a service but this host is not a
+ // recognized device, or vice versa, don't pull the repository unless
+ // we're sure it's safe because the repository has no local working copy
+ // or the working copy already exists on disk.
+ $is_cluster_repo = (bool)$service_phid;
+ $is_cluster_device = (bool)$device;
+ if ($is_cluster_repo != $is_cluster_device) {
+ $has_working_copy = $repository->hasLocalWorkingCopy();
+ if ($is_cluster_device) {
+ if (!$has_working_copy) {
+ $reasons[$key] = pht(
+ 'Repository "%s" is not a cluster repository, but the current '.
+ 'host is a cluster device ("%s") and updating this repository '.
+ 'would create a new local working copy. This is dangerous, so '.
+ 'the repository will not be updated on this host.',
+ $repository->getDisplayName(),
+ $device->getName());
+ unset($repositories[$key]);
+ continue;
+ }
+ } else {
+ $reasons[$key] = pht(
+ 'Repository "%s" is a cluster repository, but the current host '.
+ 'is not a cluster device (it has no device ID), so the '.
+ 'repository will not be updated on this host.',
+ $repository->getDisplayName());
+ unset($repositories[$key]);
+ continue;
+ }
+ }
+
+ if ($service_phid) {
+ $service_phids[] = $service_phid;
+ }
+ }
+
+ if (!$device) {
+ $this->rejectionReasons = $reasons;
+ return $repositories;
+ }
+
+ $device_phid = $device->getPHID();
+
+ if ($service_phids) {
+ // We could include `withDevicePHIDs()` here to pull a smaller result
+ // set, but we can provide more helpful diagnostic messages below if
+ // we fetch a little more data.
+ $services = id(new AlmanacServiceQuery())
+ ->setViewer($viewer)
+ ->withPHIDs($service_phids)
+ ->withServiceTypes(
+ array(
+ AlmanacClusterRepositoryServiceType::SERVICETYPE,
+ ))
+ ->needBindings(true)
+ ->execute();
+ $services = mpull($services, null, 'getPHID');
+ } else {
+ $services = array();
+ }
+
+ foreach ($repositories as $key => $repository) {
+ $service_phid = $repository->getAlmanacServicePHID();
+
+ if (!$service_phid) {
+ continue;
+ }
+
+ $service = idx($services, $service_phid);
+ if (!$service) {
+ $reasons[$key] = pht(
+ 'Repository "%s" is on cluster service "%s", but that service '.
+ 'could not be loaded, so the repository will not be updated on '.
+ 'this host.',
+ $repository->getDisplayName(),
+ $service_phid);
+ unset($repositories[$key]);
+ continue;
+ }
+
+ $bindings = $service->getBindings();
+ $bindings = mgroup($bindings, 'getDevicePHID');
+ $bindings = idx($bindings, $device_phid);
+ if (!$bindings) {
+ $reasons[$key] = pht(
+ 'Repository "%s" is on cluster service "%s", but that service is '.
+ 'not bound to this device ("%s"), so the repository will not be '.
+ 'updated on this host.',
+ $repository->getDisplayName(),
+ $service->getName(),
+ $device->getName());
+ unset($repositories[$key]);
+ continue;
+ }
+
+ $all_disabled = true;
+ foreach ($bindings as $binding) {
+ if (!$binding->getIsDisabled()) {
+ $all_disabled = false;
+ break;
+ }
+ }
+
+ if ($all_disabled) {
+ $reasons[$key] = pht(
+ 'Repository "%s" is on cluster service "%s", but the binding '.
+ 'between that service and this device ("%s") is disabled, so it '.
+ 'can not be updated on this host.',
+ $repository->getDisplayName(),
+ $service->getName(),
+ $device->getName());
+ unset($repositories[$key]);
+ continue;
+ }
+
+ // We have a valid service that is actively bound to the current host
+ // device, so we're good to go.
+ }
+
+ $this->rejectionReasons = $reasons;
+ return $repositories;
+ }
+
+}
diff --git a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
--- a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
+++ b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
@@ -354,117 +354,17 @@
}
}
- $service_phids = array();
- foreach ($repositories as $key => $repository) {
- $service_phid = $repository->getAlmanacServicePHID();
-
- // If the repository is bound to a service but this host is not a
- // recognized device, or vice versa, don't pull the repository.
- $is_cluster_repo = (bool)$service_phid;
- $is_cluster_device = (bool)$device;
- if ($is_cluster_repo != $is_cluster_device) {
- if ($is_cluster_device) {
- $this->log(
- pht(
- 'Repository "%s" is not a cluster repository, but the current '.
- 'host is a cluster device ("%s"), so the repository will not '.
- 'be updated on this host.',
- $repository->getDisplayName(),
- $device->getName()));
- } else {
- $this->log(
- pht(
- 'Repository "%s" is a cluster repository, but the current '.
- 'host is not a cluster device (it has no device ID), so the '.
- 'repository will not be updated on this host.',
- $repository->getDisplayName()));
- }
- unset($repositories[$key]);
- continue;
- }
-
- if ($service_phid) {
- $service_phids[] = $service_phid;
- }
- }
+ $viewer = $this->getViewer();
- if ($device) {
- $device_phid = $device->getPHID();
-
- if ($service_phids) {
- // We could include `withDevicePHIDs()` here to pull a smaller result
- // set, but we can provide more helpful diagnostic messages below if
- // we fetch a little more data.
- $services = id(new AlmanacServiceQuery())
- ->setViewer($this->getViewer())
- ->withPHIDs($service_phids)
- ->withServiceTypes(
- array(
- AlmanacClusterRepositoryServiceType::SERVICETYPE,
- ))
- ->needBindings(true)
- ->execute();
- $services = mpull($services, null, 'getPHID');
- } else {
- $services = array();
- }
+ $filter = id(new DiffusionLocalRepositoryFilter())
+ ->setViewer($viewer)
+ ->setDevice($device)
+ ->setRepositories($repositories);
- foreach ($repositories as $key => $repository) {
- $service_phid = $repository->getAlmanacServicePHID();
+ $repositories = $filter->execute();
- $service = idx($services, $service_phid);
- if (!$service) {
- $this->log(
- pht(
- 'Repository "%s" is on cluster service "%s", but that service '.
- 'could not be loaded, so the repository will not be updated '.
- 'on this host.',
- $repository->getDisplayName(),
- $service_phid));
- unset($repositories[$key]);
- continue;
- }
-
- $bindings = $service->getBindings();
- $bindings = mgroup($bindings, 'getDevicePHID');
- $bindings = idx($bindings, $device_phid);
- if (!$bindings) {
- $this->log(
- pht(
- 'Repository "%s" is on cluster service "%s", but that service '.
- 'is not bound to this device ("%s"), so the repository will '.
- 'not be updated on this host.',
- $repository->getDisplayName(),
- $service->getName(),
- $device->getName()));
- unset($repositories[$key]);
- continue;
- }
-
- $all_disabled = true;
- foreach ($bindings as $binding) {
- if (!$binding->getIsDisabled()) {
- $all_disabled = false;
- break;
- }
- }
-
- if ($all_disabled) {
- $this->log(
- pht(
- 'Repository "%s" is on cluster service "%s", but the binding '.
- 'between that service and this device ("%s") is disabled, so '.
- 'the not be updated on this host.',
- $repository->getDisplayName(),
- $service->getName(),
- $device->getName()));
- unset($repositories[$key]);
- continue;
- }
-
- // We have a valid service that is actively bound to the current host
- // device, so we're good to go.
- }
+ foreach ($filter->getRejectionReasons() as $reason) {
+ $this->log($reason);
}
// Shuffle the repositories, then re-key the array since shuffle()
diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php
--- a/src/applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php
@@ -27,7 +27,7 @@
}
public function execute(PhutilArgumentParser $args) {
- $repos = $this->loadRepositories($args, 'repos');
+ $repos = $this->loadLocalRepositories($args, 'repos');
if (!$repos) {
throw new PhutilArgumentUsageException(
diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementMirrorWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementMirrorWorkflow.php
--- a/src/applications/repository/management/PhabricatorRepositoryManagementMirrorWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementMirrorWorkflow.php
@@ -23,7 +23,7 @@
}
public function execute(PhutilArgumentParser $args) {
- $repos = $this->loadRepositories($args, 'repos');
+ $repos = $this->loadLocalRepositories($args, 'repos');
if (!$repos) {
throw new PhutilArgumentUsageException(
diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php
--- a/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementPullWorkflow.php
@@ -22,7 +22,7 @@
}
public function execute(PhutilArgumentParser $args) {
- $repos = $this->loadRepositories($args, 'repos');
+ $repos = $this->loadLocalRepositories($args, 'repos');
if (!$repos) {
throw new PhutilArgumentUsageException(
diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php
--- a/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php
@@ -22,7 +22,7 @@
}
public function execute(PhutilArgumentParser $args) {
- $repos = $this->loadRepositories($args, 'repos');
+ $repos = $this->loadLocalRepositories($args, 'repos');
if (!$repos) {
throw new PhutilArgumentUsageException(
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
@@ -44,7 +44,7 @@
$this->setVerbose($args->getArg('verbose'));
$console = PhutilConsole::getConsole();
- $repos = $this->loadRepositories($args, 'repos');
+ $repos = $this->loadLocalRepositories($args, 'repos');
if (count($repos) !== 1) {
throw new PhutilArgumentUsageException(
pht('Specify exactly one repository to update.'));
diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php
--- a/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php
+++ b/src/applications/repository/management/PhabricatorRepositoryManagementWorkflow.php
@@ -33,6 +33,32 @@
return array_values($repositories);
}
+ protected function loadLocalRepositories(
+ PhutilArgumentParser $args,
+ $param) {
+
+ $repositories = $this->loadRepositories($args, $param);
+ if (!$repositories) {
+ return $repositories;
+ }
+
+ $device = AlmanacKeys::getLiveDevice();
+ $viewer = $this->getViewer();
+
+ $filter = id(new DiffusionLocalRepositoryFilter())
+ ->setViewer($viewer)
+ ->setDevice($device)
+ ->setRepositories($repositories);
+
+ $repositories = $filter->execute();
+
+ foreach ($filter->getRejectionReasons() as $reason) {
+ throw new PhutilArgumentUsageException($reason);
+ }
+
+ return $repositories;
+ }
+
protected function loadCommits(PhutilArgumentParser $args, $param) {
$names = $args->getArg($param);
if (!$names) {
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Mon, May 13, 10:07 PM (2 w, 4 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6290532
Default Alt Text
D15902.id38301.diff (16 KB)
Attached To
Mode
D15902: Provide a more consistent, mostly relaxed severity for updating non-cluster repositories on cluster devices
Attached
Detach File
Event Timeline
Log In to Comment