Page MenuHomePhabricator

D21339.id50802.diff
No OneTemporary

D21339.id50802.diff

diff --git a/src/land/engine/ArcanistGitLandEngine.php b/src/land/engine/ArcanistGitLandEngine.php
--- a/src/land/engine/ArcanistGitLandEngine.php
+++ b/src/land/engine/ArcanistGitLandEngine.php
@@ -38,7 +38,7 @@
$log->writeStatus(
pht('CLEANUP'),
- pht('Destroying branch "%s". To recover, run:', $branch_name));
+ pht('Cleaning up branch "%s". To recover, run:', $branch_name));
echo tsprintf(
"\n **$** %s\n\n",
@@ -238,11 +238,13 @@
$into_commit = $api->writeRawCommit($empty_commit);
}
- $api->execxLocal('checkout %s --', $into_commit);
-
$commits = $set->getCommits();
+
+ $min_commit = head($commits);
+ $min_hash = $min_commit->getHash();
+
$max_commit = last($commits);
- $source_commit = $max_commit->getHash();
+ $max_hash = $max_commit->getHash();
// NOTE: See T11435 for some history. See PHI1727 for a case where a user
// modified their working copy while running "arc land". This attempts to
@@ -252,7 +254,7 @@
list($changes) = $api->execxLocal(
'diff --no-ext-diff %s..%s --',
$into_commit,
- $source_commit);
+ $max_hash);
$changes = trim($changes);
if (!strlen($changes)) {
@@ -263,7 +265,7 @@
pht(
'Merging local "%s" into "%s" produces an empty diff. '.
'This usually means these changes have already landed.',
- $this->getDisplayHash($source_commit),
+ $this->getDisplayHash($max_hash),
$this->getDisplayHash($into_commit)));
}
@@ -271,7 +273,7 @@
pht('MERGING'),
pht(
'%s %s',
- $this->getDisplayHash($source_commit),
+ $this->getDisplayHash($max_hash),
$max_commit->getDisplaySummary()));
$argv = array();
@@ -294,25 +296,44 @@
}
$argv[] = '--';
- $argv[] = $source_commit;
+ $is_rebasing = false;
+ $is_merging = false;
try {
- $api->execxLocal('merge %Ls', $argv);
- } catch (CommandException $ex) {
+ if ($this->isSquashStrategy() && !$is_empty) {
+ // If we're performing a squash merge, we're going to rebase the
+ // commit range first. We only want to merge the specific commits
+ // in the range, and merging too much can create conflicts.
- // TODO: If we previously succeeded with at least one merge, we could
- // provide a hint that "--incremental" can do some of the work.
+ $api->execxLocal('checkout %s --', $max_hash);
- $api->execManualLocal('merge --abort');
- $api->execManualLocal('reset --hard HEAD --');
+ $is_rebasing = true;
+ $api->execxLocal(
+ 'rebase --onto %s -- %s',
+ $into_commit,
+ $min_hash.'^');
+ $is_rebasing = false;
+ $merge_hash = $api->getCanonicalRevisionName('HEAD');
+ } else {
+ $merge_hash = $max_hash;
+ }
+
+ $api->execxLocal('checkout %s --', $into_commit);
+
+ $argv[] = $merge_hash;
+
+ $is_merging = true;
+ $api->execxLocal('merge %Ls', $argv);
+ $is_merging = false;
+ } catch (CommandException $ex) {
$direct_symbols = $max_commit->getDirectSymbols();
$indirect_symbols = $max_commit->getIndirectSymbols();
if ($direct_symbols) {
$message = pht(
'Local commit "%s" (%s) does not merge cleanly into "%s". '.
'Merge or rebase local changes so they can merge cleanly.',
- $this->getDisplayHash($source_commit),
+ $this->getDisplayHash($max_hash),
$this->getDisplaySymbols($direct_symbols),
$this->getDisplayHash($into_commit));
} else if ($indirect_symbols) {
@@ -320,22 +341,47 @@
'Local commit "%s" (reachable from: %s) does not merge cleanly '.
'into "%s". Merge or rebase local changes so they can merge '.
'cleanly.',
- $this->getDisplayHash($source_commit),
+ $this->getDisplayHash($max_hash),
$this->getDisplaySymbols($indirect_symbols),
$this->getDisplayHash($into_commit));
} else {
$message = pht(
'Local commit "%s" does not merge cleanly into "%s". Merge or '.
'rebase local changes so they can merge cleanly.',
- $this->getDisplayHash($source_commit),
+ $this->getDisplayHash($max_hash),
$this->getDisplayHash($into_commit));
}
- throw new PhutilArgumentUsageException($message);
+ echo tsprintf(
+ "\n%!\n%W\n\n",
+ pht('MERGE CONFLICT'),
+ $message);
+
+ if ($this->getHasUnpushedChanges()) {
+ echo tsprintf(
+ "%?\n\n",
+ pht(
+ 'Use "--incremental" to merge and push changes one by one.'));
+ }
+
+ if ($is_rebasing) {
+ $api->execManualLocal('rebase --abort');
+ }
+
+ if ($is_merging) {
+ $api->execManualLocal('merge --abort');
+ }
+
+ if ($is_merging || $is_rebasing) {
+ $api->execManualLocal('reset --hard HEAD --');
+ }
+
+ throw new PhutilArgumentUsageException(
+ pht('Encountered a merge conflict.'));
}
list($original_author, $original_date) = $this->getAuthorAndDate(
- $source_commit);
+ $max_hash);
$revision_ref = $set->getRevisionRef();
$commit_message = $revision_ref->getCommitMessage();
@@ -362,7 +408,7 @@
if ($this->isSquashStrategy()) {
$raw_commit->setParents(array());
} else {
- $raw_commit->setParents(array($source_commit));
+ $raw_commit->setParents(array($merge_hash));
}
$new_cursor = $api->writeRawCommit($raw_commit);
diff --git a/src/land/engine/ArcanistLandEngine.php b/src/land/engine/ArcanistLandEngine.php
--- a/src/land/engine/ArcanistLandEngine.php
+++ b/src/land/engine/ArcanistLandEngine.php
@@ -28,6 +28,7 @@
private $intoLocal;
private $localState;
+ private $hasUnpushedChanges;
final public function setOntoRemote($onto_remote) {
$this->ontoRemote = $onto_remote;
@@ -228,6 +229,15 @@
return $this->localState;
}
+ private function setHasUnpushedChanges($unpushed) {
+ $this->hasUnpushedChanges = $unpushed;
+ return $this;
+ }
+
+ final protected function getHasUnpushedChanges() {
+ return $this->hasUnpushedChanges;
+ }
+
final protected function getOntoConfigurationKey() {
return 'arc.land.onto';
}
@@ -1228,6 +1238,7 @@
while (true) {
$into_commit = $this->executeMerge($set, $into_commit);
+ $this->setHasUnpushedChanges(true);
if ($is_hold) {
$should_push = false;
@@ -1241,6 +1252,7 @@
if ($should_push) {
try {
$this->pushChange($into_commit);
+ $this->setHasUnpushedChanges(false);
} catch (Exception $ex) {
// TODO: If the push fails, fetch and retry if the remote ref

File Metadata

Mime Type
text/plain
Expires
Mon, Apr 14, 10:38 AM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7385034
Default Alt Text
D21339.id50802.diff (6 KB)

Event Timeline