Make WorkingCopyBlueprint responsible for performing merges

Summary:
Ref T182. Currently, the "RepositoryLand" operation is responsible for performing merges when landing a revision.

However, we'd like to be able to perform these merges in a larger set of cases in the future. For example:

  - After Releeph is revamped, when someone says "I want to merge bug fix X into stable branch Y", it would probably be nice to make that a Buildable and let tests run against it without requring that it actually be pushed anywhere.
  - Same deal if we want a merge-from-Diffusion or cherry-pick-from-Diffusion operation.
  - Similar deal if we want a "random web UI edits from Diffusion".

Move the merging part into WorkingCopy so more applications can share/use it in the future.

A big chunk of this is me making stuff up for now (the ol' undocumented dictionary full of arbitrary magic keys), but I anticipate formalizing it as we move along.

Test Plan: Pushed rGITTEST0d58eef3ce0fa5a10732d2efefc56aec126bc219 up from my local install via "Land Revision".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T182

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

src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php

@@ -237,8 +237,7 @@ final class DrydockWorkingCopyBlueprintImplementation
       $cmd = array();
       $arg = array();
 
-      $cmd[] = 'cd %s';
+      $interface->pushWorkingDirectory("{$root}/repo/{$directory}/");
-      $arg[] = "{$root}/repo/{$directory}/";
 
       $cmd[] = 'git clean -d --force';
       $cmd[] = 'git fetch';
@@ -285,6 +284,15 @@ final class DrydockWorkingCopyBlueprintImplementation
       if (idx($spec, 'default')) {
         $default = $directory;
       }
+
+      $merges = idx($spec, 'merges');
+      if ($merges) {
+        foreach ($merges as $merge) {
+          $this->applyMerge($interface, $merge);
+        }
+      }
+
+      $interface->popWorkingDirectory();
     }
 
     if ($default === null) {
@@ -333,7 +341,7 @@ final class DrydockWorkingCopyBlueprintImplementation
         $command_interface = $host_lease->getInterface($type);
 
         $path = $lease->getAttribute('workingcopy.default');
-        $command_interface->setWorkingDirectory($path);
+        $command_interface->pushWorkingDirectory($path);
 
         return $command_interface;
     }
@@ -407,5 +415,28 @@ final class DrydockWorkingCopyBlueprintImplementation
     return true;
   }
 
+  private function applyMerge(
+    DrydockCommandInterface $interface,
+    array $merge) {
+
+    $src_uri = $merge['src.uri'];
+    $src_ref = $merge['src.ref'];
+
+    $interface->execx(
+      'git fetch --no-tags -- %s +%s:%s',
+      $src_uri,
+      $src_ref,
+      $src_ref);
+
+    try {
+      $interface->execx(
+        'git merge --no-stat --squash --ff-only -- %s',
+        $src_ref);
+    } catch (CommandException $ex) {
+      // TODO: Specifically note this as a merge conflict.
+      throw $ex;
+    }
+  }
+
 
 }

src/applications/drydock/interface/command/DrydockCommandInterface.php

