Page MenuHomePhabricator

D21680.id51633.diff
No OneTemporary

D21680.id51633.diff

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,6 +6,8 @@
private $ontoBranchMarker;
private $ontoMarkers;
+ private $rebasedActiveCommit;
+
protected function getDefaultSymbols() {
$api = $this->getRepositoryAPI();
$log = $this->getLogEngine();
@@ -698,8 +700,11 @@
$commit_map = array();
foreach ($symbols as $symbol) {
$symbol_commit = $symbol->getCommit();
- $template = '{node}-{parents}-';
+ $template = '{node}-{parents % \'{node} \'}-{desc|firstline}\\n';
+ // The returned array of commits is expected to be ordered by newest
+ // to oldest commits. Use 'reverse()' in the template so the output
+ // is ordered with newest commit as the first line.
if ($into_commit === null) {
list($commits) = $api->execxLocal(
'log --rev %s --template %s --',
@@ -789,8 +794,11 @@
$commits = $set->getCommits();
- $min_commit = last($commits)->getHash();
- $max_commit = head($commits)->getHash();
+ // confirmCommits() reverses the order of the commits as they're ordered
+ // above in selectCommits(), now the head of the list is the oldest and
+ // last is the newest.
+ $newest_commit = last($commits)->getHash();
+ $oldest_commit = head($commits)->getHash();
$revision_ref = $set->getRevisionRef();
$commit_message = $revision_ref->getCommitMessage();
@@ -812,7 +820,7 @@
$argv[] = hgsprintf('%s', $into_commit);
$argv[] = '--rev';
- $argv[] = hgsprintf('%s..%s', $min_commit, $max_commit);
+ $argv[] = hgsprintf('%s..%s', $oldest_commit, $newest_commit);
$argv[] = '--logfile';
$argv[] = '-';
@@ -825,8 +833,9 @@
$future->resolvex();
} catch (CommandException $ex) {
- // TODO
- // $api->execManualLocal('rebase --abort');
+ // Aborting the rebase should restore the same state prior to running the
+ // rebase command.
+ $api->execManualLocal('rebase --abort');
throw $ex;
}
@@ -855,13 +864,21 @@
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;
}
protected function pushChange($into_commit) {
$api = $this->getRepositoryAPI();
- list($head, $body, $tail) = $this->newPushCommands($into_commit);
+ list($head, $body, $tail_pass, $tail_fail) = $this->newPushCommands(
+ $into_commit);
foreach ($head as $command) {
$api->execxLocal('%Ls', $command);
@@ -876,10 +893,20 @@
'Push failed! Fix the error and run "arc land" again.'));
}
}
- } finally {
- foreach ($tail as $command) {
+
+ foreach ($tail_pass as $command) {
+ $api->execxLocal('%Ls', $command);
+ }
+ } catch (Exception $ex) {
+ foreach ($tail_fail as $command) {
$api->execxLocal('%Ls', $command);
}
+ throw $ex;
+ } catch (Throwable $ex) {
+ foreach ($tail_fail as $command) {
+ $api->execxLocal('%Ls', $command);
+ }
+ throw $ex;
}
}
@@ -888,7 +915,8 @@
$head_commands = array();
$body_commands = array();
- $tail_commands = array();
+ $tail_pass_commands = array();
+ $tail_fail_commands = array();
$bookmarks = array();
foreach ($this->ontoMarkers as $onto_marker) {
@@ -902,7 +930,8 @@
// to the merge commit. (There doesn't seem to be any way to specify
// "push commit X as bookmark Y" in Mercurial.)
- $restore = array();
+ $delete_bookmarks = array();
+ $restore_bookmarks = array();
if ($bookmarks) {
$markers = $api->newMarkerRefQuery()
->withNames(mpull($bookmarks, 'getName'))
@@ -917,6 +946,7 @@
$new_position = $into_commit;
if ($old_position === $new_position) {
+ $delete_bookmarks[] = $bookmark_name;
continue;
}
@@ -934,7 +964,11 @@
hgsprintf('%s', $new_position),
$bookmark_name);
- $restore[$bookmark_name] = $old_position;
+ if ($old_position === null) {
+ $delete_bookmarks[] = $bookmark_name;
+ } else {
+ $restore_bookmarks[$bookmark_name] = $old_position;
+ }
}
}
@@ -961,30 +995,68 @@
$body_commands[] = $argv;
- // Finally, restore the bookmarks.
+ // Finally, clean up the bookmarks.
+
+ // If the onto-marker didn't exist locally when landing it will be pulled
+ // down and created above. To restore the original state these bookmarks
+ // are tracked so we can delete them when done, regardless of whether the
+ // land succeeded.
+ // NOTE: This doesn't end up having working because any calls to
+ // newMarkerRefQuery() prior to this function will have pulled down
+ // remote bookmarks. To properly support this the state of only the
+ // local bookmarks needs to be captured prior to any other marker ref
+ // queries. In practice pulling down the remote markers which were
+ // landed onto and leaving them here is probably fine.
+ if ($delete_bookmarks) {
+ // Delete all bookmarks in a single command rather than individually.
+ $tail = array(
+ 'bookmark',
+ '--delete',
+ '--',
+ );
+ foreach ($delete_bookmarks as $bookmark_name) {
+ $tail[] = $bookmark_name;
+ }
- foreach ($restore as $bookmark_name => $old_position) {
- $tail = array();
- $tail[] = 'bookmark';
+ $tail_pass_commands[] = $tail;
+ $tail_fail_commands[] = $tail;
+ }
- if ($old_position === null) {
- $tail[] = '--delete';
- } else {
- $tail[] = '--force';
- $tail[] = '--rev';
- $tail[] = hgsprintf('%s', $api->getDisplayHash($old_position));
+ if ($restore_bookmarks) {
+ // Instead of restoring the previous state, assume landing onto bookmarks
+ // also updates those bookmarks in the remote. After pushing, pull the
+ // latest state of these bookmarks. Mercurial allows pulling multiple
+ // bookmarks in a single pull command which will be faster than pulling
+ // them from a remote individually.
+ $tail = array(
+ 'pull',
+ );
+
+ foreach ($restore_bookmarks as $bookmark_name => $old_position) {
+ $tail[] = '--bookmark';
+ $tail[] = $bookmark_name;
+
+ // In the failure case restore the state of the bookmark. Mercurial
+ // does not provide a way to move multiple bookmarks in a single
+ // command however these commands do not involve the remote.
+ $tail_fail_commands[] = array(
+ 'bookmark',
+ '--force',
+ '--rev',
+ hgsprintf('%s', $api->getDisplayHash($old_position)),
+ );
}
- $tail[] = '--';
- $tail[] = $bookmark_name;
-
- $tail_commands[] = $tail;
+ if ($tail) {
+ $tail_pass_commands[] = $tail;
+ }
}
return array(
$head_commands,
$body_commands,
- $tail_commands,
+ $tail_pass_commands,
+ $tail_fail_commands,
);
}
@@ -1020,7 +1092,9 @@
$child_hash,
$new_commit);
} catch (CommandException $ex) {
- // TODO: Recover state.
+ // Aborting the rebase should restore the same state prior to running
+ // the rebase command.
+ $api->execManualLocal('rebase --abort');
throw $ex;
}
}
@@ -1040,6 +1114,8 @@
$revs = array();
$obsolete_map = array();
+ $using_evolve = $api->getMercurialFeature('evolve');
+
// We've rebased all descendants already, so we can safely delete all
// of these commits.
@@ -1047,10 +1123,17 @@
foreach ($sets as $set) {
$commits = $set->getCommits();
- $min_commit = head($commits)->getHash();
- $max_commit = last($commits)->getHash();
+ $oldest_commit = head($commits)->getHash();
+ $newest_commit = last($commits)->getHash();
- $revs[] = hgsprintf('%s::%s', $min_commit, $max_commit);
+ if ($using_evolve) {
+ // If non-head series of commits are rebased while the evolve extension
+ // is in use, the rebase leaves behind the entire series of descendants
+ // in which case the entire chain needs removed, not just a section.
+ $revs[] = hgsprintf('%s::', $oldest_commit);
+ } else {
+ $revs[] = hgsprintf('%s::%s', $oldest_commit, $newest_commit);
+ }
foreach ($commits as $commit) {
$obsolete_map[$commit->getHash()] = true;
@@ -1071,10 +1154,7 @@
// bookmarks which point at these now-obsoleted commits.
$bookmark_refs = $api->newMarkerRefQuery()
- ->withMarkerTypes(
- array(
- ArcanistMarkerRef::TYPE_BOOKMARK,
- ))
+ ->withMarkerTypes(array(ArcanistMarkerRef::TYPE_BOOKMARK))
->execute();
foreach ($bookmark_refs as $bookmark_ref) {
$bookmark_hash = $bookmark_ref->getCommitHash();
@@ -1093,7 +1173,7 @@
$bookmark_name);
}
- if ($api->getMercurialFeature('evolve')) {
+ if ($using_evolve) {
$api->execxLocal(
'prune --rev %s',
$rev_set);
@@ -1108,7 +1188,54 @@
$into_commit,
ArcanistRepositoryLocalState $state) {
- // TODO: For now, just leave users wherever they ended up.
+ $api = $this->getRepositoryAPI();
+
+ // If the starting working state was not part of land process just update
+ // to that original working state.
+ if (!$this->rebasedActiveCommit) {
+ $update_marker = $this->getLocalState()->getLocalCommit();
+ if ($this->getLocalState()->getLocalBookmark() !== null) {
+ $update_marker = $this->getLocalState()->getLocalBookmark();
+ }
+
+ $api->execxLocal(
+ 'update -- %s',
+ $update_marker);
+
+ $state->discardLocalState();
+ return;
+ }
+
+ // If the working state was landed into multiple destinations then the
+ // resulting working state is ambiguous.
+ if (count($this->ontoMarkers) != 1) {
+ $state->discardLocalState();
+ return;
+ }
+
+ // Get the current state of bookmarks
+ $bookmark_refs = $api->newMarkerRefQuery()
+ ->withMarkerTypes(array(ArcanistMarkerRef::TYPE_BOOKMARK))
+ ->execute();
+
+ $update_marker = $this->rebasedActiveCommit;
+
+ // 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
+ // the destination marker then we use that bookmark as the update in order
+ // for it to become active.
+ $onto_marker = $this->ontoMarkers[0]->getName();
+ foreach ($bookmark_refs as $bookmark_ref) {
+ if ($bookmark_ref->getCommitHash() == $this->rebasedActiveCommit &&
+ $bookmark_ref->getName() == $onto_marker) {
+ $update_marker = $onto_marker;
+ break;
+ }
+ }
+
+ $api->execxLocal(
+ 'update -- %s',
+ $update_marker);
$state->discardLocalState();
}
@@ -1120,8 +1247,9 @@
$message = pht(
'Holding changes locally, they have not been pushed.');
- list($head, $body, $tail) = $this->newPushCommands($into_commit);
- $commands = array_merge($head, $body, $tail);
+ list($head, $body, $tail_pass, $tail_fail) = $this->newPushCommands(
+ $into_commit);
+ $commands = array_merge($head, $body, $tail_pass);
echo tsprintf(
"\n%!\n%s\n\n",
diff --git a/src/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php b/src/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php
--- a/src/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php
+++ b/src/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php
@@ -22,6 +22,14 @@
// TODO: This has a lot in common with the Git query in the same role.
+ // TODO: In Mercurial if commits are rebased or changed locally they are
+ // given brand new commit hashes, resulting in this query not being
+ // able to identify revisions associated with the commit. This can
+ // happen after landing a non-tip revision in a chain of dependent
+ // revisions. This can be confusing as `arc which` will still be able
+ // to identify the right revision by using the `base` ruleset. Maybe
+ // this could re-use the same `base` ruleset logic as a fallback?
+
$hashes = array();
$map = array();
foreach ($refs as $ref_key => $ref) {
diff --git a/src/repository/state/ArcanistMercurialLocalState.php b/src/repository/state/ArcanistMercurialLocalState.php
--- a/src/repository/state/ArcanistMercurialLocalState.php
+++ b/src/repository/state/ArcanistMercurialLocalState.php
@@ -7,6 +7,14 @@
private $localBranch;
private $localBookmark;
+ public function getLocalCommit() {
+ return $this->localCommit;
+ }
+
+ public function getLocalBookmark() {
+ return $this->localBookmark;
+ }
+
protected function executeSaveLocalState() {
$api = $this->getRepositoryAPI();
$log = $this->getWorkflow()->getLogEngine();

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 18, 10:14 PM (9 h, 7 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7710450
Default Alt Text
D21680.id51633.diff (13 KB)

Event Timeline