When landing changes in an empty repository, merge cleanly in Git

Summary:
Fixes T12876. Ref T13546. When you make the first change in a new Git repository, "arc land" currently can not merge it because there's nothing to merge into.

Support merging into the empty state formally, reachable by using "--into-empty" (which should be uncommon) or "arc land" in an empty repository.

Test Plan:
  - Used "arc land --into-empty --hold ..." to generate merges against the empty state under "squash" and "merge" strategies in Git.
    - Got sensible result commits with appropriate parents and content.

Maniphest Tasks: T13546, T12876

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

src/__phutil_library_map__.php

@@ -220,6 +220,8 @@ phutil_register_library_map(array(
     'ArcanistGitCommitSymbolCommitHardpointQuery' => 'ref/commit/ArcanistGitCommitSymbolCommitHardpointQuery.php',
     'ArcanistGitLandEngine' => 'land/engine/ArcanistGitLandEngine.php',
     'ArcanistGitLocalState' => 'repository/state/ArcanistGitLocalState.php',
+    'ArcanistGitRawCommit' => 'repository/raw/ArcanistGitRawCommit.php',
+    'ArcanistGitRawCommitTestCase' => 'repository/raw/__tests__/ArcanistGitRawCommitTestCase.php',
     'ArcanistGitUpstreamPath' => 'repository/api/ArcanistGitUpstreamPath.php',
     'ArcanistGitWorkingCopy' => 'workingcopy/ArcanistGitWorkingCopy.php',
     'ArcanistGitWorkingCopyRevisionHardpointQuery' => 'query/ArcanistGitWorkingCopyRevisionHardpointQuery.php',
@@ -1239,6 +1241,8 @@ phutil_register_library_map(array(
     'ArcanistGitCommitSymbolCommitHardpointQuery' => 'ArcanistWorkflowGitHardpointQuery',
     'ArcanistGitLandEngine' => 'ArcanistLandEngine',
     'ArcanistGitLocalState' => 'ArcanistRepositoryLocalState',
+    'ArcanistGitRawCommit' => 'Phobject',
+    'ArcanistGitRawCommitTestCase' => 'PhutilTestCase',
     'ArcanistGitUpstreamPath' => 'Phobject',
     'ArcanistGitWorkingCopy' => 'ArcanistWorkingCopy',
     'ArcanistGitWorkingCopyRevisionHardpointQuery' => 'ArcanistWorkflowGitHardpointQuery',

src/land/engine/ArcanistGitLandEngine.php

@@ -226,20 +226,18 @@ final class ArcanistGitLandEngine
     return $this->getLandTargetLocalCommit($target);
   }
 
-  private function updateWorkingCopy($into_commit) {
-    $api = $this->getRepositoryAPI();
-    if ($into_commit === null) {
-      throw new Exception('TODO: Author a new empty state.');
-    } else {
-      $api->execxLocal('checkout %s --', $into_commit);
-    }
-  }
-
   protected function executeMerge(ArcanistLandCommitSet $set, $into_commit) {
     $api = $this->getRepositoryAPI();
     $log = $this->getLogEngine();
 
-    $this->updateWorkingCopy($into_commit);
+    $is_empty = ($into_commit === null);
+
+    if ($is_empty) {
+      $empty_commit = ArcanistGitRawCommit::newEmptyCommit();
+      $into_commit = $api->writeRawCommit($empty_commit);
+    }
+
+    $api->execxLocal('checkout %s --', $into_commit);
 
     $commits = $set->getCommits();
     $max_commit = last($commits);
@@ -251,7 +249,8 @@ final class ArcanistGitLandEngine
     // as changes.
 
     list($changes) = $api->execxLocal(
-      'diff --no-ext-diff HEAD..%s --',
+      'diff --no-ext-diff %s..%s --',
+      $into_commit,
       $source_commit);
     $changes = trim($changes);
     if (!strlen($changes)) {
@@ -274,20 +273,30 @@ final class ArcanistGitLandEngine
         $this->getDisplayHash($source_commit),
         $max_commit->getDisplaySummary()));
 
+    $argv = array();
+    $argv[] = '--no-stat';
+    $argv[] = '--no-commit';
+
+    // When we're merging into the empty state, Git refuses to perform the
+    // merge until we tell it explicitly that we're doing something unusual.
+    if ($is_empty) {
+      $argv[] = '--allow-unrelated-histories';
+    }
+
+    if ($this->isSquashStrategy()) {
+      // NOTE: We're explicitly specifying "--ff" to override the presence
+      // of "merge.ff" options in user configuration.
+      $argv[] = '--ff';
+      $argv[] = '--squash';
+    } else {
+      $argv[] = '--no-ff';
+    }
+
+    $argv[] = '--';
+    $argv[] = $source_commit;
+
     try {
-
+      $api->execxLocal('merge %Ls', $argv);
-      if ($this->isSquashStrategy()) {
-        // 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_commit);
-      } else {
-        $api->execxLocal(
-          'merge --no-stat --no-commit --no-ff -- %s',
-          $source_commit);
-      }
     } catch (CommandException $ex) {
 
       // TODO: If we previously succeeded with at least one merge, we could
@@ -340,14 +349,23 @@ final class ArcanistGitLandEngine
     list($stdout) = $api->execxLocal('rev-parse --verify %s', 'HEAD');
     $new_cursor = trim($stdout);
 
-    if ($into_commit === null) {
+    if ($is_empty) {
+      // See T12876. If we're landing into the empty state, we just did a fake
+      // merge on top of an empty commit. We're now on a commit with all of the
+      // right details except that it has an extra empty commit as a parent.
+
+      // Create a new commit which is the same as the current HEAD, except that
+      // it doesn't have the extra parent.
+
+      $raw_commit = $api->readRawCommit($new_cursor);
       if ($this->isSquashStrategy()) {
-        throw new Exception(
+        $raw_commit->setParents(array());
-          pht('TODO: Rewrite HEAD to have no parents.'));
       } else {
-        throw new Exception(
+        $raw_commit->setParents(array($source_commit));
-          pht('TODO: Rewrite HEAD to have only source as a parent.'));
       }
+      $new_cursor = $api->writeRawCommit($raw_commit);
+
+      $api->execxLocal('checkout %s --', $new_cursor);
     }
 
     return $new_cursor;
@@ -720,9 +738,14 @@ final class ArcanistGitLandEngine
     );
   }
 
-  private function didHoldChanges() {
+  protected function didHoldChanges(
+    ArcanistRepositoryLocalState $state) {
     $log = $this->getLogEngine();
 
+    // TODO: This probably needs updates.
+
+    // TODO: We should refuse "--hold" if we stash.
+
     if ($this->getIsGitPerforce()) {
       $this->writeInfo(
         pht('HOLD'),
@@ -738,16 +761,15 @@ final class ArcanistGitLandEngine
         pht(
           'Holding change locally, it has not been pushed.'));
 
-      $push_command = csprintf(
+      $push_command = 'TODO: ...';
-        '$ git push -- %R %R:%R',
+      // csprintf(
-        $this->getTargetRemote(),
+      //   '$ git push -- %R %R:%R',
-        $this->mergedRef,
+      //   $this->getOntoRemote(),
-        $this->getTargetOnto());
+      //   $this->mergedRef,
+      //   $this->getOnto());
     }
 
-    $restore_command = csprintf(
+    $restore_command = 'TODO: ...';
-      '$ git checkout %R --',
-      $this->localRef);
 
     echo tsprintf(
       "\n%s\n\n".

src/land/engine/ArcanistLandEngine.php

@@ -1266,8 +1266,8 @@ abstract class ArcanistLandEngine extends Phobject {
       }
 
       if ($is_hold) {
-        $this->didHoldChanges();
+        $this->didHoldChanges($local_state);
-        $this->discardLocalState();
+        $local_state->discardLocalState();
       } else {
         $this->reconcileLocalState($into_commit, $local_state);
       }
@@ -1275,6 +1275,7 @@ abstract class ArcanistLandEngine extends Phobject {
       // TODO: Restore this.
       // $this->getWorkflow()->askForRepositoryUpdate();
 
+      // TODO: This is misleading under "--hold".
       $log->writeSuccess(
         pht('DONE'),
         pht('Landed changes.'));
@@ -1287,7 +1288,6 @@ abstract class ArcanistLandEngine extends Phobject {
     }
   }
 
-
   protected function validateArguments() {
     $log = $this->getLogEngine();
 

src/repository/api/ArcanistGitAPI.php

@@ -1753,4 +1753,22 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
       ->setRepositoryAPI($this);
   }
 
+  public function readRawCommit($hash) {
+    list($stdout) = $this->execxLocal(
+      'cat-file commit -- %s',
+      $hash);
+
+    return ArcanistGitRawCommit::newFromRawBlob($stdout);
+  }
+
+  public function writeRawCommit(ArcanistGitRawCommit $commit) {
+    $blob = $commit->getRawBlob();
+
+    $future = $this->execFutureLocal('hash-object -t commit --stdin -w');
+    $future->write($blob);
+    list($stdout) = $future->resolvex();
+
+    return trim($stdout);
+  }
+
 }

src/repository/raw/ArcanistGitRawCommit.php

@@ -0,0 +1,183 @@
+<?php
+
+final class ArcanistGitRawCommit
+  extends Phobject {
+
+  private $treeHash;
+  private $parents = array();
+  private $rawAuthor;
+  private $rawCommitter;
+  private $message;
+
+  const GIT_EMPTY_TREE_HASH = '4b825dc642cb6eb9a060e54bf8d69288fbee4904';
+
+  public static function newEmptyCommit() {
+    $raw = new self();
+    $raw->setTreeHash(self::GIT_EMPTY_TREE_HASH);
+    return $raw;
+  }
+
+  public static function newFromRawBlob($blob) {
+    $lines = phutil_split_lines($blob);
+
+    $seen = array();
+    $raw = new self();
+
+    $pattern = '(^(\w+) ([^\n]+)\n?\z)';
+    foreach ($lines as $key => $line) {
+      unset($lines[$key]);
+
+      $is_divider = ($line === "\n");
+      if ($is_divider) {
+        break;
+      }
+
+      $matches = null;
+      $ok = preg_match($pattern, $line, $matches);
+      if (!$ok) {
+        throw new Exception(
+          pht(
+            'Expected to match pattern "%s" against line "%s" in raw commit '.
+            'blob: %s',
+            $pattern,
+            $line,
+            $blob));
+      }
+
+      $label = $matches[1];
+      $value = $matches[2];
+
+      // Detect unexpected repeated lines.
+
+      if (isset($seen[$label])) {
+        switch ($label) {
+          case 'parent':
+            break;
+          default:
+            throw new Exception(
+              pht(
+                'Encountered two "%s" lines ("%s", "%s") while parsing raw '.
+                'commit blob, expected at most one: %s',
+                $label,
+                $seen[$label],
+                $line,
+                $blob));
+        }
+      } else {
+        $seen[$label] = $line;
+      }
+
+      switch ($label) {
+        case 'tree':
+          $raw->setTreeHash($value);
+          break;
+        case 'parent':
+          $raw->addParent($value);
+          break;
+        case 'author':
+          $raw->setRawAuthor($value);
+          break;
+        case 'committer':
+          $raw->setRawCommitter($value);
+          break;
+        default:
+          throw new Exception(
+            pht(
+              'Unknown attribute label "%s" in line "%s" while parsing raw '.
+              'commit blob: %s',
+              $label,
+              $line,
+              $blob));
+      }
+    }
+
+    $message = implode('', $lines);
+    $raw->setMessage($message);
+
+    return $raw;
+  }
+
+  public function getRawBlob() {
+    $out = array();
+
+    $tree = $this->getTreeHash();
+    if ($tree !== null) {
+      $out[] = sprintf("tree %s\n", $tree);
+    }
+
+    $parents = $this->getParents();
+    foreach ($parents as $parent) {
+      $out[] = sprintf("parent %s\n", $parent);
+    }
+
+    $raw_author = $this->getRawAuthor();
+    if ($raw_author !== null) {
+      $out[] = sprintf("author %s\n", $raw_author);
+    }
+
+    $raw_committer = $this->getRawCommitter();
+    if ($raw_committer !== null) {
+      $out[] = sprintf("committer %s\n", $raw_committer);
+    }
+
+    $out[] = "\n";
+
+    $message = $this->getMessage();
+    if ($message !== null) {
+      $out[] = $message;
+    }
+
+    return implode('', $out);
+  }
+
+  public function setTreeHash($tree_hash) {
+    $this->treeHash = $tree_hash;
+    return $this;
+  }
+
+  public function getTreeHash() {
+    return $this->treeHash;
+  }
+
+  public function setRawAuthor($raw_author) {
+    $this->rawAuthor = $raw_author;
+    return $this;
+  }
+
+  public function getRawAuthor() {
+    return $this->rawAuthor;
+  }
+
+  public function setRawCommitter($raw_committer) {
+    $this->rawCommitter = $raw_committer;
+    return $this;
+  }
+
+  public function getRawCommitter() {
+    return $this->rawCommitter;
+  }
+
+  public function setParents(array $parents) {
+    $this->parents = $parents;
+    return $this;
+  }
+
+  public function getParents() {
+    return $this->parents;
+  }
+
+  public function addParent($hash) {
+    $this->parents[] = $hash;
+    return $this;
+  }
+
+  public function setMessage($message) {
+    $this->message = $message;
+    return $this;
+  }
+
+  public function getMessage() {
+    return $this->message;
+  }
+
+}

src/repository/raw/__tests__/ArcanistGitRawCommitTestCase.php

@@ -0,0 +1,91 @@
+<?php
+
+final class ArcanistGitRawCommitTestCase
+  extends PhutilTestCase {
+
+  public function testGitRawCommitParser() {
+    $cases = array(
+      array(
+        'name' => 'empty',
+        'blob' => array(
+          'tree fcfd0454eac6a28c729aa6bf7d38a5f1efc5cc5d',
+          '',
+          '',
+        ),
+        'tree' => 'fcfd0454eac6a28c729aa6bf7d38a5f1efc5cc5d',
+      ),
+      array(
+        'name' => 'parents',
+        'blob' => array(
+          'tree 63ece8fd5a8283f1da2c14735d059669a09ba628',
+          'parent 4aebaaf60895c3f3dd32a8cadff00db2c8f74899',
+          'parent 0da1a2e17d921dc27ce9afa76b123cb4c8b73b17',
+          'author alice',
+          'committer alice',
+          '',
+          'Quack quack quack.',
+          '',
+        ),
+        'tree' => '63ece8fd5a8283f1da2c14735d059669a09ba628',
+        'parents' => array(
+          '4aebaaf60895c3f3dd32a8cadff00db2c8f74899',
+          '0da1a2e17d921dc27ce9afa76b123cb4c8b73b17',
+        ),
+        'author' => 'alice',
+        'committer' => 'alice',
+        'message' => "Quack quack quack.\n",
+      ),
+    );
+
+    foreach ($cases as $case) {
+      $name = $case['name'];
+      $blob = $case['blob'];
+
+      if (is_array($blob)) {
+        $blob = implode("\n", $blob);
+      }
+
+      $raw = ArcanistGitRawCommit::newFromRawBlob($blob);
+      $out = $raw->getRawBlob();
+
+      $this->assertEqual(
+        $blob,
+        $out,
+        pht(
+          'Expected read + write to produce the original raw Git commit '.
+          'blob in case "%s".',
+          $name));
+
+      $tree = idx($case, 'tree');
+      $this->assertEqual(
+        $tree,
+        $raw->getTreeHash(),
+        pht('Tree hashes in case "%s".', $name));
+
+      $parents = idx($case, 'parents', array());
+      $this->assertEqual(
+        $parents,
+        $raw->getParents(),
+        pht('Parents in case "%s".', $name));
+
+      $author = idx($case, 'author');
+      $this->assertEqual(
+        $author,
+        $raw->getRawAuthor(),
+        pht('Authors in case "%s".', $name));
+
+      $committer = idx($case, 'committer');
+      $this->assertEqual(
+        $committer,
+        $raw->getRawCommitter(),
+        pht('Committer in case "%s".', $name));
+
+      $message = idx($case, 'message', '');
+      $this->assertEqual(
+        $message,
+        $raw->getMessage(),
+        pht('Message in case "%s".', $name));
+    }
+  }
+
+}