When the push phase of "Land Revision" fails, show the error in the UI

Summary: T10447

Test Plan: tested on my dev instance

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: Korvin, yelirekim

Differential Revision: https://secure.phabricator.com/D15350

src/__phutil_library_map__.php

@@ -27,6 +27,7 @@ phutil_register_library_map(array(
     'AlmanacClusterServiceType' => 'applications/almanac/servicetype/AlmanacClusterServiceType.php',
     'AlmanacConsoleController' => 'applications/almanac/controller/AlmanacConsoleController.php',
     'AlmanacController' => 'applications/almanac/controller/AlmanacController.php',
+    'AlmanacCreateClusterServicesCapability' => 'applications/almanac/capability/AlmanacCreateClusterServicesCapability.php',
     'AlmanacCreateDevicesCapability' => 'applications/almanac/capability/AlmanacCreateDevicesCapability.php',
     'AlmanacCreateNamespacesCapability' => 'applications/almanac/capability/AlmanacCreateNamespacesCapability.php',
     'AlmanacCreateNetworksCapability' => 'applications/almanac/capability/AlmanacCreateNetworksCapability.php',
@@ -57,9 +58,10 @@ phutil_register_library_map(array(
     'AlmanacInterfaceQuery' => 'applications/almanac/query/AlmanacInterfaceQuery.php',
     'AlmanacInterfaceTableView' => 'applications/almanac/view/AlmanacInterfaceTableView.php',
     'AlmanacKeys' => 'applications/almanac/util/AlmanacKeys.php',
-    'AlmanacManageClusterServicesCapability' => 'applications/almanac/capability/AlmanacManageClusterServicesCapability.php',
+    'AlmanacManagementLockWorkflow' => 'applications/almanac/management/AlmanacManagementLockWorkflow.php',
     'AlmanacManagementRegisterWorkflow' => 'applications/almanac/management/AlmanacManagementRegisterWorkflow.php',
     'AlmanacManagementTrustKeyWorkflow' => 'applications/almanac/management/AlmanacManagementTrustKeyWorkflow.php',
+    'AlmanacManagementUnlockWorkflow' => 'applications/almanac/management/AlmanacManagementUnlockWorkflow.php',
     'AlmanacManagementUntrustKeyWorkflow' => 'applications/almanac/management/AlmanacManagementUntrustKeyWorkflow.php',
     'AlmanacManagementWorkflow' => 'applications/almanac/management/AlmanacManagementWorkflow.php',
     'AlmanacNames' => 'applications/almanac/util/AlmanacNames.php',
@@ -971,6 +973,7 @@ phutil_register_library_map(array(
     'DrydockWebrootInterface' => 'applications/drydock/interface/webroot/DrydockWebrootInterface.php',
     'DrydockWorker' => 'applications/drydock/worker/DrydockWorker.php',
     'DrydockWorkingCopyBlueprintImplementation' => 'applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php',
+    'DrydockCommandError' => 'applications/drydock/DrydockCommandError/DrydockCommandError.php',
     'FeedConduitAPIMethod' => 'applications/feed/conduit/FeedConduitAPIMethod.php',
     'FeedPublishConduitAPIMethod' => 'applications/feed/conduit/FeedPublishConduitAPIMethod.php',
     'FeedPublisherHTTPWorker' => 'applications/feed/worker/FeedPublisherHTTPWorker.php',
@@ -3994,7 +3997,6 @@ phutil_register_library_map(array(
       'PhabricatorApplicationTransactionInterface',
       'AlmanacPropertyInterface',
       'PhabricatorDestructibleInterface',
-      'PhabricatorExtendedPolicyInterface',
     ),
     'AlmanacBindingDisableController' => 'AlmanacServiceController',
     'AlmanacBindingEditController' => 'AlmanacServiceController',
@@ -4012,6 +4014,7 @@ phutil_register_library_map(array(
     'AlmanacClusterServiceType' => 'AlmanacServiceType',
     'AlmanacConsoleController' => 'AlmanacController',
     'AlmanacController' => 'PhabricatorController',
+    'AlmanacCreateClusterServicesCapability' => 'PhabricatorPolicyCapability',
     'AlmanacCreateDevicesCapability' => 'PhabricatorPolicyCapability',
     'AlmanacCreateNamespacesCapability' => 'PhabricatorPolicyCapability',
     'AlmanacCreateNetworksCapability' => 'PhabricatorPolicyCapability',
@@ -4028,7 +4031,6 @@ phutil_register_library_map(array(
       'PhabricatorDestructibleInterface',
       'PhabricatorNgramsInterface',
       'PhabricatorConduitResultInterface',
-      'PhabricatorExtendedPolicyInterface',
     ),
     'AlmanacDeviceController' => 'AlmanacController',
     'AlmanacDeviceEditController' => 'AlmanacDeviceController',
@@ -4058,9 +4060,10 @@ phutil_register_library_map(array(
     'AlmanacInterfaceQuery' => 'AlmanacQuery',
     'AlmanacInterfaceTableView' => 'AphrontView',
     'AlmanacKeys' => 'Phobject',
-    'AlmanacManageClusterServicesCapability' => 'PhabricatorPolicyCapability',
+    'AlmanacManagementLockWorkflow' => 'AlmanacManagementWorkflow',
     'AlmanacManagementRegisterWorkflow' => 'AlmanacManagementWorkflow',
     'AlmanacManagementTrustKeyWorkflow' => 'AlmanacManagementWorkflow',
+    'AlmanacManagementUnlockWorkflow' => 'AlmanacManagementWorkflow',
     'AlmanacManagementUntrustKeyWorkflow' => 'AlmanacManagementWorkflow',
     'AlmanacManagementWorkflow' => 'PhabricatorManagementWorkflow',
     'AlmanacNames' => 'Phobject',
@@ -4128,7 +4131,6 @@ phutil_register_library_map(array(
       'PhabricatorDestructibleInterface',
       'PhabricatorNgramsInterface',
       'PhabricatorConduitResultInterface',
-      'PhabricatorExtendedPolicyInterface',
     ),
     'AlmanacServiceController' => 'AlmanacController',
     'AlmanacServiceDatasource' => 'PhabricatorTypeaheadDatasource',

src/applications/drydock/DrydockCommandError/DrydockCommandError.php

@@ -0,0 +1,18 @@
+<?php
+
+class DrydockCommandError {
+  public static function newFromCommandException(
+    $phase,
+    $command,
+    CommandException $ex) {
+    $error = array(
+      'phase' => $phase,
+      'command' => (string)$command,
+      'raw' => (string)$ex->getCommand(),
+      'err' => $ex->getError(),
+      'stdout' => $ex->getStdout(),
+      'stderr' => $ex->getStderr(),
+    );
+    return $error;
+  }
+}

src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php

@@ -448,35 +448,17 @@ final class DrydockWorkingCopyBlueprintImplementation
     try {
       $interface->execx('%C', $real_command);
     } catch (CommandException $ex) {
-      $this->setWorkingCopyVCSErrorFromCommandException(
-        $lease,
+      $error = DrydockCommandError::newFromCommandException(
         self::PHASE_SQUASHMERGE,
         $show_command,
         $ex);
 
+      $lease->setAttribute('workingcopy.vcs.error', $error);
       throw $ex;
     }
   }
 
-  protected function setWorkingCopyVCSErrorFromCommandException(
-    DrydockLease $lease,
-    $phase,
-    $command,
-    CommandException $ex) {
-
-    $error = array(
-      'phase' => $phase,
-      'command' => (string)$command,
-      'raw' => (string)$ex->getCommand(),
-      'err' => $ex->getError(),
-      'stdout' => $ex->getStdout(),
-      'stderr' => $ex->getStderr(),
-    );
-
-    $lease->setAttribute('workingcopy.vcs.error', $error);
-  }
-
-  public function getWorkingCopyVCSError(DrydockLease $lease) {
+  public function getCommandError(DrydockLease $lease) {
     $error = $lease->getAttribute('workingcopy.vcs.error');
     if (!$error) {
       return null;

src/applications/drydock/operation/DrydockLandRepositoryOperation.php

@@ -4,6 +4,7 @@ final class DrydockLandRepositoryOperation
   extends DrydockRepositoryOperationType {
 
   const OPCONST = 'land';
+  const PHASE_PUSH = 'push';
 
   public function getOperationDescription(
     DrydockRepositoryOperation $operation,
@@ -122,10 +123,23 @@ final class DrydockLandRepositoryOperation
       ->write($commit_message)
       ->resolvex();
 
-    $interface->execx(
+    try {
+        $interface->execx(
+          'git push origin -- %s:%s',
+          'HEAD',
+          $push_dst);
+    } catch (CommandException $ex) {
+      $show_command = csprintf(
       'git push origin -- %s:%s',
       'HEAD',
       $push_dst);
+      $error = DrydockCommandError::newFromCommandException(
+        self::PHASE_PUSH,
+        $show_command,
+        $ex);
+      $operation->setCommandError($error);
+      throw $ex;
+    }
   }
 
   private function getCommitterInfo(DrydockRepositoryOperation $operation) {

src/applications/drydock/storage/DrydockRepositoryOperation.php

@@ -178,11 +178,11 @@ final class DrydockRepositoryOperation extends DrydockDAO
     return $this->getProperty('exec.leasePHID');
   }
 
-  public function setWorkingCopyVCSError(array $error) {
+  public function setCommandError(array $error) {
     return $this->setProperty('exec.workingcopy.error', $error);
   }
 
-  public function getWorkingCopyVCSError() {
+  public function getCommandError() {
     return $this->getProperty('exec.workingcopy.error');
   }
 

src/applications/drydock/view/DrydockRepositoryOperationStatusView.php

@@ -74,7 +74,7 @@ final class DrydockRepositoryOperationStatusView
     if ($state != DrydockRepositoryOperation::STATE_FAIL) {
       $item->addAttribute($operation->getOperationCurrentStatus($viewer));
     } else {
-      $vcs_error = $operation->getWorkingCopyVCSError();
+      $vcs_error = $operation->getCommandError();
       if ($vcs_error) {
         switch ($vcs_error['phase']) {
           case DrydockWorkingCopyBlueprintImplementation::PHASE_SQUASHMERGE:
@@ -82,6 +82,12 @@ final class DrydockRepositoryOperationStatusView
               'This change did not merge cleanly. This usually indicates '.
               'that the change is out of date and needs to be updated.');
             break;
+          case DrydockLandRepositoryOperation::PHASE_PUSH:
+            $message = pht(
+              'The push failed. This usually indicates '.
+              'that the change is breaking some rules or '.
+              'some custom commit hook has failed.');
+            break;
           default:
             $message = pht(
               'Operation encountered an error while performing repository '.

src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php

@@ -138,10 +138,10 @@ final class DrydockRepositoryOperationUpdateWorker
     }
 
     if (!$lease->isActive()) {
-      $vcs_error = $working_copy->getWorkingCopyVCSError($lease);
+      $vcs_error = $working_copy->getCommandError($lease);
       if ($vcs_error) {
         $operation
-          ->setWorkingCopyVCSError($vcs_error)
+          ->setCommandError($vcs_error)
           ->save();
       }