Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13187394
D21724.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
10 KB
Referenced Files
None
Subscribers
None
D21724.diff
View Options
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
@@ -1257,53 +1257,55 @@
$should_push = $is_last;
}
- if ($should_push) {
- try {
- $this->pushChange($into_commit);
- $this->setHasUnpushedChanges(false);
- } catch (ArcanistLandPushFailureException $ex) {
-
- // TODO: If the push fails, fetch and retry if the remote ref
- // has moved ahead of us.
-
- if ($this->getIntoLocal()) {
- $can_retry = false;
- } else if ($this->getIntoEmpty()) {
- $can_retry = false;
- } else if ($this->getIntoRemote() !== $this->getOntoRemote()) {
- $can_retry = false;
- } else {
- $can_retry = false;
- }
-
- if ($can_retry) {
- // New commit state here
- $into_commit = '..';
- continue;
- }
-
- throw new PhutilArgumentUsageException(
- $ex->getMessage());
+ if ($should_push === false) {
+ break;
+ }
+
+ try {
+ $this->pushChange($into_commit);
+ $this->setHasUnpushedChanges(false);
+ } catch (ArcanistLandPushFailureException $ex) {
+
+ // TODO: If the push fails, fetch and retry if the remote ref
+ // has moved ahead of us.
+
+ if ($this->getIntoLocal()) {
+ $can_retry = false;
+ } else if ($this->getIntoEmpty()) {
+ $can_retry = false;
+ } else if ($this->getIntoRemote() !== $this->getOntoRemote()) {
+ $can_retry = false;
+ } else {
+ $can_retry = false;
+ }
+
+ if ($can_retry) {
+ // New commit state here
+ $into_commit = '..';
+ continue;
}
- if ($need_cascade) {
+ throw new PhutilArgumentUsageException(
+ $ex->getMessage());
+ }
- // NOTE: We cascade each set we've pushed, but we're going to
- // cascade them from most recent to least recent. This way,
- // branches which descend from more recent changes only cascade
- // once, directly in to the correct state.
+ if ($need_cascade) {
- $need_cascade = array_reverse($need_cascade);
- foreach ($need_cascade as $cascade_set) {
- $this->cascadeState($set, $into_commit);
- }
- $need_cascade = array();
- }
+ // NOTE: We cascade each set we've pushed, but we're going to
+ // cascade them from most recent to least recent. This way,
+ // branches which descend from more recent changes only cascade
+ // once, directly in to the correct state.
- if (!$is_keep) {
- $this->pruneBranches($need_prune);
- $need_prune = array();
+ $need_cascade = array_reverse($need_cascade);
+ foreach ($need_cascade as $cascade_set) {
+ $this->cascadeState($cascade_set, $into_commit);
}
+ $need_cascade = array();
+ }
+
+ if (!$is_keep) {
+ $this->pruneBranches($need_prune);
+ $need_prune = array();
}
break;
diff --git a/src/land/engine/ArcanistMercurialLandEngine.php b/src/land/engine/ArcanistMercurialLandEngine.php
--- a/src/land/engine/ArcanistMercurialLandEngine.php
+++ b/src/land/engine/ArcanistMercurialLandEngine.php
@@ -6,7 +6,9 @@
private $ontoBranchMarker;
private $ontoMarkers;
- private $rebasedActiveCommit;
+ // During merge this populates the original commits being landed mapped to
+ // the new rebased/squashed commit.
+ private $rebasedCommitMap = array();
protected function getDefaultSymbols() {
$api = $this->getRepositoryAPI();
@@ -771,8 +773,12 @@
protected function executeMerge(ArcanistLandCommitSet $set, $into_commit) {
$api = $this->getRepositoryAPI();
+ $log = $this->getLogEngine();
if ($this->getStrategy() !== 'squash') {
+ $log->writeError(
+ pht('TODO'),
+ pht('Merge strategy is not yet supported on Mercurial repositories.'));
throw new Exception(pht('TODO: Support merge strategies'));
}
@@ -783,10 +789,7 @@
// we continue.
$bookmark_refs = $api->newMarkerRefQuery()
- ->withMarkerTypes(
- array(
- ArcanistMarkerRef::TYPE_BOOKMARK,
- ))
+ ->withMarkerTypes(array(ArcanistMarkerRef::TYPE_BOOKMARK))
->execute();
// TODO: Add a Mercurial version check requiring 2.1.1 or newer.
@@ -842,6 +845,14 @@
$future->resolvex();
} catch (CommandException $ex) {
+ $log->writeError(
+ pht('REBASE CONFLICT'),
+ pht(
+ 'Commits for %s do not rebase cleanly onto %s. Manually rebase and '.
+ 'resolve conflicts before landing again.',
+ $revision_ref->getRefDisplayName(),
+ $api->getDisplayHash($into_commit)));
+
// Aborting the rebase should restore the same state prior to running the
// rebase command.
$api->execManualLocalWithExtension(
@@ -850,19 +861,21 @@
throw $ex;
}
- // Find all the bookmarks which pointed at commits we just rebased, and
- // put them back the way they were before rebasing moved them. We aren't
- // deleting the old commits yet and don't want to move the bookmarks.
+ list($stdout) = $api->execxLocal('log --rev tip --template %s', '{node}');
+ $new_cursor = trim($stdout);
- $obsolete_map = array();
foreach ($set->getCommits() as $commit) {
- $obsolete_map[$commit->getHash()] = true;
+ $this->rebasedCommitMap[$commit->getHash()] = $new_cursor;
}
+ // Find all the bookmarks which pointed at commits we just rebased, and
+ // put them back the way they were before rebasing moved them. We aren't
+ // deleting the old commits yet and don't want to move the bookmarks.
+
foreach ($bookmark_refs as $bookmark_ref) {
$bookmark_hash = $bookmark_ref->getCommitHash();
- if (!isset($obsolete_map[$bookmark_hash])) {
+ if (!isset($this->rebasedCommitMap[$bookmark_hash])) {
continue;
}
@@ -872,16 +885,6 @@
$bookmark_ref->getName());
}
- list($stdout) = $api->execxLocal('log --rev tip --template %s', '{node}');
- $new_cursor = trim($stdout);
-
- // If any of the commits that were rebased was the active commit before the
- // workflow started, track the new commit so it can be used as the working
- // directory after the land has succeeded.
- if (isset($obsolete_map[$this->getLocalState()->getLocalCommit()])) {
- $this->rebasedActiveCommit = $new_cursor;
- }
-
return $new_cursor;
}
@@ -1053,6 +1056,12 @@
$old_commit = last($set->getCommits())->getHash();
$new_commit = $into_commit;
+ // Rebase the child commits onto the commit which $old_commit was rebased
+ // into. The $into_commit will be the last set's final rebased commit but
+ // this set may be an intermediate set that was landed.
+ if (isset($this->rebasedCommitMap[$old_commit])) {
+ $new_commit = $this->rebasedCommitMap[$old_commit];
+ }
list($output) = $api->execxLocal(
'log --rev %s --template %s',
@@ -1065,8 +1074,11 @@
continue;
}
- // TODO: If the only heads which are descendants of this child will
- // be deleted, we can skip this rebase?
+ // If this child was rebased as part of landing then it shouldn't be
+ // rebased now. Doing so will result in rebase conflicts.
+ if (isset($this->rebasedCommitMap[$child_hash])) {
+ continue;
+ }
try {
$api->execxLocalWithExtension(
@@ -1075,6 +1087,14 @@
$child_hash,
$new_commit);
} catch (CommandException $ex) {
+ $log->writeError(
+ pht('REBASE CONFLICT'),
+ pht(
+ 'Failed to rebase %s cleanly onto %s. Manually rebase and '.
+ 'resolve conflicts.',
+ $api->getDisplayHash($child_hash),
+ $api->getDisplayHash($new_commit)));
+
// Aborting the rebase should restore the same state prior to running
// the rebase command.
$api->execManualLocalWithExtension(
@@ -1181,10 +1201,11 @@
$api = $this->getRepositoryAPI();
- // If the starting working state was not part of land process just update
- // to that original working state.
- if ($this->rebasedActiveCommit === null) {
- $update_marker = $this->getLocalState()->getLocalCommit();
+ // If the previoius working directory was not part of land process just
+ // update to that original working directory.
+ $prev_local_commit = $this->getLocalState()->getLocalCommit();
+ if (!isset($this->rebasedCommitMap[$prev_local_commit])) {
+ $update_marker = $prev_local_commit;
if ($this->getLocalState()->getLocalBookmark() !== null) {
$update_marker = $this->getLocalState()->getLocalBookmark();
}
@@ -1209,7 +1230,9 @@
->withMarkerTypes(array(ArcanistMarkerRef::TYPE_BOOKMARK))
->execute();
- $update_marker = $this->rebasedActiveCommit;
+ // Resolve the previous working directory to the new rebased/squashed
+ // commit, to make that the new working directory.
+ $update_marker = $this->rebasedCommitMap[$prev_local_commit];
// Find any bookmarks which exist on the commit which is the result of the
// starting working directory's rebase. If any of those bookmarks are also
@@ -1217,7 +1240,7 @@
// for it to become active.
$onto_marker = $this->ontoMarkers[0]->getName();
foreach ($bookmark_refs as $bookmark_ref) {
- if ($bookmark_ref->getCommitHash() == $this->rebasedActiveCommit &&
+ if ($bookmark_ref->getCommitHash() == $update_marker &&
$bookmark_ref->getName() == $onto_marker) {
$update_marker = $onto_marker;
break;
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sun, May 12, 4:30 AM (3 w, 19 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6288977
Default Alt Text
D21724.diff (10 KB)
Attached To
Mode
D21724: Update Mercurial's cascading of commit sets to rebase non-landed commits
Attached
Detach File
Event Timeline
Log In to Comment