From 2900ab6abdc4867f5b7451fad76bf8ebe6d9887d Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Thu, 21 Mar 2013 15:51:39 -0700 Subject: [PATCH] Added automatic stash / unstash support for Git in arc diff. Summary: - Added arc.autostash option to have this behaviour off by default (but configurable on a per-project basis). - Automatic stashing of changes now informs the user of how to restore their working directory if Arcanist unexpectedly terminates. - Fixed an issue with finalizeWorkingCopy when the workflow didn't require a clean working copy. Test Plan: Test `arc diff` when there are changes in the working directory; by default it should tell you to stash or commit. Turn on the arc.autostash option and try again; it should automatically stash with a message on how to recover, and it should restore the working directory automatically under almost any circumstances (other than an unrecoverable error). Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D5385 --- scripts/arcanist.php | 10 ++++-- src/configuration/ArcanistSettings.php | 10 ++++++ src/repository/api/ArcanistGitAPI.php | 13 ++++++++ src/repository/api/ArcanistRepositoryAPI.php | 11 +++++++ src/workflow/ArcanistBaseWorkflow.php | 32 ++++++++++++++++++-- 5 files changed, 72 insertions(+), 4 deletions(-) diff --git a/scripts/arcanist.php b/scripts/arcanist.php index d9e14cd6..6236c5a6 100755 --- a/scripts/arcanist.php +++ b/scripts/arcanist.php @@ -268,8 +268,14 @@ try { $config->willRunWorkflow($command, $workflow); $workflow->willRunWorkflow(); - $err = $workflow->run(); - $config->didRunWorkflow($command, $workflow, $err); + try { + $err = $workflow->run(); + $config->didRunWorkflow($command, $workflow, $err); + } catch (Exception $e) { + $workflow->finalize(); + throw $e; + } + $workflow->finalize(); exit((int)$err); } catch (ArcanistNoEffectException $ex) { diff --git a/src/configuration/ArcanistSettings.php b/src/configuration/ArcanistSettings.php index e2057ed7..4867f980 100644 --- a/src/configuration/ArcanistSettings.php +++ b/src/configuration/ArcanistSettings.php @@ -94,6 +94,16 @@ final class ArcanistSettings { "Password to use for basic auth over http transports", 'example' => '"bobhasasecret"', ), + 'arc.autostash' => array( + 'type' => 'bool', + 'help' => + 'Whether arc should permit the automatic stashing of changes in '. + 'the working directory when requiring a clean working copy. '. + 'This option should only be used when users understand how '. + 'to restore their working directory from the local stash if '. + 'an Arcanist operation causes an unrecoverable error.', + 'example' => 'false', + ), ); } diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php index b6704a74..bcf0a876 100644 --- a/src/repository/api/ArcanistGitAPI.php +++ b/src/repository/api/ArcanistGitAPI.php @@ -1000,4 +1000,17 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { return null; } + public function canStashChanges() { + return true; + } + + public function stashChanges() { + $this->execxLocal('stash'); + $this->reloadWorkingCopy(); + } + + public function unstashChanges() { + $this->execxLocal('stash pop'); + } + } diff --git a/src/repository/api/ArcanistRepositoryAPI.php b/src/repository/api/ArcanistRepositoryAPI.php index 1d41c677..5e42918b 100644 --- a/src/repository/api/ArcanistRepositoryAPI.php +++ b/src/repository/api/ArcanistRepositoryAPI.php @@ -422,6 +422,17 @@ abstract class ArcanistRepositoryAPI { abstract protected function buildLocalFuture(array $argv); + public function canStashChanges() { + return false; + } + + public function stashChanges() { + throw new ArcanistCapabilityNotSupportedException($this); + } + + public function unstashChanges() { + throw new ArcanistCapabilityNotSupportedException($this); + } /* -( Scratch Files )------------------------------------------------------ */ diff --git a/src/workflow/ArcanistBaseWorkflow.php b/src/workflow/ArcanistBaseWorkflow.php index 5a7883f5..1406dba9 100644 --- a/src/workflow/ArcanistBaseWorkflow.php +++ b/src/workflow/ArcanistBaseWorkflow.php @@ -60,6 +60,8 @@ abstract class ArcanistBaseWorkflow extends Phobject { private $passedArguments; private $command; + private $stashed; + private $projectInfo; private $arcanistConfiguration; @@ -77,6 +79,14 @@ abstract class ArcanistBaseWorkflow extends Phobject { abstract public function run(); + /** + * Finalizes any cleanup operations that need to occur regardless of + * whether the command succeeded or failed. + */ + public function finalize() { + $this->finalizeWorkingCopy(); + } + /** * Return the command used to invoke this workflow from the command like, * e.g. "help" for @{class:ArcanistHelpWorkflow}. @@ -761,6 +771,14 @@ abstract class ArcanistBaseWorkflow extends Phobject { return $this; } + public function finalizeWorkingCopy() { + if ($this->stashed) { + $api = $this->getRepositoryAPI(); + $api->unstashChanges(); + echo "Restored stashed changes to the working directory.\n"; + } + } + public function requireCleanWorkingCopy() { $api = $this->getRepositoryAPI(); @@ -838,8 +856,18 @@ abstract class ArcanistBaseWorkflow extends Phobject { $api->addToCommit($unstaged); $must_commit += array_flip($unstaged); } else { - throw new ArcanistUsageException( - "Stage and commit (or revert) them before proceeding."); + $permit_autostash = $this->getWorkingCopy()->getConfigFromAnySource( + 'arc.autostash', + false); + if ($permit_autostash && $api->canStashChanges()) { + echo "Stashing uncommitted changes. (You can restore them with ". + "`git stash pop`.)\n"; + $api->stashChanges(); + $this->stashed = true; + } else { + throw new ArcanistUsageException( + "Stage and commit (or revert) them before proceeding."); + } } }