Support "arc land --pick" to pick specific changes out of a sequence

Summary:
Ref T13546. If you have "feature1", "feature2", etc., "arc land feature4" will now land the entire sequence.

Provide "arc land --pick feature4" to work more like the old "arc land" did. This cherry-picks the commits associated with "feature4", then cascades onto the ancestor of the range.

Test Plan: Ran "arc land --pick land14" to pick a change out of a stack.

Maniphest Tasks: T13546

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

src/land/ArcanistLandCommitSet.php

@@ -5,6 +5,7 @@ final class ArcanistLandCommitSet
 
   private $revisionRef;
   private $commits;
+  private $isPick;
 
   public function setRevisionRef(ArcanistRevisionRef $revision_ref) {
     $this->revisionRef = $revision_ref;
@@ -49,4 +50,23 @@ final class ArcanistLandCommitSet
     return false;
   }
 
+  public function hasDirectSymbols() {
+    foreach ($this->commits as $commit) {
+      if ($commit->getDirectSymbols()) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  public function setIsPick($is_pick) {
+    $this->isPick = $is_pick;
+    return $this;
+  }
+
+  public function getIsPick() {
+    return $this->isPick;
+  }
+
 }

src/land/engine/ArcanistGitLandEngine.php

@@ -122,6 +122,7 @@ final class ArcanistGitLandEngine
       return;
     }
 
+    $min_commit = head($set->getCommits())->getHash();
     $old_commit = last($set->getCommits())->getHash();
     $new_commit = $into_commit;
 
@@ -143,17 +144,38 @@ final class ArcanistGitLandEngine
           'Rebasing "%s" onto landed state...',
           $branch_name));
 
+      // If we used "--pick" to select this commit, we want to rebase branches
+      // that descend from it onto its ancestor, not onto the landed change.
+
+      // For example, if the change sequence was "W", "X", "Y", "Z" and we
+      // landed "Y" onto "master" using "--pick", we want to rebase "Z" onto
+      // "X" (so "W" and "X", which it will often depend on, are still
+      // its ancestors), not onto the new "master".
+
+      if ($set->getIsPick()) {
+        $rebase_target = $min_commit.'^';
+      } else {
+        $rebase_target = $new_commit;
+      }
+
       try {
         $api->execxLocal(
           'rebase --onto %s -- %s %s',
-          $new_commit,
+          $rebase_target,
           $old_commit,
           $branch_name);
       } catch (CommandException $ex) {
-        // TODO: If we have a stashed state or are not running in incremental
+        $api->execManualLocal('rebase --abort');
-        // mode: abort the rebase, restore the local state, and pop the stash.
+        $api->execManualLocal('reset --hard HEAD --');
-        // Otherwise, drop the user out here.
+
-        throw $ex;
+        $log->writeWarning(
+          pht('REBASE CONFLICT'),
+          pht(
+            'Branch "%s" does not rebase cleanly from "%s" onto '.
+            '"%s", skipping.',
+            $branch_name,
+            $this->getDisplayHash($old_commit),
+            $this->getDisplayHash($rebase_target)));
       }
     }
   }

src/land/engine/ArcanistLandEngine.php

@@ -29,6 +29,7 @@ abstract class ArcanistLandEngine
 
   private $localState;
   private $hasUnpushedChanges;
+  private $pickArgument;
 
   final public function setOntoRemote($onto_remote) {
     $this->ontoRemote = $onto_remote;
@@ -75,6 +76,15 @@ abstract class ArcanistLandEngine
     return $this->intoEmpty;
   }
 
+  final public function setPickArgument($pick_argument) {
+    $this->pickArgument = $pick_argument;
+    return $this;
+  }
+
+  final public function getPickArgument() {
+    return $this->pickArgument;
+  }
+
   final public function setIntoLocal($into_local) {
     $this->intoLocal = $into_local;
     return $this;
@@ -1360,6 +1370,13 @@ abstract class ArcanistLandEngine
     $strategy = $this->selectMergeStrategy();
     $this->setStrategy($strategy);
 
+    $is_pick = $this->getPickArgument();
+    if ($is_pick && !$this->isSquashStrategy()) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'You can not "--pick" changes under the "merge" strategy.'));
+    }
+
     // Build the symbol ref here (which validates the format of the symbol),
     // but don't load the object until later on when we're sure we actually
     // need it, since loading it requires a relatively expensive Conduit call.
@@ -1486,16 +1503,7 @@ abstract class ArcanistLandEngine
           continue;
         }
 
-        $symbols = null;
+        if ($set->hasDirectSymbols()) {
-        foreach ($set->getCommits() as $commit) {
-          $commit_symbols = $commit->getDirectSymbols();
-          if ($commit_symbols) {
-            $symbols = $commit_symbols;
-            break;
-          }
-        }
-
-        if ($symbols) {
           continue;
         }
 
@@ -1524,6 +1532,18 @@ abstract class ArcanistLandEngine
     // set of commits, and try to confirm that the state we're about to land
     // is the current state in Differential.
 
+    $is_pick = $this->getPickArgument();
+    if ($is_pick) {
+      foreach ($sets as $key => $set) {
+        if ($set->hasDirectSymbols()) {
+          $set->setIsPick(true);
+          continue;
+        }
+
+        unset($sets[$key]);
+      }
+    }
+
     return $sets;
   }
 

src/workflow/ArcanistLandWorkflow.php

@@ -32,7 +32,9 @@ name, a bookmark name, a topic name, a raw commit hash, a symbolic reference,
 etc.
 
 When you provide a __ref__, all unpublished changes which are present in
-ancestors of that __ref__ will be selected for publishing.
+ancestors of that __ref__ will be selected for publishing. (With the
+**--pick** flag, only the unpublished changes you directly reference will be
+selected.)
 
 For example, if you provide local branch "feature3" as a __ref__ argument, that
 may also select the changes in "feature1" and "feature2" (if they are ancestors
@@ -222,6 +224,11 @@ EOTEXT
               'complicated changes by allowing you to make progress one '.
               'step at a time.'),
           )),
+      $this->newWorkflowArgument('pick')
+        ->setHelp(
+          pht(
+            'Land only the changes directly named by arguments, instead '.
+            'of all reachable ancestors.')),
       $this->newWorkflowArgument('ref')
         ->setWildcard(true),
     );
@@ -308,6 +315,7 @@ EOTEXT
 
     $revision = $this->getArgument('revision');
     $strategy = $this->getArgument('strategy');
+    $pick = $this->getArgument('pick');
 
     $land_engine
       ->setViewer($this->getViewer())
@@ -324,6 +332,7 @@ EOTEXT
       ->setIntoEmptyArgument($into_empty)
       ->setIntoLocalArgument($into_local)
       ->setIntoArgument($into)
+      ->setPickArgument($pick)
       ->setIsIncremental($is_incremental)
       ->setRevisionSymbol($revision);
 

src/workflow/ArcanistWorkWorkflow.php

@@ -38,7 +38,7 @@ branch or bookmark that exists in the working copy. If it finds one, it will
 switch to it. If it does not find one, it will attempt to create a new branch
 or bookmark.
 
-When "arc work" creates a branch or bookmark, it will use "--start" as the
+When "arc work" creates a branch or bookmark, it will use **--start** as the
 branchpoint if it is provided. Otherwise, the current working copy state will
 serve as the starting point.
 EOHELP