diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 96d7da91..ec6a78b5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -112,6 +112,8 @@ phutil_register_library_map(array( 'ArcanistGeneratedLinterTestCase' => 'lint/linter/__tests__/ArcanistGeneratedLinterTestCase.php', 'ArcanistGetConfigWorkflow' => 'workflow/ArcanistGetConfigWorkflow.php', 'ArcanistGitAPI' => 'repository/api/ArcanistGitAPI.php', + 'ArcanistGitLandEngine' => 'land/ArcanistGitLandEngine.php', + 'ArcanistGitUpstreamPath' => 'repository/api/ArcanistGitUpstreamPath.php', 'ArcanistGlobalVariableXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistGlobalVariableXHPASTLinterRule.php', 'ArcanistGoLintLinter' => 'lint/linter/ArcanistGoLintLinter.php', 'ArcanistGoLintLinterTestCase' => 'lint/linter/__tests__/ArcanistGoLintLinterTestCase.php', @@ -144,6 +146,7 @@ phutil_register_library_map(array( 'ArcanistJscsLinterTestCase' => 'lint/linter/__tests__/ArcanistJscsLinterTestCase.php', 'ArcanistKeywordCasingXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistKeywordCasingXHPASTLinterRule.php', 'ArcanistLambdaFuncFunctionXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLambdaFuncFunctionXHPASTLinterRule.php', + 'ArcanistLandEngine' => 'land/ArcanistLandEngine.php', 'ArcanistLandWorkflow' => 'workflow/ArcanistLandWorkflow.php', 'ArcanistLanguageConstructParenthesesXHPASTLinterRule' => 'lint/linter/xhpast/rules/ArcanistLanguageConstructParenthesesXHPASTLinterRule.php', 'ArcanistLesscLinter' => 'lint/linter/ArcanistLesscLinter.php', @@ -398,6 +401,8 @@ phutil_register_library_map(array( 'ArcanistGeneratedLinterTestCase' => 'ArcanistLinterTestCase', 'ArcanistGetConfigWorkflow' => 'ArcanistWorkflow', 'ArcanistGitAPI' => 'ArcanistRepositoryAPI', + 'ArcanistGitLandEngine' => 'ArcanistLandEngine', + 'ArcanistGitUpstreamPath' => 'Phobject', 'ArcanistGlobalVariableXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistGoLintLinter' => 'ArcanistExternalLinter', 'ArcanistGoLintLinterTestCase' => 'ArcanistExternalLinterTestCase', @@ -430,6 +435,7 @@ phutil_register_library_map(array( 'ArcanistJscsLinterTestCase' => 'ArcanistExternalLinterTestCase', 'ArcanistKeywordCasingXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistLambdaFuncFunctionXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', + 'ArcanistLandEngine' => 'Phobject', 'ArcanistLandWorkflow' => 'ArcanistWorkflow', 'ArcanistLanguageConstructParenthesesXHPASTLinterRule' => 'ArcanistXHPASTLinterRule', 'ArcanistLesscLinter' => 'ArcanistExternalLinter', diff --git a/src/internationalization/ArcanistUSEnglishTranslation.php b/src/internationalization/ArcanistUSEnglishTranslation.php index 04140399..e83e099e 100644 --- a/src/internationalization/ArcanistUSEnglishTranslation.php +++ b/src/internationalization/ArcanistUSEnglishTranslation.php @@ -78,6 +78,11 @@ final class ArcanistUSEnglishTranslation extends PhutilTranslation { 'Ignore the changes to this submodule and continue?', 'Ignore the changes to these submodules and continue?', ), + + 'These %s commit(s) will be landed:' => array( + 'This commit will be landed:', + 'These commits will be landed:', + ), ); } diff --git a/src/land/ArcanistGitLandEngine.php b/src/land/ArcanistGitLandEngine.php new file mode 100644 index 00000000..95b33aa1 --- /dev/null +++ b/src/land/ArcanistGitLandEngine.php @@ -0,0 +1,532 @@ +verifySourceAndTargetExist(); + $this->fetchTarget(); + + $this->printLandingCommits(); + + if ($this->getShouldPreview()) { + $this->writeInfo( + pht('PREVIEW'), + pht('Completed preview of operation.')); + return; + } + + $this->saveLocalState(); + + try { + $this->identifyRevision(); + $this->updateWorkingCopy(); + + if ($this->getShouldHold()) { + $this->writeInfo( + pht('HOLD'), + pht('Holding change locally, it has not been pushed.')); + } else { + $this->pushChange(); + $this->reconcileLocalState(); + + $api = $this->getRepositoryAPI(); + $api->execxLocal('submodule update --init --recursive'); + + if ($this->getShouldKeep()) { + echo tsprintf( + "%s\n", + pht('Keeping local branch.')); + } else { + $this->destroyLocalBranch(); + } + + $this->writeOkay( + pht('DONE'), + pht('Landed changes.')); + } + + $this->restoreWhenDestroyed = false; + } catch (Exception $ex) { + $this->restoreLocalState(); + throw $ex; + } + } + + public function __destruct() { + if ($this->restoreWhenDestroyed) { + $this->writeWARN( + pht('INTERRUPTED!'), + pht('Restoring working copy to its original state.')); + + $this->restoreLocalState(); + } + } + + protected function getLandingCommits() { + $api = $this->getRepositoryAPI(); + + list($out) = $api->execxLocal( + 'log --oneline %s..%s --', + $this->getTargetFullRef(), + $this->sourceCommit); + + $out = trim($out); + + if (!strlen($out)) { + return array(); + } else { + return phutil_split_lines($out, false); + } + } + + private function identifyRevision() { + $api = $this->getRepositoryAPI(); + $api->execxLocal('checkout %s --', $this->getSourceRef()); + call_user_func($this->getBuildMessageCallback(), $this); + } + + private function verifySourceAndTargetExist() { + $api = $this->getRepositoryAPI(); + + list($err) = $api->execManualLocal( + 'rev-parse --verify %s', + $this->getTargetFullRef()); + + if ($err) { + throw new Exception( + pht( + 'Branch "%s" does not exist in remote "%s".', + $this->getTargetOnto(), + $this->getTargetRemote())); + } + + list($err, $stdout) = $api->execManualLocal( + 'rev-parse --verify %s', + $this->getSourceRef()); + + if ($err) { + throw new Exception( + pht( + 'Branch "%s" does not exist in the local working copy.', + $this->getSourceRef())); + } + + $this->sourceCommit = trim($stdout); + } + + private function fetchTarget() { + $api = $this->getRepositoryAPI(); + + $ref = $this->getTargetFullRef(); + + $this->writeInfo( + pht('FETCH'), + pht('Fetching %s...', $ref)); + + $api->execxLocal( + 'fetch -- %s %s', + $this->getTargetRemote(), + $this->getTargetOnto()); + } + + private function updateWorkingCopy() { + $api = $this->getRepositoryAPI(); + $source = $this->sourceCommit; + + $api->execxLocal( + 'checkout %s --', + $this->getTargetFullRef()); + + list($original_author, $original_date) = $this->getAuthorAndDate($source); + + try { + if ($this->getShouldSquash()) { + // NOTE: We're explicitly specifying "--ff" to override the presence + // of "merge.ff" options in user configuration. + + $api->execxLocal( + 'merge --no-stat --no-commit --ff --squash -- %s', + $source); + } else { + $api->execxLocal( + 'merge --no-stat --no-commit --no-ff -- %s', + $source); + } + } catch (Exception $ex) { + $api->execManualLocal('merge --abort'); + $api->execManualLocal('reset --hard HEAD --'); + + throw new Exception( + pht( + 'Local "%s" does not merge cleanly into "%s". Merge or rebase '. + 'local changes so they can merge cleanly.', + $this->getSourceRef(), + $this->getTargetFullRef())); + } + + list($changes) = $api->execxLocal('diff HEAD --'); + $changes = trim($changes); + if (!strlen($changes)) { + throw new Exception( + pht( + 'Merging local "%s" into "%s" produces an empty diff. '. + 'This usually means these changes have already landed.', + $this->getSourceRef(), + $this->getTargetFullRef())); + } + + $api->execxLocal( + 'commit --author %s --date %s -F %s --', + $original_author, + $original_date, + $this->getCommitMessageFile()); + + $this->getWorkflow()->didCommitMerge(); + + list($stdout) = $api->execxLocal( + 'rev-parse --verify %s', + 'HEAD'); + $this->mergedRef = trim($stdout); + } + + private function pushChange() { + $api = $this->getRepositoryAPI(); + + $this->writeInfo( + pht('PUSHING'), + pht('Pushing changes to "%s".', $this->getTargetFullRef())); + + $err = $api->execPassthru( + 'push -- %s %s:%s', + $this->getTargetRemote(), + $this->mergedRef, + $this->getTargetOnto()); + + if ($err) { + throw new ArcanistUsageException( + pht( + 'Push failed! Fix the error and run "%s" again.', + 'arc land')); + } + } + + private function reconcileLocalState() { + $api = $this->getRepositoryAPI(); + + // Try to put the user into the best final state we can. This is very + // complicated because users are incredibly creative and their local + // branches may have the same names as branches in the remote but no + // relationship to them. + + if ($this->localRef != $this->getSourceRef()) { + // The user ran `arc land X` but was on a different branch, so just put + // them back wherever they were before. + $this->writeInfo( + pht('RESTORE'), + pht('Switching back to "%s".', $this->localRef)); + $this->restoreLocalState(); + return; + } + + // We're going to try to find a path to the upstream target branch. We + // try in two different ways: + // + // - follow the source branch directly along tracking branches until + // we reach the upstream; or + // - follow a local branch with the same name as the target branch until + // we reach the upstream. + + // First, get the path from whatever we landed to wherever it goes. + $local_branch = $this->getSourceRef(); + + $path = $api->getPathToUpstream($local_branch); + if ($path->getLength()) { + // We may want to discard the thing we landed from the path, if we're + // going to delete it. In this case, we don't want to update it or worry + // if it's dirty. + if ($this->getSourceRef() == $this->getTargetOnto()) { + // In this case, we've done something like land "master" onto itself, + // so we do want to update the actual branch. We're going to use the + // entire path. + } else { + // Otherwise, we're going to delete the branch at the end of the + // workflow, so throw it away the most-local branch that isn't long + // for this world. + $path->removeUpstream($local_branch); + + if (!$path->getLength()) { + $this->writeInfo( + pht('UPDATE'), + pht( + 'Local branch "%s" directly tracks remote, staying on '. + 'detached HEAD.', + $local_branch)); + return; + } + + $local_branch = head($path->getLocalBranches()); + } + } else { + // The source branch has no upstream, so look for a local branch with + // the same name as the target branch. This corresponds to the common + // case where you have "master" and checkout local branches from it + // with "git checkout -b feature", then land onto "master". + + $local_branch = $this->getTargetOnto(); + + list($err) = $api->execManualLocal( + 'rev-parse --verify %s', + $local_branch); + if ($err) { + $this->writeInfo( + pht('UPDATE'), + pht( + 'Local branch "%s" does not exist, staying on detached HEAD.', + $local_branch)); + return; + } + + $path = $api->getPathToUpstream($local_branch); + } + + if ($path->getCycle()) { + $this->writeWarn( + pht('LOCAL CYCLE'), + pht( + 'Local branch "%s" tracks an upstream but following it leads to '. + 'a local cycle, staying on detached HEAD.', + $local_branch)); + return; + } + + if (!$path->isConnectedToRemote()) { + $this->writeInfo( + pht('UPDATE'), + pht( + 'Local branch "%s" is not connected to a remote, staying on '. + 'detached HEAD.', + $local_branch)); + return; + } + + $remote_remote = $path->getRemoteRemoteName(); + $remote_branch = $path->getRemoteBranchName(); + + $remote_actual = $remote_remote.'/'.$remote_branch; + $remote_expect = $this->getTargetFullRef(); + if ($remote_actual != $remote_expect) { + $this->writeInfo( + pht('UPDATE'), + pht( + 'Local branch "%s" is connected to a remote ("%s") other than '. + 'the target remote ("%s"), staying on detached HEAD.', + $local_branch, + $remote_actual, + $remote_expect)); + return; + } + + // If we get this far, we have a sequence of branches which ultimately + // connect to the remote. We're going to try to update them all in reverse + // order, from most-upstream to most-local. + + $cascade_branches = $path->getLocalBranches(); + $cascade_branches = array_reverse($cascade_branches); + + // First, check if any of them are ahead of the remote. + + $ahead_of_remote = array(); + foreach ($cascade_branches as $cascade_branch) { + list($stdout) = $api->execxLocal( + 'log %s..%s --', + $this->mergedRef, + $cascade_branch); + $stdout = trim($stdout); + + if (strlen($stdout)) { + $ahead_of_remote[$cascade_branch] = $cascade_branch; + } + } + + // We're going to handle the last branch (the thing we ultimately intend + // to check out) differently. It's OK if it's ahead of the remote, as long + // as we just landed it. + + $local_ahead = isset($ahead_of_remote[$local_branch]); + unset($ahead_of_remote[$local_branch]); + $land_self = ($this->getTargetOnto() === $this->getSourceRef()); + + // We aren't going to pull anything if anything upstream from us is ahead + // of the remote, or the local is ahead of the remote and we didn't land + // it onto itself. + $skip_pull = ($ahead_of_remote || ($local_ahead && !$land_self)); + + if ($skip_pull) { + $this->writeInfo( + pht('UPDATE'), + pht( + 'Local "%s" is ahead of remote "%s". Checking out "%s" but '. + 'not pulling changes.', + nonempty(head($ahead_of_remote), $local_branch), + $this->getTargetFullRef(), + $local_branch)); + + $this->writeInfo( + pht('CHECKOUT'), + pht( + 'Checking out "%s".', + $local_branch)); + + $api->execxLocal('checkout %s --', $local_branch); + + return; + } + + // If nothing upstream from our nearest branch is ahead of the remote, + // pull it all. + + $cascade_targets = array(); + if (!$ahead_of_remote) { + foreach ($cascade_branches as $cascade_branch) { + if ($local_ahead && ($local_branch == $cascade_branch)) { + continue; + } + $cascade_targets[] = $cascade_branch; + } + } + + if ($cascade_targets) { + $this->writeInfo( + pht('UPDATE'), + pht( + 'Local "%s" tracks target remote "%s", checking out and '. + 'pulling changes.', + $local_branch, + $this->getTargetFullRef())); + + foreach ($cascade_targets as $cascade_branch) { + $this->writeInfo( + pht('PULL'), + pht( + 'Checking out and pulling "%s".', + $cascade_branch)); + + $api->execxLocal('checkout %s --', $cascade_branch); + $api->execxLocal('pull --'); + } + + if (!$local_ahead) { + return; + } + } + + // In this case, the user did something like land a branch onto itself, + // and the branch is tracking the correct remote. We're going to discard + // the local state and reset it to the state we just pushed. + + $this->writeInfo( + pht('RESET'), + pht( + 'Local "%s" landed into remote "%s", resetting local branch to '. + 'remote state.', + $this->getTargetOnto(), + $this->getTargetFullRef())); + + $api->execxLocal('checkout %s --', $local_branch); + $api->execxLocal('reset --hard %s --', $this->getTargetFullRef()); + + return; + } + + private function destroyLocalBranch() { + $api = $this->getRepositoryAPI(); + + if ($this->getSourceRef() == $this->getTargetOnto()) { + // If we landed a branch into a branch with the same name, so don't + // destroy it. This prevents us from cleaning up "master" if you're + // landing master into itself. + return; + } + + // TODO: Maybe this should also recover the proper upstream? + + $recovery_command = csprintf( + 'git checkout -b %R %R', + $this->getSourceRef(), + $this->sourceCommit); + + echo tsprintf( + "%s\n", + pht('Cleaning up branch "%s"...', $this->getSourceRef())); + + echo tsprintf( + "%s\n", + pht('(Use `%s` if you want it back.)', $recovery_command)); + + $api->execxLocal('branch -D -- %s', $this->getSourceRef()); + } + + /** + * Save the local working copy state so we can restore it later. + */ + private function saveLocalState() { + $api = $this->getRepositoryAPI(); + + $this->localCommit = $api->getWorkingCopyRevision(); + + list($ref) = $api->execxLocal('rev-parse --abbrev-ref HEAD'); + $ref = trim($ref); + if ($ref === 'HEAD') { + $ref = $this->localCommit; + } + + $this->localRef = $ref; + + $this->restoreWhenDestroyed = true; + } + + /** + * Restore the working copy to the state it was in before we started + * performing writes. + */ + private function restoreLocalState() { + $api = $this->getRepositoryAPI(); + + $api->execxLocal('checkout %s --', $this->localRef); + $api->execxLocal('reset --hard %s --', $this->localCommit); + $api->execxLocal('submodule update --init --recursive'); + + $this->restoreWhenDestroyed = false; + } + + private function getTargetFullRef() { + return $this->getTargetRemote().'/'.$this->getTargetOnto(); + } + + private function getAuthorAndDate($commit) { + $api = $this->getRepositoryAPI(); + + // TODO: This is working around Windows escaping problems, see T8298. + + list($info) = $api->execxLocal( + 'log -n1 --format=%C %s --', + '%aD%n%an%n%ae', + $commit); + + $info = trim($info); + list($date, $author, $email) = explode("\n", $info, 3); + + return array( + "$author <{$email}>", + $date, + ); + } + +} diff --git a/src/land/ArcanistLandEngine.php b/src/land/ArcanistLandEngine.php new file mode 100644 index 00000000..679a629a --- /dev/null +++ b/src/land/ArcanistLandEngine.php @@ -0,0 +1,161 @@ +workflow = $workflow; + return $this; + } + + final public function getWorkflow() { + return $this->workflow; + } + + final public function setRepositoryAPI( + ArcanistRepositoryAPI $repository_api) { + $this->repositoryAPI = $repository_api; + return $this; + } + + final public function getRepositoryAPI() { + return $this->repositoryAPI; + } + + final public function setShouldHold($should_hold) { + $this->shouldHold = $should_hold; + return $this; + } + + final public function getShouldHold() { + return $this->shouldHold; + } + + final public function setShouldKeep($should_keep) { + $this->shouldKeep = $should_keep; + return $this; + } + + final public function getShouldKeep() { + return $this->shouldKeep; + } + + final public function setShouldSquash($should_squash) { + $this->shouldSquash = $should_squash; + return $this; + } + + final public function getShouldSquash() { + return $this->shouldSquash; + } + + final public function setShouldPreview($should_preview) { + $this->shouldPreview = $should_preview; + return $this; + } + + final public function getShouldPreview() { + return $this->shouldPreview; + } + + final public function setTargetRemote($target_remote) { + $this->targetRemote = $target_remote; + return $this; + } + + final public function getTargetRemote() { + return $this->targetRemote; + } + + final public function setTargetOnto($target_onto) { + $this->targetOnto = $target_onto; + return $this; + } + + final public function getTargetOnto() { + return $this->targetOnto; + } + + final public function setSourceRef($source_ref) { + $this->sourceRef = $source_ref; + return $this; + } + + final public function getSourceRef() { + return $this->sourceRef; + } + + final public function setBuildMessageCallback($build_message_callback) { + $this->buildMessageCallback = $build_message_callback; + return $this; + } + + final public function getBuildMessageCallback() { + return $this->buildMessageCallback; + } + + final public function setCommitMessageFile($commit_message_file) { + $this->commitMessageFile = $commit_message_file; + return $this; + } + + final public function getCommitMessageFile() { + return $this->commitMessageFile; + } + + abstract public function execute(); + + abstract protected function getLandingCommits(); + + protected function printLandingCommits() { + $logs = $this->getLandingCommits(); + + if (!$logs) { + throw new ArcanistUsageException( + pht( + 'There are no commits on "%s" which are not already present on '. + 'the target.', + $this->getSourceRef())); + } + + $list = id(new PhutilConsoleList()) + ->setWrap(false) + ->addItems($logs); + + id(new PhutilConsoleBlock()) + ->addParagraph( + pht( + 'These %s commit(s) will be landed:', + new PhutilNumber(count($logs)))) + ->addList($list) + ->draw(); + } + + protected function writeWarn($title, $message) { + return $this->getWorkflow()->writeWarn($title, $message); + } + + protected function writeInfo($title, $message) { + return $this->getWorkflow()->writeInfo($title, $message); + } + + protected function writeOkay($title, $message) { + return $this->getWorkflow()->writeOkay($title, $message); + } + + +} diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php index 84ac017f..d93b8083 100644 --- a/src/repository/api/ArcanistGitAPI.php +++ b/src/repository/api/ArcanistGitAPI.php @@ -1333,4 +1333,75 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { $this->resolvedHeadCommit = null; } + /** + * Follow the chain of tracking branches upstream until we reach a remote + * or cycle locally. + * + * @param string Ref to start from. + * @return list Path to an upstream. + */ + public function getPathToUpstream($start) { + $cursor = $start; + $path = new ArcanistGitUpstreamPath(); + while (true) { + list($err, $upstream) = $this->execManualLocal( + 'rev-parse --symbolic-full-name %s@{upstream}', + $cursor); + + if ($err) { + // We ended up somewhere with no tracking branch, so we're done. + break; + } + + $upstream = trim($upstream); + + if (preg_match('(^refs/heads/)', $upstream)) { + $upstream = preg_replace('(^refs/heads/)', '', $upstream); + + $is_cycle = $path->getUpstream($upstream); + + $path->addUpstream( + $cursor, + array( + 'type' => ArcanistGitUpstreamPath::TYPE_LOCAL, + 'name' => $upstream, + 'cycle' => $is_cycle, + )); + + if ($is_cycle) { + // We ran into a local cycle, so we're done. + break; + } + + // We found another local branch, so follow that one upriver. + $cursor = $upstream; + continue; + } + + if (preg_match('(^refs/remotes/)', $upstream)) { + $upstream = preg_replace('(^refs/remotes/)', '', $upstream); + list($remote, $branch) = explode('/', $upstream, 2); + + $path->addUpstream( + $cursor, + array( + 'type' => ArcanistGitUpstreamPath::TYPE_REMOTE, + 'name' => $branch, + 'remote' => $remote, + )); + + // We found a remote, so we're done. + break; + } + + throw new Exception( + pht( + 'Got unrecognized upstream format ("%s") from Git, expected '. + '"refs/heads/..." or "refs/remotes/...".', + $upstream)); + } + + return $path; + } + } diff --git a/src/repository/api/ArcanistGitUpstreamPath.php b/src/repository/api/ArcanistGitUpstreamPath.php new file mode 100644 index 00000000..ad487106 --- /dev/null +++ b/src/repository/api/ArcanistGitUpstreamPath.php @@ -0,0 +1,90 @@ +path[$key] = $spec; + return $this; + } + + public function removeUpstream($key) { + unset($this->path[$key]); + return $this; + } + + public function getUpstream($key) { + return idx($this->path, $key); + } + + public function getLength() { + return count($this->path); + } + + /** + * Test if this path eventually connects to a remote. + * + * @return bool True if the path connects to a remote. + */ + public function isConnectedToRemote() { + $last = last($this->path); + + if (!$last) { + return false; + } + + return ($last['type'] == self::TYPE_REMOTE); + } + + public function getLocalBranches() { + return array_keys($this->path); + } + + public function getRemoteBranchName() { + if (!$this->isConnectedToRemote()) { + return null; + } + + return idx(last($this->path), 'name'); + } + + public function getRemoteRemoteName() { + if (!$this->isConnectedToRemote()) { + return null; + } + + return idx(last($this->path), 'remote'); + } + + + /** + * If this path contains a cycle, return a description of it. + * + * @return list|null Cycle, if the path contains one. + */ + public function getCycle() { + $last = last($this->path); + if (!$last) { + return null; + } + + if (empty($last['cycle'])) { + return null; + } + + $parts = array(); + foreach ($this->path as $key => $item) { + $parts[] = $key; + } + $parts[] = $item['name']; + $parts[] = pht('...'); + + return $parts; + } + +} diff --git a/src/unit/engine/ArcanistUnitTestEngine.php b/src/unit/engine/ArcanistUnitTestEngine.php index ad66bb90..e105f216 100644 --- a/src/unit/engine/ArcanistUnitTestEngine.php +++ b/src/unit/engine/ArcanistUnitTestEngine.php @@ -6,7 +6,7 @@ abstract class ArcanistUnitTestEngine extends Phobject { private $workingCopy; - private $paths; + private $paths = array(); private $arguments = array(); private $enableAsyncTests; private $enableCoverage; diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php index 8f4356ee..4525eeb4 100644 --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -38,7 +38,7 @@ final class ArcanistLandWorkflow extends ArcanistWorkflow { public function getCommandSynopses() { return phutil_console_format(<<readArguments(); + + $engine = null; + if ($this->isGit && !$this->isGitSvn) { + $engine = new ArcanistGitLandEngine(); + } + + if ($engine) { + $this->readEngineArguments(); + + $obsolete = array( + 'delete-remote', + 'update-with-merge', + 'update-with-rebase', + ); + + foreach ($obsolete as $flag) { + if ($this->getArgument($flag)) { + throw new ArcanistUsageException( + pht( + 'Flag "%s" is no longer supported under Git.', + '--'.$flag)); + } + } + + $this->requireCleanWorkingCopy(); + + $should_hold = $this->getArgument('hold'); + + $engine + ->setWorkflow($this) + ->setRepositoryAPI($this->getRepositoryAPI()) + ->setSourceRef($this->branch) + ->setTargetRemote($this->remote) + ->setTargetOnto($this->onto) + ->setShouldHold($should_hold) + ->setShouldKeep($this->keepBranch) + ->setShouldSquash($this->useSquash) + ->setShouldPreview($this->preview) + ->setBuildMessageCallback(array($this, 'buildEngineMessage')); + + $engine->execute(); + + if (!$should_hold && !$this->preview) { + $this->didPush(); + } + + return 0; + } + $this->validate(); try { @@ -256,6 +358,123 @@ EOTEXT return null; } + private function readEngineArguments() { + // NOTE: This is hard-coded for Git right now. + // TODO: Clean this up and move it into LandEngines. + + $onto = $this->getEngineOnto(); + $remote = $this->getEngineRemote(); + + // This just overwrites work we did earlier, but it has to be up in this + // class for now because other parts of the workflow still depend on it. + $this->onto = $onto; + $this->remote = $remote; + $this->ontoRemoteBranch = $this->remote.'/'.$onto; + } + + private function getEngineOnto() { + $onto = $this->getArgument('onto'); + if ($onto !== null) { + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", selected by the --onto flag.', + $onto)); + return $onto; + } + + $api = $this->getRepositoryAPI(); + $path = $api->getPathToUpstream($this->branch); + + if ($path->getLength()) { + $cycle = $path->getCycle(); + if ($cycle) { + $this->writeWarn( + pht('LOCAL CYCLE'), + pht( + 'Local branch tracks an upstream, but following it leads to a '. + 'local cycle; ignoring branch upstream.')); + + echo tsprintf( + "\n %s\n\n", + implode(' -> ', $cycle)); + + } else { + if ($path->isConnectedToRemote()) { + $onto = $path->getRemoteBranchName(); + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", selected by following tracking branches '. + 'upstream to the closest remote.', + $onto)); + return $onto; + } else { + $this->writeInfo( + pht('NO PATH TO UPSTREAM'), + pht( + 'Local branch tracks an upstream, but there is no path '. + 'to a remote; ignoring branch upstream.')); + } + } + } + + $config_key = 'arc.land.onto.default'; + $onto = $this->getConfigFromAnySource($config_key); + if ($onto !== null) { + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", selected by "%s" configuration.', + $onto, + $config_key)); + return $onto; + } + + $onto = 'master'; + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", the default target under git.', + $onto)); + return $onto; + } + + private function getEngineRemote() { + $remote = $this->getArgument('remote'); + if ($remote !== null) { + $this->writeInfo( + pht('REMOTE'), + pht( + 'Using remote "%s", selected by the --remote flag.', + $remote)); + return $remote; + } + + $api = $this->getRepositoryAPI(); + $path = $api->getPathToUpstream($this->branch); + + $remote = $path->getRemoteRemoteName(); + if ($remote !== null) { + $this->writeInfo( + pht('REMOTE'), + pht( + 'Using remote "%s", selected by following tracking branches '. + 'upstream to the closest remote.', + $remote)); + return $remote; + } + + $remote = 'origin'; + $this->writeInfo( + pht('REMOTE'), + pht( + 'Using remote "%s", the default remote under git.', + $remote)); + return $remote; + } + + private function readArguments() { $repository_api = $this->getRepositoryAPI(); $this->isGit = $repository_api instanceof ArcanistGitAPI; @@ -273,9 +492,12 @@ EOTEXT $branch = $this->getArgument('branch'); if (empty($branch)) { $branch = $this->getBranchOrBookmark(); - if ($branch) { $this->branchType = $this->getBranchType($branch); + + // TODO: This message is misleading when landing a detached head or + // a tag in Git. + echo pht("Landing current %s '%s'.", $this->branchType, $branch), "\n"; $branch = array($branch); } @@ -1032,9 +1254,7 @@ EOTEXT // We dispatch this event so we can run checks on the merged revision, // right before it gets pushed out. It's easier to do this in arc land // than to try to hook into git/hg. - $this->dispatchEvent( - ArcanistEventType::TYPE_LAND_WILLPUSHREVISION, - array()); + $this->didCommitMerge(); } catch (Exception $ex) { $this->executeCleanupAfterFailedPush(); throw $ex; @@ -1085,16 +1305,7 @@ EOTEXT $cmd)); } - $this->askForRepositoryUpdate(); - - $mark_workflow = $this->buildChildWorkflow( - 'close-revision', - array( - '--finalize', - '--quiet', - $this->revision['id'], - )); - $mark_workflow->run(); + $this->didPush(); echo "\n"; } @@ -1193,6 +1404,11 @@ EOTEXT $repository_api = $this->getRepositoryAPI(); if ($this->isGit) { $branch = $repository_api->getBranchName(); + + // If we don't have a branch name, just use whatever's at HEAD. + if (!strlen($branch) && !$this->isGitSvn) { + $branch = $repository_api->getWorkingCopyRevision(); + } } else if ($this->isHg) { $branch = $repository_api->getActiveBookmark(); if (!$branch) { @@ -1317,4 +1533,29 @@ EOTEXT } } + public function buildEngineMessage(ArcanistLandEngine $engine) { + // TODO: This is oh-so-gross. + $this->findRevision(); + $engine->setCommitMessageFile($this->messageFile); + } + + public function didCommitMerge() { + $this->dispatchEvent( + ArcanistEventType::TYPE_LAND_WILLPUSHREVISION, + array()); + } + + public function didPush() { + $this->askForRepositoryUpdate(); + + $mark_workflow = $this->buildChildWorkflow( + 'close-revision', + array( + '--finalize', + '--quiet', + $this->revision['id'], + )); + $mark_workflow->run(); + } + } diff --git a/src/workflow/ArcanistWorkflow.php b/src/workflow/ArcanistWorkflow.php index b1f828ce..e222914c 100644 --- a/src/workflow/ArcanistWorkflow.php +++ b/src/workflow/ArcanistWorkflow.php @@ -1361,7 +1361,7 @@ abstract class ArcanistWorkflow extends Phobject { fwrite(STDERR, $msg); } - final protected function writeInfo($title, $message) { + final public function writeInfo($title, $message) { $this->writeStatusMessage( phutil_console_format( "** %s ** %s\n", @@ -1369,7 +1369,7 @@ abstract class ArcanistWorkflow extends Phobject { $message)); } - final protected function writeWarn($title, $message) { + final public function writeWarn($title, $message) { $this->writeStatusMessage( phutil_console_format( "** %s ** %s\n", @@ -1377,7 +1377,7 @@ abstract class ArcanistWorkflow extends Phobject { $message)); } - final protected function writeOkay($title, $message) { + final public function writeOkay($title, $message) { $this->writeStatusMessage( phutil_console_format( "** %s ** %s\n",