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,7 @@ hgsprintf('%s', $new_position), $bookmark_name); - $restore[$bookmark_name] = $old_position; + $restore_bookmarks[$bookmark_name] = $old_position; } } @@ -961,30 +991,58 @@ $body_commands[] = $argv; - // Finally, restore the bookmarks. + // Finally, clean up the bookmarks. + + // Delete all bookmarks in a single command rather than individually. + if ($delete_bookmarks) { + $tail = array( + 'bookmark', + '--delete', + '--', + ); + foreach ($delete_bookmarks as $bookmark_name) { + $tail[] = $bookmark_name; + } - foreach ($restore as $bookmark_name => $old_position) { - $tail = array(); - $tail[] = 'bookmark'; + // Do this regardless of success or failure. + $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)); + // Mercurial allows pulling multiple bookmarks in a single command which + // will be faster if we're landing onto multiple bookmarks. + if ($restore_bookmarks) { + $tail = array( + 'pull', + ); + + foreach ($restore_bookmarks as $bookmark_name => $old_position) { + // Instead of restoring the previous state, assume landing onto + // bookmarks is supposed to be synced with the remote. Pull the latest + // state of these bookmarks. + $tail[] = '--bookmark'; + $tail[] = $bookmark_name; + + // Unable to move multiple bookmarks in a single command, so add + // separate commands for each individual bookmark. + $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 +1078,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; } } @@ -1108,7 +1168,58 @@ $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(); + + $onto_marker = $this->ontoMarkers[0]->getName(); + $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_markers = mpull($this->ontoMarkers, null, '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 +1231,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/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();