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 @@ -803,8 +803,8 @@ // descendants and the min commit has no ancestors. The min/max terms are // used in a topological sense as chronological terms for commits can be // misleading or incorrect in certain situations. - $max_commit = last($commits)->getHash(); $min_commit = head($commits)->getHash(); + $max_commit = last($commits)->getHash(); $revision_ref = $set->getRevisionRef(); $commit_message = $revision_ref->getCommitMessage(); diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php --- a/src/repository/api/ArcanistMercurialAPI.php +++ b/src/repository/api/ArcanistMercurialAPI.php @@ -658,34 +658,144 @@ public function doCommit($message) { $tmp_file = new TempFile(); Filesystem::writeFile($tmp_file, $message); - $this->execxLocal('commit -l %s', $tmp_file); + $this->execxLocal('commit --logfile %s', $tmp_file); $this->reloadWorkingCopy(); } public function amendCommit($message = null) { + $path_statuses = $this->buildUncommittedStatus(); + if ($message === null) { + if (empty($path_statuses)) { + // If there are changes to the working directory and the message is not + // being changed then there's nothing to amend. + return; + } + $message = $this->getCommitMessage('.'); } $tmp_file = new TempFile(); Filesystem::writeFile($tmp_file, $message); + if ($this->getMercurialFeature('evolve')) { + $this->execxLocal('amend --logfile %s --', $tmp_file); + try { + $this->execxLocal('evolve --all --'); + } catch (CommandException $ex) { + $this->execxLocal('evolve --abort --'); + throw $ex; + } + $this->reloadWorkingCopy(); + return; + } + + // Handling the amending of a non-head changeset without evolve means some + // extra rebasing needs to happen. Rebase the current changeset onto the + // same parent but with a new commit message, then rebase each child branch + // onto the resulting changeset, and strip the original. + + $hg_analyzer = PhutilBinaryAnalyzer::getForBinary('hg'); + if ($hg_analyzer->isMercurialTemplatePnodeAvailable()) { + $template = '{p1.node} {node}'; + } else { + $template = '{p1node} {node}'; + } + + list($current_and_parent) = $this->execxLocal( + 'log --template %s --rev . --', + $template); + $nodes = explode(' ', $current_and_parent); + $parent = $nodes[0]; + $current = $nodes[1]; + + // For some reason including "{children % '{node}'}" in the above template + // unexpectedly prints the current node and not child nodes. If it worked + // this could be a single log invocation. + list($children) = $this->execxLocal( + 'log --template %s --rev %s', + '{node} ', + 'children(.)'); + $child_nodes = array_filter(explode(' ', $children)); + + if (empty($child_nodes)) { + try { + $this->execxLocal('commit --amend --logfile %s --', $tmp_file); + } catch (CommandException $ex) { + if (preg_match('/nothing changed/', $ex->getStdout())) { + // NOTE: Mercurial considers it an error to make a no-op amend. + // Although we generally defer to the underlying VCS to dictate + // behavior, this one seems a little goofy, and we use amend as part + // of various workflows under the assumption that no-op amends are + // fine. If this amend failed because it's a no-op, just continue. + } else { + throw $ex; + } + } + $this->reloadWorkingCopy(); + return; + } + + // If there are file changes which will be amended we need to stash to get + // a clean working directory so we can rebase to make the new commit. + $stash_name = null; + $local_state = $this->newLocalState(); + if (!empty($path_statuses)) { + $stash_name = $local_state->saveStash(); + } + try { + // Mercurial doesn't allow rebasing with a new commit message unless + // the `--collapse` flag is also specified... It shouldn't matter + // since we're only rebasing a single commit here. $this->execxLocal( - 'commit --amend -l %s', + 'rebase --dest %s --rev . --keep --collapse --logfile %s --', + $parent, $tmp_file); } catch (CommandException $ex) { - if (preg_match('/nothing changed/', $ex->getStdout())) { - // NOTE: Mercurial considers it an error to make a no-op amend. Although - // we generally defer to the underlying VCS to dictate behavior, this - // one seems a little goofy, and we use amend as part of various - // workflows under the assumption that no-op amends are fine. If this - // amend failed because it's a no-op, just continue. - } else { - throw $ex; + $this->execxLocal('rebase --abort --'); + + if ($stash_name !== null) { + $local_state->restoreStash($stash_name); + } + + throw $ex; + } + + list($new_commit) = $this->execxLocal( + 'log --rev tip --template %s --', + '{node}'); + + // Restore any stashed changes and amend the new commit + if ($stash_name !== null) { + $this->execxLocal('update --rev %s --', $new_commit); + $local_state->restoreStash($stash_name); + + $this->execxLocal('commit --amend --'); + + // Amending the commit will result in a new hash + list($new_commit) = $this->execxLocal( + 'log --rev tip --template %s --', + '{node}'); + } + + try { + foreach ($child_nodes as $child) { + // descendants(rev) will also include rev itself which is why this + // can't just use a single rebase of descendants($current). + $revset = hgsprintf('descendants(%s)', $child); + $this->execxLocal( + 'rebase --dest %s --rev %s --', + $new_commit, + $revset); } + } catch (CommandException $ex) { + $this->execxLocal('rebase --abort --'); + throw $ex; } + $this->execxLocal('--config extensions.strip= strip %s --', $current); + $this->reloadWorkingCopy(); } 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 @@ -152,9 +152,13 @@ return $commands; } - protected function saveStash() { + public function saveStash() { $api = $this->getRepositoryAPI(); - $log = $this->getWorkflow()->getLogEngine(); + + $log = null; + if ($this->getWorkflow() !== null) { + $log = $this->getWorkflow()->getLogEngine(); + } $stash_ref = sprintf( 'arc-%s', @@ -164,20 +168,25 @@ '--config extensions.shelve= shelve --unknown --name %s --', $stash_ref); - $log->writeStatus( - pht('SHELVE'), - pht('Shelving uncommitted changes from working copy.')); + if ($log !== null) { + $log->writeStatus( + pht('SHELVE'), + pht('Shelving uncommitted changes from working copy.')); + } return $stash_ref; } - protected function restoreStash($stash_ref) { + public function restoreStash($stash_ref) { $api = $this->getRepositoryAPI(); - $log = $this->getWorkflow()->getLogEngine(); - $log->writeStatus( - pht('UNSHELVE'), - pht('Restoring uncommitted changes to working copy.')); + if ($this->getWorkflow() !== null) { + $log = $this->getWorkflow()->getLogEngine(); + + $log->writeStatus( + pht('UNSHELVE'), + pht('Restoring uncommitted changes to working copy.')); + } $api->execxLocal( '--config extensions.shelve= unshelve --keep --name %s --',