From b0cfe9d94d38f0ae7f2b3dedc4f7290a9a000069 Mon Sep 17 00:00:00 2001 From: katherine Date: Sat, 18 Aug 2012 11:23:56 -0700 Subject: [PATCH] [hg - arc patch] make mercurial bookmark with arc patch Summary: w00t! Created new functions getBookmarkName, createBookmark that use mercurial commands to create a new bookmark and apply the commit message when running arc patch. Like with git, it checks if the bookmark already exists. If it does, creates arcpatch-DXXX-1, -2 etc Updated the mercurial section of run() to include applying the commit message. Pretty new to programming in general still, so I wasn't sure if I should have just modified getBranchName and createBranch to work with Mercurial (instead of creating new functions). This was easier for me to make sure I didn't break the git functionality. Let me know I can merge the functions. Test Plan: Tested in my www-hg repository. Probably need to test further (suggestions?) Ran the following trials with arc patch D539987 1) the bookmark arcpatch-D539987 already existed 2) the bookmark did not exist 3) tried an invalid commit, arc patch R539987 4) --nobookmark flag (bookmark was not created, but the commit applied) 5) --nocommit flag (boomark was created, and the commit was not applied) 6) --nocommit and --nobokmark Here's the summary of the results: https://secure.phabricator.com/P493 Reviewers: dschleimer Reviewed By: dschleimer CC: aran, epriestley, phleet, bos, csilvers Differential Revision: https://secure.phabricator.com/D3334 --- src/workflow/ArcanistPatchWorkflow.php | 133 +++++++++++++++++++++++-- 1 file changed, 124 insertions(+), 9 deletions(-) diff --git a/src/workflow/ArcanistPatchWorkflow.php b/src/workflow/ArcanistPatchWorkflow.php index 19f4d5b4..22a9fa25 100644 --- a/src/workflow/ArcanistPatchWorkflow.php +++ b/src/workflow/ArcanistPatchWorkflow.php @@ -59,7 +59,7 @@ EOTEXT 'help' => "Apply changes from a Differential revision, using the most recent ". "diff that has been attached to it. You can run 'arc patch D12345' ". - "as a shorthand for this.", + "as a shorthand.", ), 'diff' => array( 'param' => 'diff_id', @@ -94,29 +94,43 @@ EOTEXT "Update the local working copy before applying the patch.", 'conflicts' => array( 'nobranch' => true, + 'bookmark' => true, ), ), 'nocommit' => array( 'supports' => array( - 'git' + 'git', 'hg' ), 'help' => - "Normally under git if the patch is successful the changes are ". - "committed to the working copy. This flag prevents the commit.", + "Normally under git/hg, if the patch is successful, the changes ". + "are committed to the working copy. This flag prevents the commit.", ), 'nobranch' => array( 'supports' => array( 'git' ), 'help' => - "Normally under git a new branch is created and then the patch ". - "is applied and committed in the branch. This flag skips the ". + "Normally under git, a new branch is created and then the patch ". + "is applied and committed in the new branch. This flag skips the ". "branch creation step and applies and commits the patch to the ". "current 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.", + 'conflicts' => array( + 'update' => true, + ), + ), 'force' => array( 'help' => "Do not run any sanity checks.", @@ -220,6 +234,22 @@ 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(); @@ -259,6 +289,45 @@ EOTEXT return $branch_name; } + private function getBookmarkName(ArcanistBundle $bundle) { + $bookmark_name = null; + $repository_api = $this->getRepositoryAPI(); + $revision_id = $bundle->getRevisionID(); + $base_name = "arcpatch"; + if ($revision_id) { + $base_name .= "-D{$revision_id}"; + } + + $suffixes = array(null, '-1', '-2', '-3'); + foreach ($suffixes as $suffix) { + $proposed_name = $base_name.$suffix; + + list($err) = $repository_api->execManualLocal( + 'log -r %s', + $proposed_name); + + // no error means hg log found a bookmark + if (!$err) { + echo phutil_console_format( + "Bookmark name {$proposed_name} already exists; trying a new name.\n" + ); + continue; + } else { + $bookmark_name = $proposed_name; + break; + } + } + + if (!$bookmark_name) { + throw new Exception( + "Arc was unable to automagically make a name for this patch. ". + "Please clean up your working copy and try again." + ); + } + + return $bookmark_name; + } + private function createBranch(ArcanistBundle $bundle) { $branch_name = $this->getBranchName($bundle); $repository_api = $this->getRepositoryAPI(); @@ -291,6 +360,19 @@ EOTEXT $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); } @@ -376,6 +458,10 @@ EOTEXT $this->createBranch($bundle); } + if ($this->shouldBookmark()) { + $this->createBookmark($bundle); + } + $repository_api = $this->getRepositoryAPI(); if ($repository_api instanceof ArcanistSubversionAPI) { $patch_err = 0; @@ -593,7 +679,7 @@ EOTEXT phutil_console_format( "\n** WARNING ** This patch may have failed ". "because it attempts to change the case of a filename (for ". - "instance, from 'example.c' to 'Example.c'). Git can not apply ". + "instance, from 'example.c' to 'Example.c'). Git cannot apply ". "patches like this on case-insensitive filesystems. You must ". "apply this patch manually.\n")); } @@ -613,13 +699,42 @@ EOTEXT echo phutil_console_format( "** OKAY ** Successfully {$verb} patch.\n"); } else if ($repository_api instanceof ArcanistMercurialAPI) { + $future = $repository_api->execFutureLocal( 'import --no-commit -'); $future->write($bundle->toGitPatch()); - $future->resolvex(); + try { + $future->resolvex(); + } catch (CommandException $ex) { + echo phutil_console_format( + "\n** Patch Failed! **\n"); + $stderr = $ex->getStdErr(); + if (preg_match('/case-folding collision/', $stderr)) { + echo phutil_console_wrap( + phutil_console_format( + "\n** WARNING ** This patch may have failed ". + "because it attempts to change the case of a filename (for ". + "instance, from 'example.c' to 'Example.c'). Mercurial cannot ". + "apply patches like this on case-insensitive filesystems. You ". + "must apply this patch manually.\n")); + } + throw $ex; + } + + if ($this->shouldCommit()) { + $commit_message = $this->getCommitMessage($bundle); + $future = $repository_api->execFutureLocal( + 'commit -A -l -'); + $future->write($commit_message); + $future->resolvex(); + $verb = 'committed'; + } else { + $verb = 'applied'; + } echo phutil_console_format( - "** OKAY ** Successfully applied patch.\n"); + "** OKAY ** Successfully {$verb} patch.\n"); + } else { throw new Exception('Unknown version control system.'); }