From 3602d2aa15fdaa9fc60eb1668d0d0375799927c3 Mon Sep 17 00:00:00 2001 From: durham Date: Mon, 3 Dec 2012 13:03:13 -0800 Subject: [PATCH] Break arc land run() into smaller functions Summary: Refactor the arc land code into several functions so it's easier to maintain. This is in preparation for adding hg support to arc land in my next commit. Without this refactor, adding hg support makes the run() function too big. Test Plan: Set up a git repo and clone with a branch scenario. Example: https://secure.phabricator.com/P614 Ran and verified: arc land arc land --keep-branch arc land --merge arc land --merge --keep-branch arc land --hold arc land --revision Reviewers: epriestley Reviewed By: epriestley CC: dschleimer, sid0, aran, Korvin Differential Revision: https://secure.phabricator.com/D4056 --- src/workflow/ArcanistLandWorkflow.php | 370 +++++++++++++++----------- 1 file changed, 222 insertions(+), 148 deletions(-) diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php index ec8e2214..2ce4212f 100644 --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -6,6 +6,18 @@ * @group workflow */ final class ArcanistLandWorkflow extends ArcanistBaseWorkflow { + private $isGit; + + private $oldBranch; + private $branch; + private $onto; + private $ontoRemoteBranch; + private $remote; + private $useSquash; + private $keepBranch; + + private $revision; + private $message; public function getWorkflowName() { return 'land'; @@ -103,14 +115,52 @@ EOTEXT } public function run() { + $this->readArguments(); + $this->validate(); + $this->findRevision(); + $this->pullFromRemote(); + + if ($this->useSquash) { + $this->rebase(); + $this->squash(); + } else { + $this->merge(); + } + + $this->push(); + if (!$this->keepBranch) { + $this->cleanupBranch(); + } + + // If we were on some branch A and the user ran "arc land B", + // switch back to A. + if ($this->oldBranch != $this->branch && $this->oldBranch != $this->onto) { + $repository_api = $this->getRepositoryAPI(); + $repository_api->execxLocal( + 'checkout %s', + $this->oldBranch); + echo phutil_console_format( + "Switched back to branch **%s**.\n", + $this->oldBranch); + } + + echo "Done.\n"; + + return 0; + } + + private function readArguments() { $repository_api = $this->getRepositoryAPI(); - if (!($repository_api instanceof ArcanistGitAPI)) { + $this->isGit = $repository_api instanceof ArcanistGitAPI; + + if (!$this->isGit) { throw new ArcanistUsageException("'arc land' only supports git."); } $branch = $this->getArgument('branch'); if (empty($branch)) { $branch = $repository_api->getBranchName(); + if ($branch) { echo "Landing current branch '{$branch}'.\n"; $branch = array($branch); @@ -121,93 +171,60 @@ EOTEXT throw new ArcanistUsageException( "Specify exactly one branch to land changes from."); } - $branch = head($branch); + $this->branch = head($branch); $onto_default = nonempty( $this->getWorkingCopy()->getConfigFromAnySource('arc.land.onto.default'), 'master'); - $remote = $this->getArgument('remote', 'origin'); - $onto = $this->getArgument('onto', $onto_default); + $this->remote = $this->getArgument('remote', 'origin'); + $this->onto = $this->getArgument('onto', $onto_default); + $this->keepBranch = $this->getArgument('keep-branch'); - $is_immutable = $this->isHistoryImmutable(); + if ($this->getArgument('merge')) { + $this->useSquash = false; + } else if ($this->getArgument('squash')) { + $this->useSquash = true; + } else { + $this->useSquash = !$this->isHistoryImmutable(); + } - if ($onto == $branch) { + $this->ontoRemoteBranch = $this->remote.'/'.$this->onto; + + $this->oldBranch = $repository_api->getBranchName(); + } + + private function validate() { + $repository_api = $this->getRepositoryAPI(); + + if ($this->onto == $this->branch) { $message = "You can not land a branch onto itself -- you are trying to land ". - "'{$branch}' onto '{$onto}'. For more information on how to push ". - "changes, see 'Pushing and Closing Revisions' in ". + "'{$this->branch}' onto '{$this->onto}'. For more information on ". + "how to push changes, see 'Pushing and Closing Revisions' in ". "'Arcanist User Guide: arc diff' in the documentation."; - if (!$is_immutable) { + if (!$this->isHistoryImmutable()) { $message .= " You may be able to 'arc amend' instead."; } throw new ArcanistUsageException($message); } - if ($this->getArgument('merge')) { - $use_squash = false; - } else if ($this->getArgument('squash')) { - $use_squash = true; - } else { - $use_squash = !$is_immutable; - } - list($err) = $repository_api->execManualLocal( 'rev-parse --verify %s', - $branch); + $this->branch); if ($err) { - throw new ArcanistUsageException("Branch '{$branch}' does not exist."); + throw new ArcanistUsageException( + "Branch '{$this->branch}' does not exist."); } $this->requireCleanWorkingCopy(); - $repository_api->parseRelativeLocalCommit(array($remote.'/'.$onto)); + } - $old_branch = $repository_api->getBranchName(); + private function findRevision() { + $repository_api = $this->getRepositoryAPI(); - $repository_api->execxLocal('checkout %s', $onto); - - echo phutil_console_format( - "Switched to branch **%s**. Updating branch...\n", - $onto); - - $repository_api->execxLocal('pull --ff-only'); - - list($out) = $repository_api->execxLocal( - 'log %s/%s..%s', - $remote, - $onto, - $onto); - if (strlen(trim($out))) { - throw new ArcanistUsageException( - "Local branch '{$onto}' is ahead of '{$remote}/{$onto}', so landing ". - "a feature branch would push additional changes. Push or reset the ". - "changes in '{$onto}' before running 'arc land'."); - } - - $repository_api->execxLocal( - 'checkout %s', - $branch); - - echo phutil_console_format( - "Switched to branch **%s**. Identifying and merging...\n", - $branch); - - if ($use_squash) { - chdir($repository_api->getPath()); - $err = phutil_passthru('git rebase %s', $onto); - if ($err) { - throw new ArcanistUsageException( - "'git rebase {$onto}' failed. You can abort with 'git rebase ". - "--abort', or resolve conflicts and use 'git rebase --continue' to ". - "continue forward. After resolving the rebase, run 'arc land' ". - "again."); - } - - // Now that we've rebased, the merge-base of origin/master and HEAD may - // be different. Reparse the relative commit. - $repository_api->parseRelativeLocalCommit(array($remote.'/'.$onto)); - } + $repository_api->parseRelativeLocalCommit(array($this->ontoRemoteBranch)); $revision_id = $this->getArgument('revision'); if ($revision_id) { @@ -230,65 +247,134 @@ EOTEXT if (!count($revisions)) { throw new ArcanistUsageException( - "arc can not identify which revision exists on branch '{$branch}'. ". - "Update the revision with recent changes to synchronize the branch ". - "name and hashes, or use 'arc amend' to amend the commit message at ". - "HEAD, or use '--revision ' to select a revision explicitly."); + "arc can not identify which revision exists on branch ". + "'{$this->branch}'. Update the revision with recent changes ". + "to synchronize the branch name and hashes, or use 'arc amend' ". + "to amend the commit message at HEAD, or use '--revision ' ". + "to select a revision explicitly."); } else if (count($revisions) > 1) { $message = - "There are multiple revisions on feature branch '{$branch}' which are ". - "not present on '{$onto}':\n\n". + "There are multiple revisions on feature branch '{$this->branch}' ". + "which are not present on '{$onto}':\n\n". $this->renderRevisionList($revisions)."\n". "Separate these revisions onto different branches, or use ". "'--revision ' to select one."; throw new ArcanistUsageException($message); } - $revision = head($revisions); - $rev_id = $revision['id']; - $rev_title = $revision['title']; + $this->revision = head($revisions); - if ($revision['status'] != ArcanistDifferentialRevisionStatus::ACCEPTED) { + $rev_status = $this->revision['status']; + $rev_id = $this->revision['id']; + $rev_title = $this->revision['title']; + + if ($rev_status != ArcanistDifferentialRevisionStatus::ACCEPTED) { $ok = phutil_console_confirm( - "Revision 'D{$rev_id}: {$rev_title}' has not been accepted. Continue ". - "anyway?"); + "Revision 'D{$rev_id}: {$rev_title}' has not been ". + "accepted. Continue anyway?"); if (!$ok) { throw new ArcanistUserAbortException(); } } - echo "Landing revision 'D{$rev_id}: {$rev_title}'...\n"; - - $message = $this->getConduit()->callMethodSynchronous( + $this->message = $this->getConduit()->callMethodSynchronous( 'differential.getcommitmessage', array( - 'revision_id' => $revision['id'], + 'revision_id' => $rev_id, )); - $repository_api->execxLocal('checkout %s', $onto); + echo "Landing revision 'D{$rev_id}: ". + "{$rev_title}'...\n"; + } - if (!$use_squash) { - // In immutable histories, do a --no-ff merge to force a merge commit with - // the right message. + private function pullFromRemote() { + $repository_api = $this->getRepositoryAPI(); + $repository_api->execxLocal('checkout %s', $this->onto); + + echo phutil_console_format( + "Switched to branch **%s**. Updating branch...\n", + $this->onto); + + $repository_api->execxLocal('pull --ff-only'); + + list($out) = $repository_api->execxLocal( + 'log %s/%s..%s', + $this->remote, + $this->onto, + $this->onto); + if (strlen(trim($out))) { + throw new ArcanistUsageException( + "Local branch '{$this->onto}' is ahead of remote branch ". + "'{$this->ontoRemoteBranch}', so landing a feature branch ". + "would push additional changes. Push or reset the changes ". + "in '{$this->onto}' before running 'arc land'."); + } + } + + private function rebase() { + $repository_api = $this->getRepositoryAPI(); + $repository_api->execxLocal( + 'checkout %s', + $this->branch); + + echo phutil_console_format( + "Switched to branch **%s**. Identifying and merging...\n", + $this->branch); + + if ($this->useSquash) { chdir($repository_api->getPath()); - $err = phutil_passthru( - 'git merge --no-ff --no-commit %s', - $branch); + $err = phutil_passthru('git rebase %s', $this->onto); + if ($err) { throw new ArcanistUsageException( - "'git merge' failed. Your working copy has been left in a partially ". - "merged state. You can: abort with 'git merge --abort'; or follow ". - "the instructions to complete the merge."); + "'git rebase {$this->onto}' failed. ". + "You can abort with 'git rebase --abort', ". + "or resolve conflicts and use 'git rebase ". + "--continue' to continue forward. After resolving the rebase, ". + "run 'arc land' again."); } - } else { - // In mutable histories, do a --squash merge. - $repository_api->execxLocal( - 'merge --squash --ff-only %s', - $branch); + + // Now that we've rebased, the merge-base of origin/master and HEAD may + // be different. Reparse the relative commit. + $repository_api->parseRelativeLocalCommit(array($this->ontoRemoteBranch)); } + } + + private function squash() { + $repository_api = $this->getRepositoryAPI(); + $repository_api->execxLocal('checkout %s', $this->onto); + + $repository_api->execxLocal( + 'merge --squash --ff-only %s', + $this->branch); + } + + private function merge() { + $repository_api = $this->getRepositoryAPI(); + + // In immutable histories, do a --no-ff merge to force a merge commit with + // the right message. + $repository_api->execxLocal('checkout %s', $this->onto); + + chdir($repository_api->getPath()); + $err = phutil_passthru( + 'git merge --no-ff --no-commit %s', + $this->branch); + + if ($err) { + throw new ArcanistUsageException( + "'git merge' failed. Your working copy has been left in a partially ". + "merged state. You can: abort with 'git merge --abort'; or follow ". + "the instructions to complete the merge."); + } + } + + private function push() { + $repository_api = $this->getRepositoryAPI(); $tmp_file = new TempFile(); - Filesystem::writeFile($tmp_file, $message); + Filesystem::writeFile($tmp_file, $this->message); + $repository_api->execxLocal( 'commit -F %s', $tmp_file); @@ -296,18 +382,20 @@ EOTEXT if ($this->getArgument('hold')) { echo phutil_console_format( "Holding change in **%s**: it has NOT been pushed yet.\n", - $onto); + $this->onto); } else { echo "Pushing change...\n\n"; chdir($repository_api->getPath()); + $err = phutil_passthru( 'git push %s %s', - $remote, - $onto); + $this->remote, + $this->onto); if ($err) { - throw new ArcanistUsageException("'git push' failed."); + $repo_command = $repository_api->getSourceControlSystemName(); + throw new ArcanistUsageException("'{$repo_command} push' failed."); } $mark_workflow = $this->buildChildWorkflow( @@ -315,67 +403,53 @@ EOTEXT array( '--finalize', '--quiet', - $revision['id'], + $this->revision['id'], )); $mark_workflow->run(); echo "\n"; } + } - if (!$this->getArgument('keep-branch')) { - list($ref) = $repository_api->execxLocal( - 'rev-parse --verify %s', - $branch); - $ref = trim($ref); - $recovery_command = csprintf( - 'git checkout -b %s %s', - $branch, - $ref); + private function cleanupBranch() { + $repository_api = $this->getRepositoryAPI(); - echo "Cleaning up feature branch...\n"; - echo "(Use `{$recovery_command}` if you want it back.)\n"; - $repository_api->execxLocal( - 'branch -D %s', - $branch); + echo "Cleaning up feature branch...\n"; + list($ref) = $repository_api->execxLocal( + 'rev-parse --verify %s', + $this->branch); + $ref = trim($ref); + $recovery_command = csprintf( + 'git checkout -b %s %s', + $this->branch, + $ref); + echo "(Use `{$recovery_command}` if you want it back.)\n"; + $repository_api->execxLocal( + 'branch -D %s', + $this->branch); - if ($this->getArgument('delete-remote')) { - list($err, $ref) = $repository_api->execManualLocal( - 'rev-parse --verify %s/%s', - $remote, - $branch); + if ($this->getArgument('delete-remote')) { + list($err, $ref) = $repository_api->execManualLocal( + 'rev-parse --verify %s/%s', + $this->remote, + $this->branch); - if ($err) { - echo "No remote feature branch to clean up.\n"; - } else { + if ($err) { + echo "No remote feature branch to clean up.\n"; + } else { - // NOTE: In Git, you delete a remote branch by pushing it with a - // colon in front of its name: - // - // git push : + // NOTE: In Git, you delete a remote branch by pushing it with a + // colon in front of its name: + // + // git push : - echo "Cleaning up remote feature branch...\n"; - $repository_api->execxLocal( - 'push %s :%s', - $remote, - $branch); - } + echo "Cleaning up remote feature branch...\n"; + $repository_api->execxLocal( + 'push %s :%s', + $this->remote, + $this->branch); } } - - // If we were on some branch A and the user ran "arc land B", switch back - // to A. - if (($old_branch != $branch) && ($old_branch != $onto)) { - $repository_api->execxLocal( - 'checkout %s', - $old_branch); - echo phutil_console_format( - "Switched back to branch **%s**.\n", - $old_branch); - } - - echo "Done.\n"; - - return 0; } protected function getSupportedRevisionControlSystems() {