@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Make "Land Revision" show merge conflicts more clearly
Summary:
Ref T182. We just show "an error happened" right now. Improve this behavior.
This error handling chain is a bit ad-hoc for now but we can formalize it as we hit other cases.
Test Plan:
{F910247}
{F910248}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T182
Differential Revision: https://secure.phabricator.com/D14343
authored by
epriestley
and committed by
epriestley
0b24a6e2
2326d5f8
+153
-10
+41
-5
src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
+41
-5
src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
···
3
3
final class DrydockWorkingCopyBlueprintImplementation
4
4
extends DrydockBlueprintImplementation {
5
5
6
+
const PHASE_SQUASHMERGE = 'squashmerge';
7
+
6
8
public function isEnabled() {
7
9
return true;
8
10
}
···
288
290
$merges = idx($spec, 'merges');
289
291
if ($merges) {
290
292
foreach ($merges as $merge) {
291
-
$this->applyMerge($interface, $merge);
293
+
$this->applyMerge($lease, $interface, $merge);
292
294
}
293
295
}
294
296
···
416
418
}
417
419
418
420
private function applyMerge(
421
+
DrydockLease $lease,
419
422
DrydockCommandInterface $interface,
420
423
array $merge) {
421
424
···
428
431
$src_ref,
429
432
$src_ref);
430
433
434
+
$command = csprintf(
435
+
'git merge --no-stat --squash --ff-only -- %R',
436
+
$src_ref);
437
+
431
438
try {
432
-
$interface->execx(
433
-
'git merge --no-stat --squash --ff-only -- %s',
434
-
$src_ref);
439
+
$interface->execx('%C', $command);
435
440
} catch (CommandException $ex) {
436
-
// TODO: Specifically note this as a merge conflict.
441
+
$this->setWorkingCopyVCSErrorFromCommandException(
442
+
$lease,
443
+
self::PHASE_SQUASHMERGE,
444
+
$command,
445
+
$ex);
446
+
437
447
throw $ex;
438
448
}
439
449
}
440
450
451
+
protected function setWorkingCopyVCSErrorFromCommandException(
452
+
DrydockLease $lease,
453
+
$phase,
454
+
$command,
455
+
CommandException $ex) {
456
+
457
+
$error = array(
458
+
'phase' => $phase,
459
+
'command' => (string)$command,
460
+
'raw' => (string)$ex->getCommand(),
461
+
'err' => $ex->getError(),
462
+
'stdout' => $ex->getStdout(),
463
+
'stderr' => $ex->getStderr(),
464
+
);
465
+
466
+
$lease->setAttribute('workingcopy.vcs.error', $error);
467
+
}
468
+
469
+
public function getWorkingCopyVCSError(DrydockLease $lease) {
470
+
$error = $lease->getAttribute('workingcopy.vcs.error');
471
+
if (!$error) {
472
+
return null;
473
+
} else {
474
+
return $error;
475
+
}
476
+
}
441
477
442
478
}
+8
src/applications/drydock/storage/DrydockRepositoryOperation.php
+8
src/applications/drydock/storage/DrydockRepositoryOperation.php
···
175
175
return $this->getProperty('exec.leasePHID');
176
176
}
177
177
178
+
public function setWorkingCopyVCSError(array $error) {
179
+
return $this->setProperty('exec.workingcopy.error', $error);
180
+
}
181
+
182
+
public function getWorkingCopyVCSError() {
183
+
return $this->getProperty('exec.workingcopy.error');
184
+
}
185
+
178
186
179
187
180
188
/* -( PhabricatorPolicyInterface )----------------------------------------- */
+94
-2
src/applications/drydock/view/DrydockRepositoryOperationStatusView.php
+94
-2
src/applications/drydock/view/DrydockRepositoryOperationStatusView.php
···
73
73
if ($state != DrydockRepositoryOperation::STATE_FAIL) {
74
74
$item->addAttribute($operation->getOperationCurrentStatus($viewer));
75
75
} else {
76
-
// TODO: Make this more useful.
77
-
$item->addAttribute(pht('Operation encountered an error.'));
76
+
$vcs_error = $operation->getWorkingCopyVCSError();
77
+
if ($vcs_error) {
78
+
switch ($vcs_error['phase']) {
79
+
case DrydockWorkingCopyBlueprintImplementation::PHASE_SQUASHMERGE:
80
+
$message = pht(
81
+
'This change did not merge cleanly. This usually indicates '.
82
+
'that the change is out of date and needs to be updated.');
83
+
break;
84
+
default:
85
+
$message = pht(
86
+
'Operation encountered an error while performing repository '.
87
+
'operations.');
88
+
break;
89
+
}
90
+
91
+
$item->addAttribute($message);
92
+
93
+
$table = $this->renderVCSErrorTable($vcs_error);
94
+
list($links, $info) = $this->renderDetailToggles($table);
95
+
96
+
$item->addAttribute($links);
97
+
$item->appendChild($info);
98
+
} else {
99
+
$item->addAttribute(pht('Operation encountered an error.'));
100
+
}
78
101
}
79
102
80
103
return id(new PHUIObjectItemListView())
81
104
->addItem($item);
105
+
}
106
+
107
+
private function renderVCSErrorTable(array $vcs_error) {
108
+
$rows = array();
109
+
$rows[] = array(pht('Command'), $vcs_error['command']);
110
+
$rows[] = array(pht('Error'), $vcs_error['err']);
111
+
$rows[] = array(pht('Stdout'), $vcs_error['stdout']);
112
+
$rows[] = array(pht('Stderr'), $vcs_error['stderr']);
113
+
114
+
$table = id(new AphrontTableView($rows))
115
+
->setColumnClasses(
116
+
array(
117
+
'header',
118
+
'wide',
119
+
));
120
+
121
+
return $table;
122
+
}
123
+
124
+
private function renderDetailToggles(AphrontTableView $table) {
125
+
$show_id = celerity_generate_unique_node_id();
126
+
$hide_id = celerity_generate_unique_node_id();
127
+
$info_id = celerity_generate_unique_node_id();
128
+
129
+
Javelin::initBehavior('phabricator-reveal-content');
130
+
131
+
$show_details = javelin_tag(
132
+
'a',
133
+
array(
134
+
'id' => $show_id,
135
+
'href' => '#',
136
+
'sigil' => 'reveal-content',
137
+
'mustcapture' => true,
138
+
'meta' => array(
139
+
'hideIDs' => array($show_id),
140
+
'showIDs' => array($hide_id, $info_id),
141
+
),
142
+
),
143
+
pht('Show Details'));
144
+
145
+
$hide_details = javelin_tag(
146
+
'a',
147
+
array(
148
+
'id' => $hide_id,
149
+
'href' => '#',
150
+
'sigil' => 'reveal-content',
151
+
'mustcapture' => true,
152
+
'style' => 'display: none',
153
+
'meta' => array(
154
+
'hideIDs' => array($hide_id, $info_id),
155
+
'showIDs' => array($show_id),
156
+
),
157
+
),
158
+
pht('Hide Details'));
159
+
160
+
$info = javelin_tag(
161
+
'div',
162
+
array(
163
+
'id' => $info_id,
164
+
'style' => 'display: none',
165
+
),
166
+
$table);
167
+
168
+
$links = array(
169
+
$show_details,
170
+
$hide_details,
171
+
);
172
+
173
+
return array($links, $info);
82
174
}
83
175
84
176
}
+10
-3
src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php
+10
-3
src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php
···
90
90
// TODO: This is very similar to leasing in Harbormaster, maybe we can
91
91
// share some of the logic?
92
92
93
+
$working_copy = new DrydockWorkingCopyBlueprintImplementation();
94
+
$working_copy_type = $working_copy->getType();
95
+
93
96
$lease_phid = $operation->getProperty('exec.leasePHID');
94
97
if ($lease_phid) {
95
98
$lease = id(new DrydockLeaseQuery())
···
103
106
$lease_phid));
104
107
}
105
108
} else {
106
-
$working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation())
107
-
->getType();
108
-
109
109
$repository = $operation->getRepository();
110
110
111
111
$allowed_phids = $repository->getAutomationBlueprintPHIDs();
···
138
138
}
139
139
140
140
if (!$lease->isActive()) {
141
+
$vcs_error = $working_copy->getWorkingCopyVCSError($lease);
142
+
if ($vcs_error) {
143
+
$operation
144
+
->setWorkingCopyVCSError($vcs_error)
145
+
->save();
146
+
}
147
+
141
148
throw new PhabricatorWorkerPermanentFailureException(
142
149
pht(
143
150
'Lease "%s" never activated.',