revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 05:50:55 +01:00
Code Issues Releases Wiki Activity

Don't rediscover an entire branch when a new commit appears at HEAD

Browse source 
Summary:
The autoclose logic is currently doing a little too much work. We want to parse each commit at most twice:

  # When it first appears in the repository.
  # When it first appears on an autoclose branch.

These two events might not be distinct (i.e., it might first appear on an autoclose branch).

Currently, to discover commits initially appearing on autoclose branches, we check each branch, determine if it's an autoclose branch or not, and determine if the HEAD is already a known commit on an autoclose branch. This is correct so far, and allows us to ignore branches which either haven't changed or have commits at HEAD which we've already examined.

However, if an autoclose branch has a new commit, we start working backward through it. Prior to this patch, we only stop when we hit commits that we've already discovered lie on this branch. If the branch is new, none of the commits will be discovered on it (they're discovered in general, and likely discovered on other autoclose branches, but not discovered on this branch), so we'll parse all the way back to the root.

Instead, we want to stop when we hit commits that we've already discovered on //any// autoclose branch.

So do that.

Test Plan: Pushed a new branch, then pushed a new commit at HEAD. Ran discovery, verified we rediscovered only 1 commit, not every commit in history.

Reviewers: vrana, jungejason, aurelijus

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1389

Differential Revision: https://secure.phabricator.com/D2798
This commit is contained in:
epriestley 2012-06-19 13:47:01 -07:00
parent fabe52335e
commit e48830eecb
1 changed files with 7 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -255,11 +255,9 @@ final class PhabricatorRepositoryPullLocalDaemon
return true;
}
private static function isKnownCommitOnBranch(
private static function isKnownCommitOnAnyAutocloseBranch(
PhabricatorRepository $repository,
$target,
$branch,
$any_autoclose_branch = false) {
$target) {
$commit = id(new PhabricatorRepositoryCommit())->loadOneWhere(
'repositoryID = %s AND commitIdentifier = %s',
@ -271,14 +269,7 @@ final class PhabricatorRepositoryPullLocalDaemon
return false;
}
if ($any_autoclose_branch) {
if ($repository->shouldAutocloseCommit($commit, $data)) {
return true;
}
}
$branches = $data->getCommitDetail('seenOnBranches', array());
if (in_array($branch, $branches)) {
if ($repository->shouldAutocloseCommit($commit, $data)) {
return true;
}
@ -525,7 +516,6 @@ final class PhabricatorRepositoryPullLocalDaemon
$only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
$tracked_something = false;
$found_something = false;
foreach ($branches as $name => $commit) {
if (!$repository->shouldTrackBranch($name)) {
continue;
@ -557,7 +547,7 @@ final class PhabricatorRepositoryPullLocalDaemon
continue;
}
if (self::isKnownCommitOnBranch($repository, $commit, $name, true)) {
if (self::isKnownCommitOnAnyAutocloseBranch($repository, $commit)) {
continue;
}
@ -595,7 +585,9 @@ final class PhabricatorRepositoryPullLocalDaemon
}
$seen_parent[$parent] = true;
if ($branch !== null) {
$known = self::isKnownCommitOnBranch($repository, $parent, $branch);
$known = self::isKnownCommitOnAnyAutocloseBranch(
$repository,
$parent);
} else {
$known = self::isKnownCommit($repository, $parent);
}