From adcc4ee1db1df4951cf62f311c42bbb87a454af4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 26 Dec 2013 10:40:16 -0800 Subject: [PATCH] Add a "branches" rule for Herald commit rules Summary: Fixes T4195. Allows you to write a rule against a commit's branches. This completes outstanding work on T4195. Test Plan: Pushed to Git and Mercurial repositories and verified branches were selected correctly by examining transcripts. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4195 Differential Revision: https://secure.phabricator.com/D7820 --- .../engine/DiffusionCommitHookEngine.php | 29 +++++++++++++++++++ .../herald/HeraldPreCommitContentAdapter.php | 7 +++++ .../herald/adapter/HeraldAdapter.php | 3 ++ 3 files changed, 39 insertions(+) diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index 667e2013b2..2b4a9ac33a 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -26,6 +26,7 @@ final class DiffusionCommitHookEngine extends Phobject { private $transactionKey; private $mercurialHook; private $mercurialCommits = array(); + private $gitCommits = array(); private $heraldViewerProjects; @@ -427,6 +428,7 @@ final class DiffusionCommitHookEngine extends Phobject { $merge_base = rtrim($stdout, "\n"); } + $ref_update = $ref_updates[$key]; $ref_update->setMergeBase($merge_base); } @@ -525,7 +527,20 @@ final class DiffusionCommitHookEngine extends Phobject { $commits = phutil_split_lines($stdout, $retain_newlines = false); + // If we're looking at a branch, mark all of the new commits as on that + // branch. It's only possible for these commits to be on updated branches, + // since any other branch heads are necessarily behind them. + $branch_name = null; + $ref_update = $ref_updates[$key]; + $type_branch = PhabricatorRepositoryPushLog::REFTYPE_BRANCH; + if ($ref_update->getRefType() == $type_branch) { + $branch_name = $ref_update->getRefName(); + } + foreach ($commits as $commit) { + if ($branch_name) { + $this->gitCommits[$commit][] = $branch_name; + } $content_updates[$commit] = $this->newPushLog() ->setRefType(PhabricatorRepositoryPushLog::REFTYPE_COMMIT) ->setRefNew($commit) @@ -973,5 +988,19 @@ final class DiffusionCommitHookEngine extends Phobject { } } + public function loadBranches($identifier) { + $repository = $this->getRepository(); + $vcs = $repository->getVersionControlSystem(); + switch ($vcs) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + return idx($this->gitCommits, $identifier, array()); + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + return idx($this->mercurialCommits, $identifier, array()); + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + // Subversion doesn't have branches. + return array(); + } + } + } diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php index aee68f126d..95cd112fad 100644 --- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php +++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php @@ -42,6 +42,7 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter { self::FIELD_BODY, self::FIELD_AUTHOR, self::FIELD_COMMITTER, + self::FIELD_BRANCHES, self::FIELD_DIFF_FILE, self::FIELD_DIFF_CONTENT, self::FIELD_DIFF_ADDED_CONTENT, @@ -100,6 +101,8 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter { return $this->getAuthorPHID(); case self::FIELD_COMMITTER: return $this->getCommitterPHID(); + case self::FIELD_BRANCHES: + return $this->getBranches(); case self::FIELD_DIFF_FILE: return $this->getDiffContent('name'); case self::FIELD_DIFF_CONTENT: @@ -329,4 +332,8 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter { } } + private function getBranches() { + return $this->hookEngine->loadBranches($this->log->getRefNew()); + } + } diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index e05b57f562..e71f60d96c 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -33,6 +33,7 @@ abstract class HeraldAdapter { const FIELD_DIFFERENTIAL_CCS = 'differential-ccs'; const FIELD_DIFFERENTIAL_ACCEPTED = 'differential-accepted'; const FIELD_IS_MERGE_COMMIT = 'is-merge-commit'; + const FIELD_BRANCHES = 'branches'; const CONDITION_CONTAINS = 'contains'; const CONDITION_NOT_CONTAINS = '!contains'; @@ -178,6 +179,7 @@ abstract class HeraldAdapter { self::FIELD_DIFFERENTIAL_ACCEPTED => pht('Accepted Differential revision'), self::FIELD_IS_MERGE_COMMIT => pht('Commit is a merge'), + self::FIELD_BRANCHES => pht('Commit\'s branches'), ); } @@ -255,6 +257,7 @@ abstract class HeraldAdapter { self::CONDITION_NOT_EXISTS, ); case self::FIELD_DIFF_FILE: + case self::FIELD_BRANCHES: return array( self::CONDITION_CONTAINS, self::CONDITION_REGEXP,