From c46dbbf61cc7f5752c980ac4f6180b568e0135f9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 25 Jan 2012 15:10:59 -0800 Subject: [PATCH] Add "arc land" as a first-class workflow Summary: This is a fancy version of "land.sh" that uses "git merge --squash" and "arc which" to cover more cases. Test Plan: Ran "arc land" against various repository states (no such branch, not accepted, valid, etc). Things seemed OK. There are basically an infinite number of states here so it's hard to test exhaustively. Reviewers: cpiro, btrahan, jungejason, davidreuss Reviewed By: davidreuss CC: zeeg, aran, epriestley, davidreuss Maniphest Tasks: T787, T723 Differential Revision: https://secure.phabricator.com/D1488 --- src/__phutil_library_map__.php | 2 + src/workflow/base/ArcanistBaseWorkflow.php | 8 + src/workflow/land/ArcanistLandWorkflow.php | 274 +++++++++++++++++++++ src/workflow/land/__init__.php | 19 ++ 4 files changed, 303 insertions(+) create mode 100644 src/workflow/land/ArcanistLandWorkflow.php create mode 100644 src/workflow/land/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 68f1dc54..5ee1c01c 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -48,6 +48,7 @@ phutil_register_library_map(array( 'ArcanistHookAPI' => 'repository/hookapi/base', 'ArcanistInstallCertificateWorkflow' => 'workflow/install-certificate', 'ArcanistJSHintLinter' => 'lint/linter/jshint', + 'ArcanistLandWorkflow' => 'workflow/land', 'ArcanistLiberateLintEngine' => 'lint/engine/liberate', 'ArcanistLiberateWorkflow' => 'workflow/liberate', 'ArcanistLicenseLinter' => 'lint/linter/license', @@ -136,6 +137,7 @@ phutil_register_library_map(array( 'ArcanistHelpWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistInstallCertificateWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistJSHintLinter' => 'ArcanistLinter', + 'ArcanistLandWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistLiberateLintEngine' => 'ArcanistLintEngine', 'ArcanistLiberateWorkflow' => 'ArcanistBaseWorkflow', 'ArcanistLicenseLinter' => 'ArcanistLinter', diff --git a/src/workflow/base/ArcanistBaseWorkflow.php b/src/workflow/base/ArcanistBaseWorkflow.php index b8b8f024..7a459861 100644 --- a/src/workflow/base/ArcanistBaseWorkflow.php +++ b/src/workflow/base/ArcanistBaseWorkflow.php @@ -992,4 +992,12 @@ class ArcanistBaseWorkflow { return array_values($paths); } + protected function renderRevisionList(array $revisions) { + $list = array(); + foreach ($revisions as $revision) { + $list[] = ' - D'.$revision['id'].': '.$revision['title']."\n"; + } + return implode('', $list); + } + } diff --git a/src/workflow/land/ArcanistLandWorkflow.php b/src/workflow/land/ArcanistLandWorkflow.php new file mode 100644 index 00000000..efff92dc --- /dev/null +++ b/src/workflow/land/ArcanistLandWorkflow.php @@ -0,0 +1,274 @@ + array( + 'param' => 'master', + 'help' => "Land feature branch onto a branch other than ". + "'master' (default).", + ), + 'hold' => array( + 'help' => "Prepare the change to be pushed, but do not actually ". + "push it.", + ), + 'keep-branch' => array( + 'help' => "Keep the feature branch after pushing changes to the ". + "remote (by default, it is deleted).", + ), + 'remote' => array( + 'param' => 'origin', + 'help' => "Push to a remote other than 'origin' (default).", + ), + '*' => 'branch', + ); + } + + public function run() { + $this->writeStatusMessage( + phutil_console_format( + "**WARNING:** 'arc land' is new and experimental.\n")); + + $branch = $this->getArgument('branch'); + if (count($branch) !== 1) { + throw new ArcanistUsageException( + "Specify exactly one branch to land changes from."); + } + $branch = head($branch); + + $remote = $this->getArgument('remote', 'origin'); + $onto = $this->getArgument('onto', 'master'); + $is_immutable = $this->isHistoryImmutable(); + + $repository_api = $this->getRepositoryAPI(); + if (!($repository_api instanceof ArcanistGitAPI)) { + throw new ArcanistUsageException("'arc land' only supports git."); + } + + list($err) = exec_manual( + '(cd %s && git rev-parse --verify %s)', + $repository_api->getPath(), + $branch); + + if ($err) { + throw new ArcanistUsageException("Branch '{$branch}' does not exist."); + } + + $this->requireCleanWorkingCopy(); + $repository_api->parseRelativeLocalCommit(array($remote.'/'.$onto)); + + execx( + '(cd %s && git checkout %s)', + $repository_api->getPath(), + $onto); + + echo phutil_console_format( + "Switched to branch **%s**. Updating branch...\n", + $onto); + + execx( + '(cd %s && git pull --ff-only)', + $repository_api->getPath()); + + list($out) = execx( + '(cd %s && git log %s/%s..%s)', + $repository_api->getPath(), + $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'."); + } + + execx( + '(cd %s && git checkout %s)', + $repository_api->getPath(), + $branch); + + echo phutil_console_format( + "Switched to branch **%s**. Identifying and merging...\n", + $branch); + + if (!$is_immutable) { + $err = phutil_passthru( + '(cd %s && git rebase %s)', + $repository_api->getPath(), + $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)); + } + + $revisions = $repository_api->loadWorkingCopyDifferentialRevisions( + $this->getConduit(), + array( + 'authors' => array($this->getUserPHID()), + )); + + 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."); + } else if (count($revisions) > 1) { + $message = + "There are multiple revisions on feature branch '{$branch}' which are ". + "not present on '{$onto}':\n\n". + $this->renderRevisionList($revisions)."\n". + "Separate these revisions onto different branches, or manually land ". + "them in '{$onto}'."; + throw new ArcanistUsageException($message); + } + + $revision = head($revisions); + $rev_id = $revision['id']; + $rev_title = $revision['title']; + + if ($revision['status'] != ArcanistDifferentialRevisionStatus::ACCEPTED) { + $ok = phutil_console_confirm( + "Revision 'D{$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( + 'differential.getcommitmessage', + array( + 'revision_id' => $revision['id'], + )); + + execx( + '(cd %s && git checkout %s)', + $repository_api->getPath(), + $onto); + + if ($is_immutable) { + // In immutable histories, do a --no-ff merge to force a merge commit with + // the right message. + execx( + '(cd %s && git merge --no-ff -m %s %s)', + $repository_api->getPath(), + $message, + $branch); + } else { + // In mutable histories, do a --squash merge. + execx( + '(cd %s && git merge --squash --ff-only %s)', + $repository_api->getPath(), + $branch); + execx( + '(cd %s && git commit -m %s)', + $repository_api->getPath(), + $message); + } + + if ($this->getArgument('hold')) { + echo phutil_console_format( + "Holding change in **%s**: it has NOT been pushed yet.\n", + $onto); + } else { + echo "Pushing change...\n\n"; + + $err = phutil_passthru( + '(cd %s && git push %s %s)', + $repository_api->getPath(), + $remote, + $onto); + + if ($err) { + throw new ArcanistUsageException("'git push' failed."); + } + + echo "\n"; + } + + if (!$this->getArgument('keep-branch')) { + echo "Cleaning up feature branch...\n"; + execx( + '(cd %s && git branch -D %s)', + $repository_api->getPath(), + $branch); + } + + echo "Done.\n"; + + return 0; + } + + protected function getSupportedRevisionControlSystems() { + return array('git'); + } + +} diff --git a/src/workflow/land/__init__.php b/src/workflow/land/__init__.php new file mode 100644 index 00000000..eadd9ad0 --- /dev/null +++ b/src/workflow/land/__init__.php @@ -0,0 +1,19 @@ +