mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-18 21:02:41 +01:00
Allow Git repositories to track only some branches
Summary: Some installs use Git as the backbone of a CI framework or use a Git remote to share patches. The tracker scripts currently recognize associated revisions as "Committed" when they appear in any branch, even if that branch is "alincoln-personal-development_test_hack" or whatever. To address the broadest need here, allow Git repositories to be configured to track only certain branches instead of all branches. This doesn't allow you to import a branch into Diffusion but ignore it in Differential. Supporting that is somewhat technically complicated because the parser currently goes like this: - Look at HEAD of all branches. - For any commits we haven't seen before, follow them back to something we have seen (or the root). - "Discover" everything new. Since this doesn't track <branch, commit> pairs, we currently don't have enough information to tell when a commit appears in a branch for the first time, so we don't have anywhere we can put a test for whether that branch is tracked and do the Differential hook only if it is. However, I think this cruder patch satisfies most of the need and is simple and obvious in its implementation. See also D1263. Test Plan: - Updated a Git repository with various filters: "", "master, remote", "derp", " ,,, master ,,,,," - Edited SVN and Mercurial repositories to verify they didn't get caught in the crossfire. - Ran daemon in debug mode on libphutil with filter "derp", got exception about no tracked branches. Ran with filter "master", got tracking. Ran with no filter, got tracking. - Looked at Diffusion with "derp" and "master", saw no branches and "master" respectively. - Added unit tests to cover filtering logic. Reviewers: btrahan, jungejason, nh, fratrik Reviewed By: fratrik CC: aran, fratrik, epriestley Maniphest Tasks: T270 Differential Revision: https://secure.phabricator.com/D1290
This commit is contained in:
parent
ec1df21bef
commit
99231ebba4
7 changed files with 104 additions and 11 deletions
|
@ -1,7 +1,7 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Copyright 2011 Facebook, Inc.
|
* Copyright 2012 Facebook, Inc.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -33,6 +33,10 @@ final class DiffusionGitBranchQuery extends DiffusionBranchQuery {
|
||||||
|
|
||||||
$branches = array();
|
$branches = array();
|
||||||
foreach ($branch_list as $name => $head) {
|
foreach ($branch_list as $name => $head) {
|
||||||
|
if (!$repository->shouldTrackBranch($name)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
$branch = new DiffusionBranchInformation();
|
$branch = new DiffusionBranchInformation();
|
||||||
$branch->setName($name);
|
$branch->setName($name);
|
||||||
$branch->setHeadCommitIdentifier($head);
|
$branch->setHeadCommitIdentifier($head);
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Copyright 2011 Facebook, Inc.
|
* Copyright 2012 Facebook, Inc.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -229,6 +229,7 @@ class PhabricatorRepositoryEditController
|
||||||
|
|
||||||
$has_branches = ($is_git || $is_mercurial);
|
$has_branches = ($is_git || $is_mercurial);
|
||||||
$has_local = ($is_git || $is_mercurial);
|
$has_local = ($is_git || $is_mercurial);
|
||||||
|
$has_branch_filter = ($is_git);
|
||||||
$has_http_support = $is_svn;
|
$has_http_support = $is_svn;
|
||||||
|
|
||||||
if ($request->isFormPost()) {
|
if ($request->isFormPost()) {
|
||||||
|
@ -238,6 +239,14 @@ class PhabricatorRepositoryEditController
|
||||||
if ($has_local) {
|
if ($has_local) {
|
||||||
$repository->setDetail('local-path', $request->getStr('path'));
|
$repository->setDetail('local-path', $request->getStr('path'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($has_branch_filter) {
|
||||||
|
$branch_filter = $request->getStrList('branch-filter');
|
||||||
|
$branch_filter = array_fill_keys($branch_filter, true);
|
||||||
|
|
||||||
|
$repository->setDetail('branch-filter', $branch_filter);
|
||||||
|
}
|
||||||
|
|
||||||
$repository->setDetail(
|
$repository->setDetail(
|
||||||
'pull-frequency',
|
'pull-frequency',
|
||||||
max(1, $request->getInt('frequency')));
|
max(1, $request->getInt('frequency')));
|
||||||
|
@ -548,6 +557,22 @@ class PhabricatorRepositoryEditController
|
||||||
->setError($e_path));
|
->setError($e_path));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ($has_branch_filter) {
|
||||||
|
$branch_filter_str = implode(
|
||||||
|
', ',
|
||||||
|
array_keys($repository->getDetail('branch-filter', array())));
|
||||||
|
$form
|
||||||
|
->appendChild(
|
||||||
|
id(new AphrontFormTextControl())
|
||||||
|
->setName('branch-filter')
|
||||||
|
->setLabel('Track Only')
|
||||||
|
->setValue($branch_filter_str)
|
||||||
|
->setCaption(
|
||||||
|
'Optional list of branches to track. Other branches will be '.
|
||||||
|
'completely ignored. If left empty, all branches are tracked. '.
|
||||||
|
'Example: <tt>master, release</tt>'));
|
||||||
|
}
|
||||||
|
|
||||||
$form
|
$form
|
||||||
->appendChild(
|
->appendChild(
|
||||||
id(new AphrontFormTextControl())
|
id(new AphrontFormTextControl())
|
||||||
|
@ -584,7 +609,7 @@ class PhabricatorRepositoryEditController
|
||||||
$default_branch_name))
|
$default_branch_name))
|
||||||
->setError($e_branch)
|
->setError($e_branch)
|
||||||
->setCaption(
|
->setCaption(
|
||||||
'Default <strong>remote</strong> branch to show in Diffusion.'));
|
'Default branch to show in Diffusion.'));
|
||||||
}
|
}
|
||||||
|
|
||||||
$form
|
$form
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Copyright 2011 Facebook, Inc.
|
* Copyright 2012 Facebook, Inc.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -27,12 +27,14 @@ abstract class PhabricatorRepositoryCommitDiscoveryDaemon
|
||||||
}
|
}
|
||||||
|
|
||||||
final public function run() {
|
final public function run() {
|
||||||
$this->repository = $this->loadRepository();
|
|
||||||
|
|
||||||
$sleep = $this->repository->getDetail('pull-frequency');
|
|
||||||
while (true) {
|
while (true) {
|
||||||
|
// Reload the repository every time to pick up changes from the web
|
||||||
|
// console.
|
||||||
|
$this->repository = $this->loadRepository();
|
||||||
$this->discoverCommits();
|
$this->discoverCommits();
|
||||||
$this->sleep(max(2, $sleep));
|
|
||||||
|
$sleep = max(2, $this->getRepository()->getDetail('pull-frequency'));
|
||||||
|
$this->sleep($sleep);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Copyright 2011 Facebook, Inc.
|
* Copyright 2012 Facebook, Inc.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -52,7 +52,14 @@ class PhabricatorRepositoryGitCommitDiscoveryDaemon
|
||||||
$only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
|
$only_this_remote = DiffusionBranchInformation::DEFAULT_GIT_REMOTE);
|
||||||
|
|
||||||
$got_something = false;
|
$got_something = false;
|
||||||
|
$tracked_something = false;
|
||||||
foreach ($branches as $name => $commit) {
|
foreach ($branches as $name => $commit) {
|
||||||
|
if (!$repository->shouldTrackBranch($name)) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
$tracked_something = true;
|
||||||
|
|
||||||
if ($this->isKnownCommit($commit)) {
|
if ($this->isKnownCommit($commit)) {
|
||||||
continue;
|
continue;
|
||||||
} else {
|
} else {
|
||||||
|
@ -61,6 +68,14 @@ class PhabricatorRepositoryGitCommitDiscoveryDaemon
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!$tracked_something) {
|
||||||
|
$repo_name = $repository->getName();
|
||||||
|
$repo_callsign = $repository->getCallsign();
|
||||||
|
throw new Exception(
|
||||||
|
"Repository r{$repo_callsign} '{$repo_name}' has no tracked branches! ".
|
||||||
|
"Verify that your branch filtering settings are correct.");
|
||||||
|
}
|
||||||
|
|
||||||
return $got_something;
|
return $got_something;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Copyright 2011 Facebook, Inc.
|
* Copyright 2012 Facebook, Inc.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -308,4 +308,22 @@ class PhabricatorRepository extends PhabricatorRepositoryDAO {
|
||||||
return $this->getDetail('tracking-enabled', false);
|
return $this->getDetail('tracking-enabled', false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function shouldTrackBranch($branch) {
|
||||||
|
$vcs = $this->getVersionControlSystem();
|
||||||
|
|
||||||
|
$is_git = ($vcs == PhabricatorRepositoryType::REPOSITORY_TYPE_GIT);
|
||||||
|
|
||||||
|
$use_filter = ($is_git);
|
||||||
|
|
||||||
|
if ($use_filter) {
|
||||||
|
$filter = $this->getDetail('branch-filter', array());
|
||||||
|
if ($filter && !isset($filter[$branch])) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// By default, track all branches.
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,7 +1,7 @@
|
||||||
<?php
|
<?php
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Copyright 2011 Facebook, Inc.
|
* Copyright 2012 Facebook, Inc.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -34,4 +34,32 @@ final class PhabricatorRepositoryTestCase
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testBranchFilter() {
|
||||||
|
$git = PhabricatorRepositoryType::REPOSITORY_TYPE_GIT;
|
||||||
|
|
||||||
|
$repo = new PhabricatorRepository();
|
||||||
|
$repo->setVersionControlSystem($git);
|
||||||
|
|
||||||
|
$this->assertEqual(
|
||||||
|
true,
|
||||||
|
$repo->shouldTrackBranch('imaginary'),
|
||||||
|
'Track all branches by default.');
|
||||||
|
|
||||||
|
$repo->setDetail(
|
||||||
|
'branch-filter',
|
||||||
|
array(
|
||||||
|
'master' => true,
|
||||||
|
));
|
||||||
|
|
||||||
|
$this->assertEqual(
|
||||||
|
true,
|
||||||
|
$repo->shouldTrackBranch('master'),
|
||||||
|
'Track listed branches.');
|
||||||
|
|
||||||
|
$this->assertEqual(
|
||||||
|
false,
|
||||||
|
$repo->shouldTrackBranch('imaginary'),
|
||||||
|
'Do not track unlisted branches.');
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -6,6 +6,7 @@
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
phutil_require_module('phabricator', 'applications/repository/constants/repositorytype');
|
||||||
phutil_require_module('phabricator', 'applications/repository/storage/repository');
|
phutil_require_module('phabricator', 'applications/repository/storage/repository');
|
||||||
phutil_require_module('phabricator', 'infrastructure/testing/testcase');
|
phutil_require_module('phabricator', 'infrastructure/testing/testcase');
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue