Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15402377
D21680.id51633.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
D21680.id51633.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
@@ -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,11 @@
hgsprintf('%s', $new_position),
$bookmark_name);
- $restore[$bookmark_name] = $old_position;
+ if ($old_position === null) {
+ $delete_bookmarks[] = $bookmark_name;
+ } else {
+ $restore_bookmarks[$bookmark_name] = $old_position;
+ }
}
}
@@ -961,30 +995,68 @@
$body_commands[] = $argv;
- // Finally, restore the bookmarks.
+ // Finally, clean up the bookmarks.
+
+ // If the onto-marker didn't exist locally when landing it will be pulled
+ // down and created above. To restore the original state these bookmarks
+ // are tracked so we can delete them when done, regardless of whether the
+ // land succeeded.
+ // NOTE: This doesn't end up having working because any calls to
+ // newMarkerRefQuery() prior to this function will have pulled down
+ // remote bookmarks. To properly support this the state of only the
+ // local bookmarks needs to be captured prior to any other marker ref
+ // queries. In practice pulling down the remote markers which were
+ // landed onto and leaving them here is probably fine.
+ if ($delete_bookmarks) {
+ // Delete all bookmarks in a single command rather than individually.
+ $tail = array(
+ 'bookmark',
+ '--delete',
+ '--',
+ );
+ foreach ($delete_bookmarks as $bookmark_name) {
+ $tail[] = $bookmark_name;
+ }
- foreach ($restore as $bookmark_name => $old_position) {
- $tail = array();
- $tail[] = 'bookmark';
+ $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));
+ if ($restore_bookmarks) {
+ // Instead of restoring the previous state, assume landing onto bookmarks
+ // also updates those bookmarks in the remote. After pushing, pull the
+ // latest state of these bookmarks. Mercurial allows pulling multiple
+ // bookmarks in a single pull command which will be faster than pulling
+ // them from a remote individually.
+ $tail = array(
+ 'pull',
+ );
+
+ foreach ($restore_bookmarks as $bookmark_name => $old_position) {
+ $tail[] = '--bookmark';
+ $tail[] = $bookmark_name;
+
+ // In the failure case restore the state of the bookmark. Mercurial
+ // does not provide a way to move multiple bookmarks in a single
+ // command however these commands do not involve the remote.
+ $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 +1092,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;
}
}
@@ -1040,6 +1114,8 @@
$revs = array();
$obsolete_map = array();
+ $using_evolve = $api->getMercurialFeature('evolve');
+
// We've rebased all descendants already, so we can safely delete all
// of these commits.
@@ -1047,10 +1123,17 @@
foreach ($sets as $set) {
$commits = $set->getCommits();
- $min_commit = head($commits)->getHash();
- $max_commit = last($commits)->getHash();
+ $oldest_commit = head($commits)->getHash();
+ $newest_commit = last($commits)->getHash();
- $revs[] = hgsprintf('%s::%s', $min_commit, $max_commit);
+ if ($using_evolve) {
+ // If non-head series of commits are rebased while the evolve extension
+ // is in use, the rebase leaves behind the entire series of descendants
+ // in which case the entire chain needs removed, not just a section.
+ $revs[] = hgsprintf('%s::', $oldest_commit);
+ } else {
+ $revs[] = hgsprintf('%s::%s', $oldest_commit, $newest_commit);
+ }
foreach ($commits as $commit) {
$obsolete_map[$commit->getHash()] = true;
@@ -1071,10 +1154,7 @@
// bookmarks which point at these now-obsoleted commits.
$bookmark_refs = $api->newMarkerRefQuery()
- ->withMarkerTypes(
- array(
- ArcanistMarkerRef::TYPE_BOOKMARK,
- ))
+ ->withMarkerTypes(array(ArcanistMarkerRef::TYPE_BOOKMARK))
->execute();
foreach ($bookmark_refs as $bookmark_ref) {
$bookmark_hash = $bookmark_ref->getCommitHash();
@@ -1093,7 +1173,7 @@
$bookmark_name);
}
- if ($api->getMercurialFeature('evolve')) {
+ if ($using_evolve) {
$api->execxLocal(
'prune --rev %s',
$rev_set);
@@ -1108,7 +1188,54 @@
$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();
+
+ $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_marker = $this->ontoMarkers[0]->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 +1247,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/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php b/src/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php
--- a/src/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php
+++ b/src/query/ArcanistMercurialWorkingCopyRevisionHardpointQuery.php
@@ -22,6 +22,14 @@
// TODO: This has a lot in common with the Git query in the same role.
+ // TODO: In Mercurial if commits are rebased or changed locally they are
+ // given brand new commit hashes, resulting in this query not being
+ // able to identify revisions associated with the commit. This can
+ // happen after landing a non-tip revision in a chain of dependent
+ // revisions. This can be confusing as `arc which` will still be able
+ // to identify the right revision by using the `base` ruleset. Maybe
+ // this could re-use the same `base` ruleset logic as a fallback?
+
$hashes = array();
$map = array();
foreach ($refs as $ref_key => $ref) {
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();
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Tue, Mar 18, 10:14 PM (9 h, 15 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7710450
Default Alt Text
D21680.id51633.diff (13 KB)
Attached To
Mode
D21680: An assortment of fixes and updates to using arc-land with mercurial
Attached
Detach File
Event Timeline
Log In to Comment