Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F13229768
D21686.id51660.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
13 KB
Referenced Files
None
Subscribers
None
D21686.id51660.diff
View Options
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/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
--- a/src/repository/api/ArcanistGitAPI.php
+++ b/src/repository/api/ArcanistGitAPI.php
@@ -826,7 +826,7 @@
return $this;
}
- public function amendCommit($message = null) {
+ public function amendCommit($workflow, $message = null) {
if ($message === null) {
$this->execxLocal('commit --amend --allow-empty -C HEAD');
} else {
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,35 +658,179 @@
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) {
+ public function amendCommit($workflow, $message = null) {
+ $path_statuses = $this->buildUncommittedStatus();
+
if ($message === null) {
+ if (empty($path_statuses)) {
+ // If there are no 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.
+
+
+ // Get the child nodes of the current changeset.
+ list($children) = $this->execxLocal(
+ 'log --template %s --rev %s --',
+ '{node} ',
+ 'children(.)');
+ $child_nodes = array_filter(explode(' ', $children));
+
+ // For a head commit we can simply use `commit --amend` for both new commit
+ // message and amending changes from the working directory.
+ 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;
+ }
+ }
+ } else {
+ $this->amendNonHeadCommit(
+ $workflow, $path_statuses, $child_nodes, $tmp_file);
+ }
+
+ $this->reloadWorkingCopy();
+ }
+
+ /**
+ * Amends a non-head commit with a message and file changes. This strategy
+ * is for Mercurial repositories without the evolve extension.
+ *
+ * 1. Stash away any uncommitted changes, which at this point should only be
+ * changes applied from arc lint, or possibly during the initial prompt
+ * from arc diff whether changes should be amended.
+ * 2. Rebase the current changeset onto the same parent but with the new
+ * commit message, keeping the original commit.
+ * 3. Unstash changes from step 1 onto the new commit.
+ * 4. Amend the unstanshed changes to the new commit.
+ * 5. For each branch from the original commit, rebase onto the new commit,
+ * removing the original branch.
+ * 6. Strip the original commit.
+ *
+ * @param workflow The workflow this amend is being made for.
+ * @param array A map of uncommitted changes, should be the result of
+ * calling `buildUncommittedStatus()`.
+ * @param array The list of child changesets off the original commit.
+ * @param file The file containing the new commit message.
+ */
+ private function amendNonHeadCommit(
+ $workflow, $path_statuses, $child_nodes, $tmp_file) {
+
+ $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];
+
+ // 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()
+ ->setWorkflow($workflow);
+ 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 --');
+
+ // Unstash changes after aborting the rebase and before throwing error.
+ if ($stash_name !== null) {
+ $local_state->restoreStash($stash_name);
}
+
+ throw $ex;
}
- $this->reloadWorkingCopy();
+ 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);
+
+ // Pass in the commit message again, even though the commit has the right
+ // commit message already due to the prior rebase. If we don't specify a
+ // commit message here then the user's editor will suddenly appear and
+ // ruin everything we've done so far.
+ $this->execxLocal('commit --amend --logfile %s --', $tmp_file);
+
+ // 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 --rev %s --',
+ $current);
}
public function getCommitSummary($commit) {
diff --git a/src/repository/api/ArcanistRepositoryAPI.php b/src/repository/api/ArcanistRepositoryAPI.php
--- a/src/repository/api/ArcanistRepositoryAPI.php
+++ b/src/repository/api/ArcanistRepositoryAPI.php
@@ -366,7 +366,7 @@
throw new ArcanistCapabilityNotSupportedException($this);
}
- public function amendCommit($message = null) {
+ public function amendCommit($workflow, $message = null) {
throw new ArcanistCapabilityNotSupportedException($this);
}
diff --git a/src/repository/state/ArcanistGitLocalState.php b/src/repository/state/ArcanistGitLocalState.php
--- a/src/repository/state/ArcanistGitLocalState.php
+++ b/src/repository/state/ArcanistGitLocalState.php
@@ -120,7 +120,7 @@
);
}
- protected function saveStash() {
+ public function saveStash() {
$api = $this->getRepositoryAPI();
// NOTE: We'd prefer to "git stash create" here, because using "push"
@@ -140,7 +140,7 @@
return true;
}
- protected function restoreStash($stash_ref) {
+ public function restoreStash($stash_ref) {
$api = $this->getRepositoryAPI();
$log = $this->getWorkflow()->getLogEngine();
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,7 +152,7 @@
return $commands;
}
- protected function saveStash() {
+ public function saveStash() {
$api = $this->getRepositoryAPI();
$log = $this->getWorkflow()->getLogEngine();
@@ -171,7 +171,7 @@
return $stash_ref;
}
- protected function restoreStash($stash_ref) {
+ public function restoreStash($stash_ref) {
$api = $this->getRepositoryAPI();
$log = $this->getWorkflow()->getLogEngine();
diff --git a/src/repository/state/ArcanistRepositoryLocalState.php b/src/repository/state/ArcanistRepositoryLocalState.php
--- a/src/repository/state/ArcanistRepositoryLocalState.php
+++ b/src/repository/state/ArcanistRepositoryLocalState.php
@@ -192,11 +192,27 @@
return false;
}
- protected function saveStash() {
+ /**
+ * Stash uncommitted changes temporarily. Use {@method:restoreStash()} to
+ * bring these changes back.
+ *
+ * Note that on Git repositories the stash acts as a stack, so saving the
+ * stash must match appropriately to restoring the stash.
+ *
+ * @return wild On Git this returns true, on Mercurial this returns a name
+ * (string) which is a reference to be passed to restoreStash.
+ */
+ public function saveStash() {
throw new PhutilMethodNotImplementedException();
}
- protected function restoreStash($ref) {
+ /**
+ * Restores a previously saved stash via {@method:saveStash()}.
+ *
+ * @param wild On Git this parameter is unused, on Mercurial this should be
+ * the name (string) which was returned from saveStash().
+ */
+ public function restoreStash($ref) {
throw new PhutilMethodNotImplementedException();
}
diff --git a/src/workflow/ArcanistAmendWorkflow.php b/src/workflow/ArcanistAmendWorkflow.php
--- a/src/workflow/ArcanistAmendWorkflow.php
+++ b/src/workflow/ArcanistAmendWorkflow.php
@@ -125,7 +125,7 @@
"%B\n",
$message);
} else {
- $repository_api->amendCommit($message);
+ $repository_api->amendCommit($this, $message);
}
return 0;
diff --git a/src/workflow/ArcanistDiffWorkflow.php b/src/workflow/ArcanistDiffWorkflow.php
--- a/src/workflow/ArcanistDiffWorkflow.php
+++ b/src/workflow/ArcanistDiffWorkflow.php
@@ -542,7 +542,7 @@
$repository_api = $this->getRepositoryAPI();
if ($repository_api->supportsAmend()) {
echo pht('Updating commit message...')."\n";
- $repository_api->amendCommit($revised_message);
+ $repository_api->amendCommit($this, $revised_message);
} else {
echo pht(
'Commit message was not amended. Amending commit message is '.
@@ -1502,7 +1502,7 @@
if ($should_amend) {
$wrote = (rtrim($old_message) != rtrim($template));
if ($wrote) {
- $repository_api->amendCommit($template);
+ $repository_api->amendCommit($this, $template);
$where = pht('commit message');
}
} else {
diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php
--- a/src/workflow/ArcanistLintWorkflow.php
+++ b/src/workflow/ArcanistLintWorkflow.php
@@ -344,7 +344,7 @@
$repository_api->execxLocal('add -u');
}
- $repository_api->amendCommit();
+ $repository_api->amendCommit($this);
} else {
throw new ArcanistUsageException(
pht(
diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php
--- a/src/workflow/ArcanistWorkflow.php
+++ b/src/workflow/ArcanistWorkflow.php
@@ -1217,7 +1217,7 @@
if ($should_commit) {
if ($this->getShouldAmend()) {
$commit = head($api->getLocalCommitInformation());
- $api->amendCommit($commit['message']);
+ $api->amendCommit($this, $commit['message']);
} else if ($api->supportsLocalCommits()) {
$template = sprintf(
"\n\n# %s\n#\n# %s\n#\n",
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Tue, May 21, 7:02 PM (3 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6304984
Default Alt Text
D21686.id51660.diff (13 KB)
Attached To
Mode
D21686: Update "arc diff" to amend non-head commits with Mercurial
Attached
Detach File
Event Timeline
Log In to Comment