diff --git a/src/land/ArcanistGitLandEngine.php b/src/land/ArcanistGitLandEngine.php --- a/src/land/ArcanistGitLandEngine.php +++ b/src/land/ArcanistGitLandEngine.php @@ -9,6 +9,14 @@ private $mergedRef; private $restoreWhenDestroyed; + public function parseArguments() { + $onto = $this->getEngineOnto(); + $this->setTargetOnto($onto); + + $remote = $this->getEngineRemote(); + $this->setTargetRemote($remote); + } + public function execute() { $this->verifySourceAndTargetExist(); $this->fetchTarget(); @@ -615,4 +623,114 @@ return !$err; } + private function getEngineOnto() { + $source_ref = $this->getSourceRef(); + + $onto = $this->getOntoArgument(); + if ($onto !== null) { + $this->writeInfo( + pht('TARGET'), + pht( + 'Landing onto "%s", selected with the "--onto" flag.', + $onto)); + return $onto; + } + + $api = $this->getRepositoryAPI(); + $path = $api->getPathToUpstream($source_ref); + + 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.')); + } + } + } + + $workflow = $this->getWorkflow(); + + $config_key = 'arc.land.onto.default'; + $onto = $workflow->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() { + $source_ref = $this->getSourceRef(); + + $remote = $this->getRemoteArgument(); + if ($remote !== null) { + $this->writeInfo( + pht('REMOTE'), + pht( + 'Using remote "%s", selected with the "--remote" flag.', + $remote)); + return $remote; + } + + $api = $this->getRepositoryAPI(); + $path = $api->getPathToUpstream($source_ref); + + $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; + } + } diff --git a/src/land/ArcanistLandEngine.php b/src/land/ArcanistLandEngine.php --- a/src/land/ArcanistLandEngine.php +++ b/src/land/ArcanistLandEngine.php @@ -13,6 +13,8 @@ private $shouldSquash; private $shouldDeleteRemote; private $shouldPreview; + private $remoteArgument; + private $ontoArgument; // TODO: This is really grotesque. private $buildMessageCallback; @@ -117,6 +119,25 @@ return $this->commitMessageFile; } + final public function setRemoteArgument($remote_argument) { + $this->remoteArgument = $remote_argument; + return $this; + } + + final public function getRemoteArgument() { + return $this->remoteArgument; + } + + final public function setOntoArgument($onto_argument) { + $this->ontoArgument = $onto_argument; + return $this; + } + + final public function getOntoArgument() { + return $this->ontoArgument; + } + + abstract public function parseArguments(); abstract public function execute(); abstract protected function getLandingCommits(); diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -224,23 +224,28 @@ } if ($engine) { - $this->readEngineArguments(); - $this->requireCleanWorkingCopy(); - $should_hold = $this->getArgument('hold'); + $remote_arg = $this->getArgument('remote'); + $onto_arg = $this->getArgument('onto'); $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) + ->setRemoteArgument($remote_arg) + ->setOntoArgument($onto_arg) ->setBuildMessageCallback(array($this, 'buildEngineMessage')); + // The goal here is to raise errors with flags early (which is cheap), + // before we test if the working copy is clean (which can be slow). This + // could probably be structured more cleanly. + + $engine->parseArguments(); + $this->requireCleanWorkingCopy(); $engine->execute(); if (!$should_hold && !$this->preview) { @@ -337,123 +342,6 @@ return $refspec; } - 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;