From 51cccfd21e034a75fb178144b82ce6fce3cae961 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 17 Jan 2012 15:47:00 -0800 Subject: [PATCH] Add "nocommit" git-only flag to arc patch workflow Summary: without "nocommit" we commit the patch to the working copy. add the nocommit flag and this commit does not happen. making this happen required adding revisionID to arc bundle to fetch the proper commit message. if we can't get a commit message -- suppose the fetch fails or the source is self::SOURCE_PATCH, we ask the user for the commit message on the command line Test Plan: git reset --hard arc patch DX, where this got us to observe correct patch committed with correct commit message in working copy git reset --hard arc patch --nocommit DX, where this got us to observe correct patch landed BUT NOT committed. note "commit message" does not exist since there isn't a commit...! git diff HEAD^1 > ~/file.patch git reset --hard HEAD^1 arc patch --patch ~/file.patch observe prompted for commit message and patch committed with commit message i typed in Reviewers: epriestley Reviewed By: epriestley CC: aran Maniphest Tasks: T479 Differential Revision: https://secure.phabricator.com/D1450 --- src/parser/bundle/ArcanistBundle.php | 24 ++++-- src/workflow/base/ArcanistBaseWorkflow.php | 1 + .../export/ArcanistExportWorkflow.php | 1 + src/workflow/patch/ArcanistPatchWorkflow.php | 82 ++++++++++++++++++- src/workflow/patch/__init__.php | 1 + 5 files changed, 102 insertions(+), 7 deletions(-) diff --git a/src/parser/bundle/ArcanistBundle.php b/src/parser/bundle/ArcanistBundle.php index 1599e4eb..5acc8603 100644 --- a/src/parser/bundle/ArcanistBundle.php +++ b/src/parser/bundle/ArcanistBundle.php @@ -29,6 +29,7 @@ class ArcanistBundle { private $diskPath; private $projectID; private $baseRevision; + private $revisionID; public function setConduit(ConduitClient $conduit) { $this->conduit = $conduit; @@ -50,6 +51,15 @@ class ArcanistBundle { return $this->baseRevision; } + public function setRevisionID($revision_id) { + $this->revisionID = $revision_id; + return $this; + } + + public function getRevisionID() { + return $this->revisionID; + } + public static function newFromChanges(array $changes) { $obj = new ArcanistBundle(); $obj->changes = $changes; @@ -72,14 +82,16 @@ class ArcanistBundle { 'tar xfO %s meta.json', $path)); $meta_info = $future->resolveJSON(); - $version = idx($meta_info, 'version', 0); - $project_name = idx($meta_info, 'projectName'); + $version = idx($meta_info, 'version', 0); + $project_name = idx($meta_info, 'projectName'); $base_revision = idx($meta_info, 'baseRevision'); + $revision_id = idx($meta_info, 'revisionID'); // this arc bundle was probably made before we started storing meta info } else { - $version = 0; - $project_name = null; + $version = 0; + $project_name = null; $base_revision = null; + $revision_id = null; } $future = new ExecFuture( @@ -104,6 +116,7 @@ class ArcanistBundle { $obj->diskPath = $path; $obj->setProjectID($project_name); $obj->setBaseRevision($base_revision); + $obj->setRevisionID($revision_id); return $obj; } @@ -151,9 +164,10 @@ class ArcanistBundle { } $meta_info = array( - 'version' => 2, + 'version' => 3, 'projectName' => $this->getProjectID(), 'baseRevision' => $this->getBaseRevision(), + 'revisionID' => $this->getRevisionID(), ); $dir = Filesystem::createTemporaryDirectory(); diff --git a/src/workflow/base/ArcanistBaseWorkflow.php b/src/workflow/base/ArcanistBaseWorkflow.php index 1400263a..2960505d 100644 --- a/src/workflow/base/ArcanistBaseWorkflow.php +++ b/src/workflow/base/ArcanistBaseWorkflow.php @@ -753,6 +753,7 @@ class ArcanistBaseWorkflow { $bundle->setConduit($conduit); $bundle->setProjectID($diff['projectName']); $bundle->setBaseRevision($diff['sourceControlBaseRevision']); + $bundle->setRevisionID($diff['revisionID']); return $bundle; } diff --git a/src/workflow/export/ArcanistExportWorkflow.php b/src/workflow/export/ArcanistExportWorkflow.php index ff9b5daf..9b1ce4e8 100644 --- a/src/workflow/export/ArcanistExportWorkflow.php +++ b/src/workflow/export/ArcanistExportWorkflow.php @@ -189,6 +189,7 @@ EOTEXT $bundle->setProjectID($this->getWorkingCopy()->getProjectID()); $bundle->setBaseRevision( $repository_api->getSourceControlBaseRevision()); + // note we can't get a revision ID for SOURCE_LOCAL break; case self::SOURCE_REVISION: $bundle = $this->loadRevisionBundleFromConduit( diff --git a/src/workflow/patch/ArcanistPatchWorkflow.php b/src/workflow/patch/ArcanistPatchWorkflow.php index 76cd249e..9cae0b2b 100644 --- a/src/workflow/patch/ArcanistPatchWorkflow.php +++ b/src/workflow/patch/ArcanistPatchWorkflow.php @@ -38,7 +38,7 @@ final class ArcanistPatchWorkflow extends ArcanistBaseWorkflow { **patch** __--diff__ __diff_id__ **patch** __--patch__ __file__ **patch** __--arcbundle__ __bundlefile__ - Supports: git, svn + Supports: git, svn, hg Apply the changes in a Differential revision, patchfile, or arc bundle to the working copy. EOTEXT @@ -75,6 +75,14 @@ EOTEXT 'help' => "Apply changes from a git patchfile or unified patchfile.", ), + 'nocommit' => array( + 'supports' => array( + 'git' + ), + 'help' => + "Normally under git if the patch is successful the changes are ". + "committed to the working copy. This flag prevents the commit.", + ), 'force' => array( 'help' => "Do not run any sanity checks.", @@ -154,6 +162,15 @@ EOTEXT return $this->sourceParam; } + private function shouldCommit() { + $no_commit = $this->getArgument('nocommit', false); + if ($no_commit) { + return false; + } + + return true; + } + public function run() { $source = $this->getSource(); @@ -404,14 +421,31 @@ EOTEXT return $patch_err; } else if ($repository_api instanceof ArcanistGitAPI) { + // if we're going to commit, we should make sure the working copy + // is clean + if ($this->shouldCommit()) { + $this->requireCleanWorkingCopy(); + } + $future = new ExecFuture( '(cd %s; git apply --index --reject)', $repository_api->getPath()); $future->write($bundle->toGitPatch()); $future->resolvex(); + if ($this->shouldCommit()) { + $commit_message = $this->getCommitMessage($bundle); + $future = new ExecFuture( + '(cd %s; git commit -a -F -)', + $repository_api->getPath()); + $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 if ($repository_api instanceof ArcanistMercurialAPI) { $future = new ExecFuture( '(cd %s; hg import --no-commit -)', @@ -428,6 +462,50 @@ EOTEXT return 0; } + private function getCommitMessage(ArcanistBundle $bundle) { + $revision_id = $bundle->getRevisionID(); + $commit_message = null; + $prompt_message = null; + + // if we have a revision id the commit message is in differential + if ($revision_id) { + $conduit = $this->getConduit(); + $commit_message = $conduit->callMethodSynchronous( + 'differential.getcommitmessage', + array( + 'revision_id' => $revision_id, + )); + $prompt_message = " Note arcanist failed to load the commit message ". + "from differential for revision D{$revision_id}."; + } + + // no revision id or failed to fetch commit message so get it from the + // user on the command line + if (!$commit_message) { + $template = + "\n\n". + "# Enter a commit message for this patch. If you just want to apply ". + "the patch to the working copy without committing, re-run arc patch ". + "with the --nocommit flag.". + $prompt_message. + "\n"; + + $commit_message = id(new PhutilInteractiveEditor($template)) + ->setName('arcanist-patch-commit-message') + ->editInteractively(); + + $commit_message = preg_replace('/^\s*#.*$/m', + '', + $commit_message); + $commit_message = rtrim($commit_message); + if (!strlen($commit_message)) { + throw new ArcanistUserAbortException(); + } + } + + return $commit_message; + } + public function getShellCompletions(array $argv) { // TODO: Pull open diffs from 'arc list'? return array('ARGUMENT'); diff --git a/src/workflow/patch/__init__.php b/src/workflow/patch/__init__.php index 35015c6f..7e2852ad 100644 --- a/src/workflow/patch/__init__.php +++ b/src/workflow/patch/__init__.php @@ -15,6 +15,7 @@ phutil_require_module('arcanist', 'parser/diff/changetype'); phutil_require_module('arcanist', 'workflow/base'); phutil_require_module('phutil', 'console'); +phutil_require_module('phutil', 'console/editor'); phutil_require_module('phutil', 'filesystem'); phutil_require_module('phutil', 'filesystem/tempfile'); phutil_require_module('phutil', 'future/exec');