@@ -4,15 +4,27 @@ abstract class DrydockCommandInterface extends DrydockInterface {
 
   const INTERFACE_TYPE = 'command';
 
-  private $workingDirectory;
+  private $workingDirectoryStack = array();
 
-  public function setWorkingDirectory($working_directory) {
+  public function pushWorkingDirectory($working_directory) {
-    $this->workingDirectory = $working_directory;
+    $this->workingDirectoryStack[] = $working_directory;
     return $this;
   }
 
-  public function getWorkingDirectory() {
+  public function popWorkingDirectory() {
-    return $this->workingDirectory;
+    if (!$this->workingDirectoryStack) {
+      throw new Exception(
+        pht(
+          'Unable to pop working directory, directory stack is empty.'));
+    }
+    return array_pop($this->workingDirectoryStack);
+  }
+
+  public function peekWorkingDirectory() {
+    if ($this->workingDirectoryStack) {
+      return last($this->workingDirectoryStack);
+    }
+    return null;
   }
 
   final public function getInterfaceType() {
@@ -38,12 +50,14 @@ abstract class DrydockCommandInterface extends DrydockInterface {
   abstract public function getExecFuture($command);
 
   protected function applyWorkingDirectoryToArgv(array $argv) {
-    if ($this->getWorkingDirectory() !== null) {
+    $directory = $this->peekWorkingDirectory();
+
+    if ($directory !== null) {
       $cmd = $argv[0];
       $cmd = "(cd %s && {$cmd})";
       $argv = array_merge(
         array($cmd),
-        array($this->getWorkingDirectory()),
+        array($directory),
         array_slice($argv, 1));
     }
 

src/applications/drydock/operation/DrydockLandRepositoryOperation.php

@@ -35,6 +35,28 @@ final class DrydockLandRepositoryOperation
     }
   }
 
+  public function getWorkingCopyMerges(DrydockRepositoryOperation $operation) {
+    $repository = $operation->getRepository();
+    $merges = array();
+
+    $object = $operation->getObject();
+    if ($object instanceof DifferentialRevision) {
+      $diff = $this->loadDiff($operation);
+      $merges[] = array(
+        'src.uri' => $repository->getStagingURI(),
+        'src.ref' => $diff->getStagingRef(),
+      );
+    } else {
+      throw new Exception(
+        pht(
+          'Invalid or unknown object ("%s") for land operation, expected '.
+          'Differential Revision.',
+          $operation->getObjectPHID()));
+    }
+
+    return $merges;
+  }
+
   public function applyOperation(
     DrydockRepositoryOperation $operation,
     DrydockInterface $interface) {
@@ -48,36 +70,7 @@ final class DrydockLandRepositoryOperation
     if ($object instanceof DifferentialRevision) {
       $revision = $object;
 
-      $diff_phid = $operation->getProperty('differential.diffPHID');
+      $diff = $this->loadDiff($operation);
-
-      $diff = id(new DifferentialDiffQuery())
-        ->setViewer($viewer)
-        ->withPHIDs(array($diff_phid))
-        ->executeOne();
-      if (!$diff) {
-        throw new Exception(
-          pht(
-            'Unable to load diff "%s".',
-            $diff_phid));
-      }
-
-      $diff_revid = $diff->getRevisionID();
-      $revision_id = $revision->getID();
-      if ($diff_revid != $revision_id) {
-        throw new Exception(
-          pht(
-            'Diff ("%s") has wrong revision ID ("%s", expected "%s").',
-            $diff_phid,
-            $diff_revid,
-            $revision_id));
-      }
-
-      $cmd[] = 'git fetch --no-tags -- %s +%s:%s';
-      $arg[] = $repository->getStagingURI();
-      $arg[] = $diff->getStagingRef();
-      $arg[] = $diff->getStagingRef();
-
-      $merge_src = $diff->getStagingRef();
 
       $dict = $diff->getDiffAuthorshipDict();
       $author_name = idx($dict, 'authorName');
@@ -104,7 +97,6 @@ final class DrydockLandRepositoryOperation
     switch ($type) {
       case 'branch':
         $push_dst = 'refs/heads/'.$name;
-        $merge_dst = 'refs/remotes/origin/'.$name;
         break;
       default:
         throw new Exception(
@@ -116,30 +108,24 @@ final class DrydockLandRepositoryOperation
 
     $committer_info = $this->getCommitterInfo($operation);
 
-    $cmd[] = 'git checkout %s';
+    // NOTE: We're doing this commit with "-F -" so we don't run into trouble
-    $arg[] = $merge_dst;
+    // with enormous commit messages which might otherwise exceed the maximum
+    // size of a command.
 
-    $cmd[] = 'git merge --no-stat --squash --ff-only -- %s';
+    $future = $interface->getExecFuture(
-    $arg[] = $merge_src;
+      'git -c user.name=%s -c user.email=%s commit --author %s -F - --',
+      $committer_info['name'],
+      $committer_info['email'],
+      "{$author_name} <{$author_email}>");
 
-    $cmd[] = 'git -c user.name=%s -c user.email=%s commit --author %s -m %s';
+    $future
+      ->write($commit_message)
+      ->resolvex();
 
-    $arg[] = $committer_info['name'];
+    $interface->execx(
-    $arg[] = $committer_info['email'];
+      'git push origin -- %s:%s',
-
+      'HEAD',
-    $arg[] = "{$author_name} <{$author_email}>";
+      $push_dst);
-    $arg[] = $commit_message;
-
-    $cmd[] = 'git push origin -- %s:%s';
-    $arg[] = 'HEAD';
-    $arg[] = $push_dst;
-
-    $cmd = implode(' && ', $cmd);
-    $argv = array_merge(array($cmd), $arg);
-
-    $result = call_user_func_array(
-      array($interface, 'execx'),
-      $argv);
   }
 
   private function getCommitterInfo(DrydockRepositoryOperation $operation) {
@@ -172,4 +158,35 @@ final class DrydockLandRepositoryOperation
     );
   }
 
+  private function loadDiff(DrydockRepositoryOperation $operation) {
+    $viewer = $this->getViewer();
+    $revision = $operation->getObject();
+
+    $diff_phid = $operation->getProperty('differential.diffPHID');
+
+    $diff = id(new DifferentialDiffQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($diff_phid))
+      ->executeOne();
+    if (!$diff) {
+      throw new Exception(
+        pht(
+          'Unable to load diff "%s".',
+          $diff_phid));
+    }
+
+    $diff_revid = $diff->getRevisionID();
+    $revision_id = $revision->getID();
+    if ($diff_revid != $revision_id) {
+      throw new Exception(
+        pht(
+          'Diff ("%s") has wrong revision ID ("%s", expected "%s").',
+          $diff_phid,
+          $diff_revid,
+          $revision_id));
+    }
+
+    return $diff;
+  }
+
 }

src/applications/drydock/operation/DrydockRepositoryOperationType.php

@@ -16,6 +16,10 @@ abstract class DrydockRepositoryOperationType extends Phobject {
     DrydockRepositoryOperation $operation,
     PhabricatorUser $viewer);
 
+  public function getWorkingCopyMerges(DrydockRepositoryOperation $operation) {
+    return array();
+  }
+
   final public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
     return $this;

src/applications/drydock/storage/DrydockRepositoryOperation.php

@@ -158,6 +158,11 @@ final class DrydockRepositoryOperation extends DrydockDAO
     return false;
   }
 
+  public function getWorkingCopyMerges() {
+    return $this->getImplementation()->getWorkingCopyMerges(
+      $this);
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */
 

src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php

@@ -53,6 +53,9 @@ final class DrydockRepositoryOperationUpdateWorker
     // waiting for a lease we're holding.
 
     try {
+      $operation->getImplementation()
+        ->setViewer($viewer);
+
       $lease = $this->loadWorkingCopyLease($operation);
 
       $interface = $lease->getInterface(
@@ -61,9 +64,6 @@ final class DrydockRepositoryOperationUpdateWorker
       // No matter what happens here, destroy the lease away once we're done.
       $lease->releaseOnDestruction(true);
 
-      $operation->getImplementation()
-        ->setViewer($viewer);
-
       $operation->applyOperation($interface);
 
     } catch (PhabricatorWorkerYieldException $ex) {
@@ -166,6 +166,8 @@ final class DrydockRepositoryOperationUpdateWorker
             $target));
     }
 
+    $spec['merges'] = $operation->getWorkingCopyMerges();
+
     $map = array();
     $map[$repository->getCloneName()] = array(
       'phid' => $repository->getPHID(),