mirror of
https://we.phorge.it/source/arcanist.git
synced 2024-12-23 05:50:54 +01:00
Make arc patch create a bookmark by default in hg
Summary: Previously, arc patch would create a new commit under the existing current bookmark in mercurial. There have been two discussions about what the right behavior should be (D3334 and D3658). One side wants no commit at all, and one side wants a commit under a new bookmark. The current implementation is the worst of both worlds :( This change makes it create a new bookmark at the revision's base before commiting, same as the --bookmark flag used to do (which is now obsolete). That way the existing bookmark doesn't move (in mercurial >=1.8). This is the same behavior git has, which is convienent for groups migrating between the two. Also makes hg's getCanonicalRevision handle svn revisions just like git. This way arc patch will try to apply the patch to the appropriate revision in the history. Test Plan: Ran: arc patch - Verified it created a new bookmark and commited on top of the revision's base commit. arc patch --nobranch - Verified it put the new commit on top of the current bookmark without a new bookmark. arc patch --nocommit - Verified it left all the changes in the working copy. Also verified arc patch still works with git. Reviewers: epriestley Reviewed By: epriestley CC: sid0, bos, dschleimer, aran, Korvin Differential Revision: https://secure.phabricator.com/D5408
This commit is contained in:
parent
2900ab6abd
commit
ba78000b38
2 changed files with 100 additions and 80 deletions
|
@ -61,6 +61,12 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
|
|||
}
|
||||
|
||||
public function getCanonicalRevisionName($string) {
|
||||
$match = null;
|
||||
if ($this->isHgSubversionRepo() &&
|
||||
preg_match('/@([0-9]+)$/', $string, $match)) {
|
||||
$string = hgsprintf('svnrev(%s)', $match[1]);
|
||||
}
|
||||
|
||||
list($stdout) = $this->execxLocal(
|
||||
'log -l 1 --template %s -r %s --',
|
||||
'{node}',
|
||||
|
|
|
@ -95,26 +95,13 @@ EOTEXT
|
|||
),
|
||||
'nobranch' => array(
|
||||
'supports' => array(
|
||||
'git'
|
||||
'git', 'hg'
|
||||
),
|
||||
'help' =>
|
||||
"Normally under git, a new branch is created and then the patch ".
|
||||
"is applied and committed in the new branch. This flag ".
|
||||
"cherry-picks the resultant commit onto the original branch and ".
|
||||
"deletes the temporary branch.",
|
||||
'conflicts' => array(
|
||||
'update' => true,
|
||||
),
|
||||
),
|
||||
'bookmark' => array(
|
||||
'supports' => array(
|
||||
'hg'
|
||||
),
|
||||
'help' =>
|
||||
"Normally under hg, a new bookmark is not created and the patch ".
|
||||
"is applied and committed in the current bookmark. With this flag, ".
|
||||
"a new bookmark is created and the patch is applied and committed ".
|
||||
"in the new bookmark.",
|
||||
"Normally, a new branch (git) or bookmark (hg) is created and then ".
|
||||
"the patch is applied and committed in the new branch/bookmark. ".
|
||||
"This flag cherry-picks the resultant commit onto the original ".
|
||||
"branch and deletes the temporary branch.",
|
||||
'conflicts' => array(
|
||||
'update' => true,
|
||||
),
|
||||
|
@ -208,12 +195,9 @@ EOTEXT
|
|||
}
|
||||
|
||||
private function canBranch() {
|
||||
// git only for now
|
||||
$repository_api = $this->getRepositoryAPI();
|
||||
if (!($repository_api instanceof ArcanistGitAPI)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
return ($repository_api instanceof ArcanistGitAPI) ||
|
||||
($repository_api instanceof ArcanistMercurialAPI);
|
||||
}
|
||||
|
||||
private function shouldBranch() {
|
||||
|
@ -224,22 +208,6 @@ EOTEXT
|
|||
return true;
|
||||
}
|
||||
|
||||
private function shouldBookmark() {
|
||||
// specific to hg
|
||||
$repository_api = $this->getRepositoryAPI();
|
||||
if (!($repository_api instanceof ArcanistMercurialAPI)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$bookmark = $this->getArgument('bookmark', false);
|
||||
if ($bookmark) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
private function getBranchName(ArcanistBundle $bundle) {
|
||||
$branch_name = null;
|
||||
$repository_api = $this->getRepositoryAPI();
|
||||
|
@ -322,54 +290,70 @@ EOTEXT
|
|||
$repository_api = $this->getRepositoryAPI();
|
||||
|
||||
// verify the base revision is valid
|
||||
// in a working copy that uses the git-svn bridge, the base revision might
|
||||
// be a svn uri instead of a git ref
|
||||
if ($repository_api instanceof ArcanistGitAPI) {
|
||||
// in a working copy that uses the git-svn bridge, the base revision might
|
||||
// be a svn uri instead of a git ref
|
||||
|
||||
// NOTE: Use 'cat-file', not 'rev-parse --verify', because 'rev-parse'
|
||||
// always "verifies" any properly-formatted commit even if it does not
|
||||
// exist.
|
||||
list($err) = $repository_api->execManualLocal(
|
||||
'cat-file -t %s',
|
||||
$base_revision);
|
||||
return !$err;
|
||||
// NOTE: Use 'cat-file', not 'rev-parse --verify', because 'rev-parse'
|
||||
// always "verifies" any properly-formatted commit even if it does not
|
||||
// exist.
|
||||
list($err) = $repository_api->execManualLocal(
|
||||
'cat-file -t %s',
|
||||
$base_revision);
|
||||
return !$err;
|
||||
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
||||
return $repository_api->hasLocalCommit($base_revision);
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
private function createBranch(ArcanistBundle $bundle, $has_base_revision) {
|
||||
$branch_name = $this->getBranchName($bundle);
|
||||
$base_revision = $bundle->getBaseRevision();
|
||||
$repository_api = $this->getRepositoryAPI();
|
||||
if ($repository_api instanceof ArcanistGitAPI) {
|
||||
$branch_name = $this->getBranchName($bundle);
|
||||
$base_revision = $bundle->getBaseRevision();
|
||||
|
||||
if ($base_revision && $has_base_revision) {
|
||||
$repository_api->execxLocal(
|
||||
'checkout -b %s %s',
|
||||
$branch_name,
|
||||
$base_revision);
|
||||
} else {
|
||||
$repository_api->execxLocal(
|
||||
'checkout -b %s',
|
||||
$branch_name);
|
||||
}
|
||||
|
||||
echo phutil_console_format(
|
||||
"Created and checked out branch %s.\n",
|
||||
$branch_name);
|
||||
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
||||
$branch_name = $this->getBookmarkName($bundle);
|
||||
$base_revision = $bundle->getBaseRevision();
|
||||
|
||||
if ($base_revision && $has_base_revision) {
|
||||
$base_revision = $repository_api->getCanonicalRevisionName(
|
||||
$base_revision);
|
||||
|
||||
echo "Updating to the revision's base commit\n";
|
||||
$repository_api->execPassthru(
|
||||
'update %s',
|
||||
$base_revision);
|
||||
}
|
||||
|
||||
if ($base_revision && $has_base_revision) {
|
||||
$repository_api->execxLocal(
|
||||
'checkout -b %s %s',
|
||||
$branch_name,
|
||||
$base_revision);
|
||||
} else {
|
||||
$repository_api->execxLocal(
|
||||
'checkout -b %s',
|
||||
'bookmark %s',
|
||||
$branch_name);
|
||||
|
||||
echo phutil_console_format(
|
||||
"Created and checked out bookmark %s.\n",
|
||||
$branch_name);
|
||||
}
|
||||
|
||||
echo phutil_console_format(
|
||||
"Created and checked out branch %s.\n",
|
||||
$branch_name);
|
||||
|
||||
return $branch_name;
|
||||
}
|
||||
|
||||
private function createBookmark(ArcanistBundle $bundle) {
|
||||
$bookmark_name = $this->getBookmarkName($bundle);
|
||||
$repository_api = $this->getRepositoryAPI();
|
||||
|
||||
$repository_api->execxLocal(
|
||||
'bookmark %s',
|
||||
$bookmark_name);
|
||||
|
||||
echo phutil_console_format(
|
||||
"Created and applied bookmark %s.\n",
|
||||
$bookmark_name);
|
||||
}
|
||||
|
||||
private function shouldUpdateWorkingCopy() {
|
||||
return $this->getArgument('update', false);
|
||||
}
|
||||
|
@ -458,17 +442,23 @@ EOTEXT
|
|||
$this->canBranch() &&
|
||||
($this->shouldBranch() || $has_base_revision)) {
|
||||
|
||||
$original_branch = $repository_api->getBranchName();
|
||||
if ($repository_api instanceof ArcanistGitAPI) {
|
||||
$original_branch = $repository_api->getBranchName();
|
||||
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
||||
$original_branch = $repository_api->getActiveBookmark();
|
||||
}
|
||||
|
||||
// If we weren't on a branch, then record the ref we'll return to
|
||||
// instead.
|
||||
if ($original_branch === null) {
|
||||
$original_branch = $repository_api->getCanonicalRevisionName('HEAD');
|
||||
if ($repository_api instanceof ArcanistGitAPI) {
|
||||
$original_branch = $repository_api->getCanonicalRevisionName('HEAD');
|
||||
} else if ($repository_api instanceof ArcanistMercurialAPI) {
|
||||
$original_branch = $repository_api->getCanonicalRevisionName('.');
|
||||
}
|
||||
}
|
||||
$new_branch = $this->createBranch($bundle, $has_base_revision);
|
||||
}
|
||||
|
||||
if ($this->shouldBookmark()) {
|
||||
$this->createBookmark($bundle);
|
||||
$new_branch = $this->createBranch($bundle, $has_base_revision);
|
||||
}
|
||||
|
||||
if ($repository_api instanceof ArcanistSubversionAPI) {
|
||||
|
@ -777,10 +767,34 @@ EOTEXT
|
|||
$author_cmd);
|
||||
$future->write($commit_message);
|
||||
$future->resolvex();
|
||||
|
||||
if (!$this->shouldBranch() && $has_base_revision) {
|
||||
$original_rev = $repository_api->getCanonicalRevisionName(
|
||||
$original_branch);
|
||||
$current_parent = $repository_api->getCanonicalRevisionName(
|
||||
hgsprintf('%s^', $new_branch));
|
||||
|
||||
$err = 0;
|
||||
if ($original_rev != $current_parent) {
|
||||
list($err) = $repository_api->execManualLocal(
|
||||
'rebase --dest %s --rev %s',
|
||||
hgsprintf('%s', $original_branch),
|
||||
hgsprintf('%s', $new_branch));
|
||||
}
|
||||
|
||||
$repository_api->execxLocal('bookmark --delete %s', $new_branch);
|
||||
if ($err) {
|
||||
$repository_api->execManualLocal('rebase --abort');
|
||||
throw new ArcanistUsageException(phutil_console_format(
|
||||
"\n<bg:red>** Rebase onto $original_branch failed!**</bg>\n"));
|
||||
}
|
||||
}
|
||||
|
||||
$verb = 'committed';
|
||||
} else {
|
||||
$verb = 'applied';
|
||||
}
|
||||
|
||||
echo phutil_console_format(
|
||||
"<bg:green>** OKAY **</bg> Successfully {$verb} patch.\n");
|
||||
|
||||
|
|
Loading…
Reference in a new issue