Pull git upstream-path logic into a separate class

Summary:
Ref T9661. I need to reuse this to fix the complex workflow described in T9661 where we need to follow multiple paths to the upstream and cascade updates across them.

Pull the logic into a separate class to make this easier and less copy/pastey.

This shouldn't change any behavior.

Test Plan: Ran `arc land --preview` from detached head, remote-tracking branch, non-tracking branch, local-tracking branch. Selection of target/remote seemed correct in all cases.

Reviewers: chad

Reviewed By: chad

Subscribers: edibiase

Maniphest Tasks: T9661

Differential Revision: https://secure.phabricator.com/D14360

src/__phutil_library_map__.php

@@ -113,6 +113,7 @@ phutil_register_library_map(array(
     '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',
@@ -401,6 +402,7 @@ phutil_register_library_map(array(
     'ArcanistGetConfigWorkflow' => 'ArcanistWorkflow',
     'ArcanistGitAPI' => 'ArcanistRepositoryAPI',
     'ArcanistGitLandEngine' => 'ArcanistLandEngine',
+    'ArcanistGitUpstreamPath' => 'Phobject',
     'ArcanistGlobalVariableXHPASTLinterRule' => 'ArcanistXHPASTLinterRule',
     'ArcanistGoLintLinter' => 'ArcanistExternalLinter',
     'ArcanistGoLintLinterTestCase' => 'ArcanistExternalLinterTestCase',

src/repository/api/ArcanistGitAPI.php

@@ -1342,7 +1342,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
    */
   public function getPathToUpstream($start) {
     $cursor = $start;
-    $path = array();
+    $path = new ArcanistGitUpstreamPath();
     while (true) {
       list($err, $upstream) = $this->execManualLocal(
         'rev-parse --symbolic-full-name %s@{upstream}',
@@ -1358,13 +1358,15 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
       if (preg_match('(^refs/heads/)', $upstream)) {
         $upstream = preg_replace('(^refs/heads/)', '', $upstream);
 
-        $is_cycle = isset($path[$upstream]);
+        $is_cycle = $path->getUpstream($upstream);
 
-        $path[$cursor] = array(
+        $path->addUpstream(
-          'type' => 'local',
+          $cursor,
+          array(
+            'type' => ArcanistGitUpstreamPath::TYPE_LOCAL,
             'name' => $upstream,
             'cycle' => $is_cycle,
-        );
+          ));
 
         if ($is_cycle) {
           // We ran into a local cycle, so we're done.
@@ -1380,11 +1382,13 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
         $upstream = preg_replace('(^refs/remotes/)', '', $upstream);
         list($remote, $branch) = explode('/', $upstream, 2);
 
-        $path[$cursor] = array(
+        $path->addUpstream(
-          'type' => 'remote',
+          $cursor,
+          array(
+            'type' => ArcanistGitUpstreamPath::TYPE_REMOTE,
             'name' => $branch,
             'remote' => $remote,
-        );
+          ));
 
         // We found a remote, so we're done.
         break;

src/repository/api/ArcanistGitUpstreamPath.php

@@ -0,0 +1,82 @@
+<?php
+
+final class ArcanistGitUpstreamPath extends Phobject {
+
+  private $path = array();
+
+  const TYPE_LOCAL = 'local';
+  const TYPE_REMOTE = 'remote';
+
+
+  public function addUpstream($key, array $spec) {
+    $this->path[$key] = $spec;
+    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 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<string>|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;
+  }
+
+}

src/workflow/ArcanistLandWorkflow.php

@@ -386,9 +386,9 @@ EOTEXT
     $api = $this->getRepositoryAPI();
     $path = $api->getPathToUpstream($this->branch);
 
-    if ($path) {
+    if ($path->getLength()) {
-      $last = last($path);
+      $cycle = $path->getCycle();
-      if (isset($last['cycle'])) {
+      if ($cycle) {
         $this->writeWarn(
           pht('LOCAL CYCLE'),
           pht(
@@ -397,11 +397,11 @@ EOTEXT
 
         echo tsprintf(
           "\n    %s\n\n",
-          $this->formatUpstreamPathCycle($path));
+          implode(' -> ', $cycle));
 
       } else {
-        if ($last['type'] == 'remote') {
+        if ($path->isConnectedToRemote()) {
-          $onto = $last['name'];
+          $onto = $path->getRemoteBranchName();
           $this->writeInfo(
             pht('TARGET'),
             pht(
@@ -454,10 +454,8 @@ EOTEXT
     $api = $this->getRepositoryAPI();
     $path = $api->getPathToUpstream($this->branch);
 
-    if ($path) {
+    $remote = $path->getRemoteRemoteName();
-      $last = last($path);
+    if ($remote !== null) {
-      if ($last['type'] == 'remote') {
-        $remote = $last['remote'];
       $this->writeInfo(
         pht('REMOTE'),
         pht(
@@ -466,7 +464,6 @@ EOTEXT
           $remote));
       return $remote;
     }
-    }
 
     $remote = 'origin';
     $this->writeInfo(
@@ -477,17 +474,6 @@ EOTEXT
     return $remote;
   }
 
-  private function formatUpstreamPathCycle(array $cycle) {
-    $parts = array();
-    foreach ($cycle as $key => $value) {
-      $parts[] = $key;
-    }
-    $parts[] = idx(last($cycle), 'name');
-    $parts[] = pht('...');
-
-    return implode(' -> ', $parts);
-  }
-
 
   private function readArguments() {
     $repository_api = $this->getRepositoryAPI();