From 46008ce3b3794fc79e6f8d68bb520366b31f7397 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 28 Jun 2012 14:00:00 -0700
Subject: [PATCH] Record initial branch when recording commits

Summary:
We have a race condition right now, where we may insert a commit without `seenOnBranches`. This means shouldAutocloseCommit will return false (since it's not on any autoclose branches) if the message parser runs fast enough, causing it to associate the commit but not close the revision. This happened for D2851.

Also prompt the user to repair broken repositories.

Test Plan: Ran discovery / repair. Ran discovery on new commits. Verified 'seenOnBranches' value. Deleted some data, verified "repair" error. Repaired repository.

Reviewers: jungejason, nh, vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2858
---
 .../PhabricatorRepositoryPullLocalDaemon.php  | 44 ++++++++++++++-----
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
index 68613e5cca..3ddb9a9a16 100644
--- a/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
+++ b/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
@@ -269,6 +269,16 @@ final class PhabricatorRepositoryPullLocalDaemon
       $target);
 
     if (!$commit) {
+      $callsign = $repository->getCallsign();
+
+      $console = PhutilConsole::getConsole();
+      $console->writeErr(
+        "WARNING: Repository '%s' is missing commits ('%s' is missing from ".
+        "history). Run '%s' to repair the repository.\n",
+        $callsign,
+        $target,
+        "bin/repository discover --repair {$callsign}");
+
       return false;
     }
 
@@ -308,15 +318,25 @@ final class PhabricatorRepositoryPullLocalDaemon
   private function recordCommit(
     PhabricatorRepository $repository,
     $commit_identifier,
-    $epoch) {
+    $epoch,
+    $branch = null) {
 
     $commit = new PhabricatorRepositoryCommit();
     $commit->setRepositoryID($repository->getID());
     $commit->setCommitIdentifier($commit_identifier);
     $commit->setEpoch($epoch);
 
+    $data = new PhabricatorRepositoryCommitData();
+    if ($branch) {
+      $data->setCommitDetail('seenOnBranches', array($branch));
+    }
+
     try {
-      $commit->save();
+      $commit->openTransaction();
+        $commit->save();
+        $data->setCommitID($commit->getID());
+        $data->save();
+      $commit->saveTransaction();
 
       $event = new PhabricatorTimelineEvent(
         'cmit',
@@ -344,11 +364,12 @@ final class PhabricatorRepositoryPullLocalDaemon
       if ($this->repair) {
         // Normally, the query should throw a duplicate key exception. If we
         // reach this in repair mode, we've actually performed a repair.
-        $this->log("Repaired commit '%s'.", $commit_identifier);
+        $this->log("Repaired commit '{$commit_identifier}'.");
       }
 
       $this->setCache($repository, $commit_identifier);
     } catch (AphrontQueryDuplicateKeyException $ex) {
+      $commit->killTransaction();
       // Ignore. This can happen because we discover the same new commit
       // more than once when looking at history, or because of races or
       // data inconsistency or cosmic radiation; in any case, we're still
@@ -577,7 +598,7 @@ final class PhabricatorRepositoryPullLocalDaemon
       }
 
       $this->log("Looking for new commits.");
-      $this->executeGitDiscoverCommit($repository, $commit);
+      $this->executeGitDiscoverCommit($repository, $commit, $name, false);
     }
 
     if (!$tracked_something) {
@@ -608,7 +629,7 @@ final class PhabricatorRepositoryPullLocalDaemon
       }
 
       $this->log("Looking for new autoclose commits.");
-      $this->executeGitDiscoverCommit($repository, $commit, $name);
+      $this->executeGitDiscoverCommit($repository, $commit, $name, true);
     }
   }
 
@@ -619,7 +640,8 @@ final class PhabricatorRepositoryPullLocalDaemon
   private function executeGitDiscoverCommit(
     PhabricatorRepository $repository,
     $commit,
-    $branch = null) {
+    $branch,
+    $autoclose) {
 
     $discover = array($commit);
     $insert = array($commit);
@@ -640,7 +662,7 @@ final class PhabricatorRepositoryPullLocalDaemon
           continue;
         }
         $seen_parent[$parent] = true;
-        if ($branch !== null) {
+        if ($autoclose) {
           $known = $this->isKnownCommitOnAnyAutocloseBranch(
             $repository,
             $parent);
@@ -659,10 +681,10 @@ final class PhabricatorRepositoryPullLocalDaemon
     }
 
     $n = count($insert);
-    if ($branch !== null) {
+    if ($autoclose) {
       $this->log("Found {$n} new autoclose commits on branch '{$branch}'.");
     } else {
-      $this->log("Found {$n} new commits.");
+      $this->log("Found {$n} new commits on branch '{$branch}'.");
     }
 
     while (true) {
@@ -670,10 +692,10 @@ final class PhabricatorRepositoryPullLocalDaemon
       $epoch = $stream->getCommitDate($target);
       $epoch = trim($epoch);
 
-      if ($branch !== null) {
+      if ($autoclose) {
         $this->updateCommit($repository, $target, $branch);
       } else {
-        $this->recordCommit($repository, $target, $epoch);
+        $this->recordCommit($repository, $target, $epoch, $branch);
       }
 
       if (empty($insert)) {