Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15386190
D15427.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
8 KB
Referenced Files
None
Subscribers
None
D15427.diff
View Options
diff --git a/src/applications/differential/query/DifferentialDiffQuery.php b/src/applications/differential/query/DifferentialDiffQuery.php
--- a/src/applications/differential/query/DifferentialDiffQuery.php
+++ b/src/applications/differential/query/DifferentialDiffQuery.php
@@ -6,7 +6,9 @@
private $ids;
private $phids;
private $revisionIDs;
+
private $needChangesets = false;
+ private $needProperties;
public function withIDs(array $ids) {
$this->ids = $ids;
@@ -28,19 +30,17 @@
return $this;
}
+ public function needProperties($need_properties) {
+ $this->needProperties = $need_properties;
+ return $this;
+ }
+
+ public function newResultObject() {
+ return new DifferentialDiff();
+ }
+
protected function loadPage() {
- $table = new DifferentialDiff();
- $conn_r = $table->establishConnection('r');
-
- $data = queryfx_all(
- $conn_r,
- 'SELECT * FROM %T %Q %Q %Q',
- $table->getTableName(),
- $this->buildWhereClause($conn_r),
- $this->buildOrderClause($conn_r),
- $this->buildLimitClause($conn_r));
-
- return $table->loadAllFromArray($data);
+ return $this->loadStandardPage($this->newResultObject());
}
protected function willFilterPage(array $diffs) {
@@ -76,6 +76,23 @@
return $diffs;
}
+ protected function didFilterPage(array $diffs) {
+ if ($this->needProperties) {
+ $properties = id(new DifferentialDiffProperty())->loadAllWhere(
+ 'diffID IN (%Ld)',
+ mpull($diffs, 'getID'));
+
+ $properties = mgroup($properties, 'getDiffID');
+ foreach ($diffs as $diff) {
+ $map = idx($properties, $diff->getID(), array());
+ $map = mpull($map, 'getData', 'getName');
+ $diff->attachDiffProperties($map);
+ }
+ }
+
+ return $diffs;
+ }
+
private function loadChangesets(array $diffs) {
id(new DifferentialChangesetQuery())
->setViewer($this->getViewer())
@@ -88,32 +105,31 @@
return $diffs;
}
- protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
- $where = array();
+ protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+ $where = parent::buildWhereClauseParts($conn);
if ($this->ids) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'id IN (%Ld)',
$this->ids);
}
if ($this->phids) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'phid IN (%Ls)',
$this->phids);
}
if ($this->revisionIDs) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'revisionID IN (%Ld)',
$this->revisionIDs);
}
- $where[] = $this->buildPagingClause($conn_r);
- return $this->formatWhereClause($where);
+ return $where;
}
public function getQueryApplicationClass() {
diff --git a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
--- a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
+++ b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
@@ -248,6 +248,29 @@
);
}
+ // Check if this diff was pushed to a staging area.
+ $diff = id(new DifferentialDiffQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($revision->getActiveDiff()->getID()))
+ ->needProperties(true)
+ ->executeOne();
+
+ // Older diffs won't have this property. They may still have been pushed.
+ // At least for now, assume staging changes are present if the property
+ // is missing. This should smooth the transition to the more formal
+ // approach.
+ $has_staging = $diff->hasDiffProperty('arc.staging');
+ if ($has_staging) {
+ $staging = $diff->getProperty('arc.staging');
+ if (!is_array($staging)) {
+ $staging = array();
+ }
+ $status = idx($staging, 'status');
+ if ($status != ArcanistDiffWorkflow::STAGING_PUSHED) {
+ return $this->getBarrierToLandingFromStagingStatus($status);
+ }
+ }
+
// TODO: At some point we should allow installs to give "land reviewed
// code" permission to more users than "push any commit", because it is
// a much less powerful operation. For now, just require push so this
@@ -336,4 +359,85 @@
return null;
}
+ private function getBarrierToLandingFromStagingStatus($status) {
+ switch ($status) {
+ case ArcanistDiffWorkflow::STAGING_USER_SKIP:
+ return array(
+ 'title' => pht('Staging Area Skipped'),
+ 'body' => pht(
+ 'The diff author used the %s flag to skip pushing this change to '.
+ 'staging. Changes must be pushed to staging before they can be '.
+ 'landed from the web.',
+ phutil_tag('tt', array(), '--skip-staging')),
+ );
+ case ArcanistDiffWorkflow::STAGING_DIFF_RAW:
+ return array(
+ 'title' => pht('Raw Diff Source'),
+ 'body' => pht(
+ 'The diff was generated from a raw input source, so the change '.
+ 'could not be pushed to staging. Changes must be pushed to '.
+ 'staging before they can be landed from the web.'),
+ );
+ case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNKNOWN:
+ return array(
+ 'title' => pht('Unknown Repository'),
+ 'body' => pht(
+ 'When the diff was generated, the client was not able to '.
+ 'determine which repository it belonged to, so the change '.
+ 'was not pushed to staging. Changes must be pushed to staging '.
+ 'before they can be landed from the web.'),
+ );
+ case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNAVAILABLE:
+ return array(
+ 'title' => pht('Staging Unavailable'),
+ 'body' => pht(
+ 'When this diff was generated, the server was running an older '.
+ 'version of Phabricator which did not support staging areas, so '.
+ 'the change was not pushed to staging. Changes must be pushed '.
+ 'to staging before they can be landed from the web.'),
+ );
+ case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNSUPPORTED:
+ return array(
+ 'title' => pht('Repository Unsupported'),
+ 'body' => pht(
+ 'When this diff was generated, the server was running an older '.
+ 'version of Phabricator which did not support staging areas for '.
+ 'this version control system, so the chagne was not pushed to '.
+ 'staging. Changes must be pushed to staging before they can be '.
+ 'landed from the web.'),
+ );
+
+ case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNCONFIGURED:
+ return array(
+ 'title' => pht('Repository Unconfigured'),
+ 'body' => pht(
+ 'When this diff was generated, the repository was not configured '.
+ 'with a staging area, so the change was not pushed to staging. '.
+ 'Changes must be pushed to staging before they can be landed '.
+ 'from the web.'),
+ );
+ case ArcanistDiffWorkflow::STAGING_CLIENT_UNSUPPORTED:
+ return array(
+ 'title' => pht('Client Support Unavailable'),
+ 'body' => pht(
+ 'When this diff was generated, the client did not support '.
+ 'staging areas for this version control system, so the change '.
+ 'was not pushed to staging. Changes must be pushed to staging '.
+ 'before they can be landed from the web. Updating the client '.
+ 'may resolve this issue.'),
+ );
+ default:
+ return array(
+ 'title' => pht('Unknown Error'),
+ 'body' => pht(
+ 'When this diff was generated, it was not pushed to staging for '.
+ 'an unknown reason (the status code was "%s"). Changes must be '.
+ 'pushed to staging before they can be landed from the web. '.
+ 'The server may be running an out-of-date version of Phabricator, '.
+ 'and updating may provide more information about this error.',
+ $status),
+ );
+ }
+ }
+
}
diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildLog.php b/src/applications/harbormaster/storage/build/HarbormasterBuildLog.php
--- a/src/applications/harbormaster/storage/build/HarbormasterBuildLog.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuildLog.php
@@ -114,6 +114,8 @@
$this->rope->append($content);
$this->flush();
+
+ return $this;
}
private function flush() {
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sun, Mar 16, 12:09 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7225973
Default Alt Text
D15427.diff (8 KB)
Attached To
Mode
D15427: Improve "Land Revision" errors for issues related to staging areas
Attached
Detach File
Event Timeline
Log In to Comment