Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15421873
D21686.id51661.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.id51661.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,175 @@
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;
+ }
+
+ // 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 new 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 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 the changes from step 1 onto the new commit.
+ * 4. Amend the unstashed changes to the new commit.
+ * 5. For each branch from the original commit, rebase onto the new commit,
+ * removing the original branch. Note that there is potential for this to
+ * cause a conflict but this is something the user has to address.
+ * 6. Strip the original commit.
+ *
+ * @param workflow The workflow this amend is being made for.
+ * @param array A map of uncommitted file changes. This hould 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 first need to stash
+ // them away 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 worked so hard to set up here.
+ $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 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,28 @@
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 references the stash that was made. This name
+ * should later be passed to {@method:restoreStash()}.
+ */
+ public function saveStash() {
throw new PhutilMethodNotImplementedException();
}
- protected function restoreStash($ref) {
+ /**
+ * Restores changes that were previously stashed by {@method:saveStash()}.
+ *
+ * @param wild On Git this parameter is unused, on Mercurial this should be
+ * the name (string) returned from {@method: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
Sun, Mar 23, 2:53 AM (8 h, 49 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7704767
Default Alt Text
D21686.id51661.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