Page MenuHomePhabricator
Files F14026819

D15420.diff
No OneTemporary
Actions

D15420.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
@@ -886,7 +886,7 @@
'DrydockBlueprintTransactionQuery' => 'applications/drydock/query/DrydockBlueprintTransactionQuery.php',
'DrydockBlueprintViewController' => 'applications/drydock/controller/DrydockBlueprintViewController.php',
'DrydockCommand' => 'applications/drydock/storage/DrydockCommand.php',
- 'DrydockCommandError' => 'applications/drydock/DrydockCommandError/DrydockCommandError.php',
+ 'DrydockCommandError' => 'applications/drydock/exception/DrydockCommandError.php',
'DrydockCommandInterface' => 'applications/drydock/interface/command/DrydockCommandInterface.php',
'DrydockCommandQuery' => 'applications/drydock/query/DrydockCommandQuery.php',
'DrydockConsoleController' => 'applications/drydock/controller/DrydockConsoleController.php',
@@ -5014,6 +5014,7 @@
'DrydockDAO',
'PhabricatorPolicyInterface',
),
+ 'DrydockCommandError' => 'Phobject',
'DrydockCommandInterface' => 'DrydockInterface',
'DrydockCommandQuery' => 'DrydockQuery',
'DrydockConsoleController' => 'DrydockController',
diff --git a/src/applications/drydock/DrydockCommandError/DrydockCommandError.php b/src/applications/drydock/DrydockCommandError/DrydockCommandError.php
deleted file mode 100644
--- a/src/applications/drydock/DrydockCommandError/DrydockCommandError.php
+++ /dev/null
@@ -1,18 +0,0 @@
-<?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;
- }
-}
diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
--- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
@@ -4,6 +4,8 @@
extends DrydockBlueprintImplementation {
const PHASE_SQUASHMERGE = 'squashmerge';
+ const PHASE_REMOTEFETCH = 'blueprint.workingcopy.fetch.remote';
+ const PHASE_MERGEFETCH = 'blueprint.workingcopy.fetch.staging';
public function isEnabled() {
return true;
@@ -240,11 +242,11 @@
$default = null;
foreach ($map as $directory => $spec) {
+ $interface->pushWorkingDirectory("{$root}/repo/{$directory}/");
+
$cmd = array();
$arg = array();
- $interface->pushWorkingDirectory("{$root}/repo/{$directory}/");
-
$cmd[] = 'git clean -d --force';
$cmd[] = 'git fetch';
@@ -266,7 +268,20 @@
$cmd[] = 'git reset --hard origin/%s';
$arg[] = $branch;
- } else if ($ref) {
+ }
+
+ $this->execxv($interface, $cmd, $arg);
+
+ if (idx($spec, 'default')) {
+ $default = $directory;
+ }
+
+ // If we're fetching a ref from a remote, do that separately so we can
+ // raise a more tailored error.
+ if ($ref) {
+ $cmd = array();
+ $arg = array();
+
$ref_uri = $ref['uri'];
$ref_ref = $ref['ref'];
@@ -277,17 +292,25 @@
$cmd[] = 'git checkout %s --';
$arg[] = $ref_ref;
- }
- $cmd = implode(' && ', $cmd);
- $argv = array_merge(array($cmd), $arg);
+ try {
+ $this->execxv($interface, $cmd, $arg);
+ } catch (CommandException $ex) {
+ $display_command = csprintf(
+ 'git fetch %R %R',
+ $ref_uri,
+ $ref_ref);
- $result = call_user_func_array(
- array($interface, 'execx'),
- $argv);
+ $error = DrydockCommandError::newFromCommandException($ex)
+ ->setPhase(self::PHASE_REMOTEFETCH)
+ ->setDisplayCommand($display_command);
- if (idx($spec, 'default')) {
- $default = $directory;
+ $lease->setAttribute(
+ 'workingcopy.vcs.error',
+ $error->toDictionary());
+
+ throw $ex;
+ }
}
$merges = idx($spec, 'merges');
@@ -428,11 +451,29 @@
$src_uri = $merge['src.uri'];
$src_ref = $merge['src.ref'];
- $interface->execx(
- 'git fetch --no-tags -- %s +%s:%s',
- $src_uri,
- $src_ref,
- $src_ref);
+
+ try {
+ $interface->execx(
+ 'git fetch --no-tags -- %s +%s:%s',
+ $src_uri,
+ $src_ref,
+ $src_ref);
+ } catch (CommandException $ex) {
+ $display_command = csprintf(
+ 'git fetch %R +%R:%R',
+ $src_uri,
+ $src_ref,
+ $src_ref);
+
+ $error = DrydockCommandError::newFromCommandException($ex)
+ ->setPhase(self::PHASE_MERGEFETCH)
+ ->setDisplayCommand($display_command);
+
+ $lease->setAttribute('workingcopy.vcs.error', $error->toDictionary());
+
+ throw $ex;
+ }
+
// NOTE: This can never actually generate a commit because we pass
// "--squash", but git sometimes runs code to check that a username and
@@ -443,32 +484,36 @@
'drydock@phabricator',
$src_ref);
- // Show the user a simplified command if the operation fails and we need to
- // report an error.
- $show_command = csprintf(
- 'git merge --squash -- %R',
- $src_ref);
-
try {
$interface->execx('%C', $real_command);
} catch (CommandException $ex) {
- $error = DrydockCommandError::newFromCommandException(
- self::PHASE_SQUASHMERGE,
- $show_command,
- $ex);
+ $display_command = csprintf(
+ 'git merge --squash %R',
+ $src_ref);
+
+ $error = DrydockCommandError::newFromCommandException($ex)
+ ->setPhase(self::PHASE_SQUASHMERGE)
+ ->setDisplayCommand($display_command);
- $lease->setAttribute('workingcopy.vcs.error', $error);
+ $lease->setAttribute('workingcopy.vcs.error', $error->toDictionary());
throw $ex;
}
}
public function getCommandError(DrydockLease $lease) {
- $error = $lease->getAttribute('workingcopy.vcs.error');
- if (!$error) {
- return null;
- } else {
- return $error;
- }
+ return $lease->getAttribute('workingcopy.vcs.error');
}
+ private function execxv(
+ DrydockCommandInterface $interface,
+ array $commands,
+ array $arguments) {
+
+ $commands = implode(' && ', $commands);
+ $argv = array_merge(array($commands), $arguments);
+
+ return call_user_func_array(array($interface, 'execx'), $argv);
+ }
+
+
}
diff --git a/src/applications/drydock/exception/DrydockCommandError.php b/src/applications/drydock/exception/DrydockCommandError.php
new file mode 100644
--- /dev/null
+++ b/src/applications/drydock/exception/DrydockCommandError.php
@@ -0,0 +1,58 @@
+<?php
+
+final class DrydockCommandError extends Phobject {
+
+ private $phase;
+ private $displayCommand;
+ private $command;
+ private $error;
+ private $stdout;
+ private $stderr;
+
+ public static function newFromCommandException(CommandException $ex) {
+ $error = new self();
+
+ $error->command = (string)$ex->getCommand();
+
+ $error->error = $ex->getError();
+ $error->stdout = $ex->getStdout();
+ $error->stderr = $ex->getStderr();
+
+ return $error;
+ }
+
+ public function setPhase($phase) {
+ $this->phase = $phase;
+ return $this;
+ }
+
+ public function getPhase() {
+ return $this->phase;
+ }
+
+ public function setDisplayCommand($display_command) {
+ $this->displayCommand = (string)$display_command;
+ return $this;
+ }
+
+ public function getDisplayCommand() {
+ return $this->displayCommand;
+ }
+
+ public function toDictionary() {
+ $display_command = $this->getDisplayCommand();
+ if ($display_command === null) {
+ $display_command = $this->command;
+ }
+
+ return array(
+ 'phase' => $this->getPhase(),
+ 'command' => $display_command,
+ 'raw' => $this->command,
+ 'err' => $this->error,
+ 'stdout' => $this->stdout,
+ 'stderr' => $this->stderr,
+ );
+ }
+
+}
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
@@ -4,7 +4,9 @@
extends DrydockRepositoryOperationType {
const OPCONST = 'land';
- const PHASE_PUSH = 'push';
+
+ const PHASE_PUSH = 'op.land.push';
+ const PHASE_COMMIT = 'op.land.commit';
public function getOperationDescription(
DrydockRepositoryOperation $operation,
@@ -119,25 +121,42 @@
$committer_info['email'],
"{$author_name} <{$author_email}>");
- $future
- ->write($commit_message)
- ->resolvex();
+ $future->write($commit_message);
try {
- $interface->execx(
- 'git push origin -- %s:%s',
- 'HEAD',
- $push_dst);
+ $future->resolvex();
} 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);
+ $display_command = csprintf('git commit');
+
+ // TODO: One reason this can fail is if the changes have already been
+ // merged. We could try to detect that.
+
+ $error = DrydockCommandError::newFromCommandException($ex)
+ ->setPhase(self::PHASE_COMMIT)
+ ->setDisplayCommand($display_command);
+
+ $operation->setCommandError($error->toDictionary());
+
+ throw $ex;
+ }
+
+ try {
+ $interface->execx(
+ 'git push origin -- %s:%s',
+ 'HEAD',
+ $push_dst);
+ } catch (CommandException $ex) {
+ $display_command = csprintf(
+ 'git push origin %R:%R',
+ 'HEAD',
+ $push_dst);
+
+ $error = DrydockCommandError::newFromCommandException($ex)
+ ->setPhase(self::PHASE_PUSH)
+ ->setDisplayCommand($display_command);
+
+ $operation->setCommandError($error->toDictionary());
+
throw $ex;
}
}
diff --git a/src/applications/drydock/view/DrydockRepositoryOperationStatusView.php b/src/applications/drydock/view/DrydockRepositoryOperationStatusView.php
--- a/src/applications/drydock/view/DrydockRepositoryOperationStatusView.php
+++ b/src/applications/drydock/view/DrydockRepositoryOperationStatusView.php
@@ -82,6 +82,20 @@
'This change did not merge cleanly. This usually indicates '.
'that the change is out of date and needs to be updated.');
break;
+ case DrydockWorkingCopyBlueprintImplementation::PHASE_REMOTEFETCH:
+ $message = pht(
+ 'This change could not be fetched from the remote.');
+ break;
+ case DrydockWorkingCopyBlueprintImplementation::PHASE_MERGEFETCH:
+ $message = pht(
+ 'This change could not be fetched from the remote staging '.
+ 'area. It may not have been pushed, or may have been removed.');
+ break;
+ case DrydockLandRepositoryOperation::PHASE_COMMIT:
+ $message = pht(
+ 'Committing this change failed. It may already have been '.
+ 'merged.');
+ break;
case DrydockLandRepositoryOperation::PHASE_PUSH:
$message = pht(
'The push failed. This usually indicates '.
@@ -123,10 +137,23 @@
private function renderVCSErrorTable(array $vcs_error) {
$rows = array();
- $rows[] = array(pht('Command'), $vcs_error['command']);
+
+ $rows[] = array(
+ pht('Command'),
+ phutil_censor_credentials($vcs_error['command']),
+ );
+
$rows[] = array(pht('Error'), $vcs_error['err']);
- $rows[] = array(pht('Stdout'), $vcs_error['stdout']);
- $rows[] = array(pht('Stderr'), $vcs_error['stderr']);
+
+ $rows[] = array(
+ pht('Stdout'),
+ phutil_censor_credentials($vcs_error['stdout']),
+ );
+
+ $rows[] = array(
+ pht('Stderr'),
+ phutil_censor_credentials($vcs_error['stderr']),
+ );
$table = id(new AphrontTableView($rows))
->setColumnClasses(

File Metadata

Mime Type
text/plain
Expires
Sat, Nov 9, 2:36 AM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6741343
Default Alt Text
D15420.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility