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 @@ -825,8 +825,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; } @@ -861,7 +862,8 @@ 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 +878,15 @@ '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; } } @@ -888,7 +895,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) { @@ -961,30 +969,76 @@ $body_commands[] = $argv; - // Finally, restore the bookmarks. + // Finally, clean up the bookmarks. - foreach ($restore as $bookmark_name => $old_position) { - $tail = array(); - $tail[] = 'bookmark'; + // Delete all bookmarks in a single command rather than individually. + $did_delete_bookmarks = false; + $delete_bookmark_tail = array(); + $delete_bookmark_tail[] = 'bookmark'; + $delete_bookmark_tail[] = '--delete'; + $delete_bookmark_tail[] = '--'; + // Mercurial allows pulling multiple bookmarks in a single command which + // will be faster if we're landing onto multiple bookmarks. + $did_pull_bookmarks = false; + $pull_bookmarks_tail = array(); + $pull_bookmarks_tail[] = 'pull'; + + foreach ($restore as $bookmark_name => $old_position) { if ($old_position === null) { - $tail[] = '--delete'; + // No record of this bookmark's previous position (how?), delete it. + $did_delete_bookmarks = true; + $delete_bookmark_tail[] = $bookmark_name; } else { - $tail[] = '--force'; - $tail[] = '--rev'; - $tail[] = hgsprintf('%s', $api->getDisplayHash($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. + $did_pull_bookmarks = true; + $pull_bookmarks_tail[] = '--bookmark'; + $pull_bookmarks_tail[] = $bookmark_name; + + // Unable to restore multiple bookmarks in a single command, so add + // separate commands for each individual bookmark. + $restore_bookmarks_tail = array( + 'bookmark', + '--force', + '--rev', + hgsprintf('%s', $api->getDisplayHash($old_position)), + ); + + $tail_fail_commands[] = $restore_bookmarks_tail; } + } - $tail[] = '--'; - $tail[] = $bookmark_name; + // For any bookmark which doesn't have an old position, delete the bookmark + // in both successful and failed push scenarios. + if ($did_delete_bookmarks) { + $tail_pass_commands[] = $delete_bookmark_tail; + $tail_fail_commands[] = $delete_bookmark_tail; + } - $tail_commands[] = $tail; + // Only pull if the push succeeded, the restore commands will only be run + // if the push failed. + if ($did_pull_bookmarks) { + $tail_pass_commands[] = $pull_bookmarks_tail; } + // TODO: After pulling, any bookmarks that did not advance should be + // restored back to their original commits. Unsure how to determine + // "bookmarks that did not advance". Maybe another marker ref query + // and if the new state has bookmarks pointing to $into_commit then + // restore to the value set in $restore? + + // TODO: If current working directory is on a $this->ontoMarker or one of + // the landed changes, then update working directory to one of the + // (first?) $this->ontoMarker. + + return array( $head_commands, $body_commands, - $tail_commands, + $tail_pass_commands, + $tail_fail_commands, ); } @@ -1020,7 +1074,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; } } @@ -1120,8 +1176,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",