revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-29 18:22:41 +01:00
Code Issues Releases Wiki Activity

Record initial branch when recording commits

Browse source 
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
This commit is contained in:
epriestley 2012-06-28 14:00:00 -07:00
parent 1e3cd8afa8
commit 46008ce3b3
1 changed files with 33 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php
View file

@ -269,6 +269,16 @@ final class PhabricatorRepositoryPullLocalDaemon
$target); $target);
if (!$commit) { 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; return false;
} }
@ -308,15 +318,25 @@ final class PhabricatorRepositoryPullLocalDaemon
private function recordCommit( private function recordCommit(
PhabricatorRepository $repository, PhabricatorRepository $repository,
$commit_identifier, $commit_identifier,
$epoch) { $epoch,
$branch = null) {
$commit = new PhabricatorRepositoryCommit(); $commit = new PhabricatorRepositoryCommit();
$commit->setRepositoryID($repository->getID()); $commit->setRepositoryID($repository->getID());
$commit->setCommitIdentifier($commit_identifier); $commit->setCommitIdentifier($commit_identifier);
$commit->setEpoch($epoch); $commit->setEpoch($epoch);
$data = new PhabricatorRepositoryCommitData();
if ($branch) {
$data->setCommitDetail('seenOnBranches', array($branch));
}
try { try {
$commit->save(); $commit->openTransaction();
$commit->save();
$data->setCommitID($commit->getID());
$data->save();
$commit->saveTransaction();
$event = new PhabricatorTimelineEvent( $event = new PhabricatorTimelineEvent(
'cmit', 'cmit',
@ -344,11 +364,12 @@ final class PhabricatorRepositoryPullLocalDaemon
if ($this->repair) { if ($this->repair) {
// Normally, the query should throw a duplicate key exception. If we // Normally, the query should throw a duplicate key exception. If we
// reach this in repair mode, we've actually performed a repair. // 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); $this->setCache($repository, $commit_identifier);
} catch (AphrontQueryDuplicateKeyException $ex) { } catch (AphrontQueryDuplicateKeyException $ex) {
$commit->killTransaction();
// Ignore. This can happen because we discover the same new commit // Ignore. This can happen because we discover the same new commit
// more than once when looking at history, or because of races or // more than once when looking at history, or because of races or
// data inconsistency or cosmic radiation; in any case, we're still // 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->log("Looking for new commits.");
$this->executeGitDiscoverCommit($repository, $commit); $this->executeGitDiscoverCommit($repository, $commit, $name, false);
} }
if (!$tracked_something) { if (!$tracked_something) {
@ -608,7 +629,7 @@ final class PhabricatorRepositoryPullLocalDaemon
} }
$this->log("Looking for new autoclose commits."); $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( private function executeGitDiscoverCommit(
PhabricatorRepository $repository, PhabricatorRepository $repository,
$commit, $commit,
$branch = null) { $branch,
$autoclose) {
$discover = array($commit); $discover = array($commit);
$insert = array($commit); $insert = array($commit);
@ -640,7 +662,7 @@ final class PhabricatorRepositoryPullLocalDaemon
continue; continue;
} }
$seen_parent[$parent] = true; $seen_parent[$parent] = true;
if ($branch !== null) { if ($autoclose) {
$known = $this->isKnownCommitOnAnyAutocloseBranch( $known = $this->isKnownCommitOnAnyAutocloseBranch(
$repository, $repository,
$parent); $parent);
@ -659,10 +681,10 @@ final class PhabricatorRepositoryPullLocalDaemon
} }
$n = count($insert); $n = count($insert);
if ($branch !== null) { if ($autoclose) {
$this->log("Found {$n} new autoclose commits on branch '{$branch}'."); $this->log("Found {$n} new autoclose commits on branch '{$branch}'.");
} else { } else {
$this->log("Found {$n} new commits."); $this->log("Found {$n} new commits on branch '{$branch}'.");
} }
while (true) { while (true) {
@ -670,10 +692,10 @@ final class PhabricatorRepositoryPullLocalDaemon
$epoch = $stream->getCommitDate($target); $epoch = $stream->getCommitDate($target);
$epoch = trim($epoch); $epoch = trim($epoch);
if ($branch !== null) { if ($autoclose) {
$this->updateCommit($repository, $target, $branch); $this->updateCommit($repository, $target, $branch);
} else { } else {
$this->recordCommit($repository, $target, $epoch); $this->recordCommit($repository, $target, $epoch, $branch);
} }
if (empty($insert)) { if (empty($insert)) {