mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-30 09:20:58 +01:00
Add "raw author name" and "raw committer name" as Herald fields for commit content hooks
Summary: Ref T4195. A legitimate rule which needs this field is "do not allow commits as root". Interestingly, we have exactly one commit as root in each Phabricator, Arcanist and libphutil. Since the committer and author don't need to be Phabricator accounts (just the Pusher), the existing "Committer" and "Author" fields can't express this rule (they'll be empty). Test Plan: {F93406} Reviewers: btrahan Reviewed By: btrahan CC: SEJeff, aran Maniphest Tasks: T4195 Differential Revision: https://secure.phabricator.com/D7841
This commit is contained in:
parent
ce78bf1de4
commit
9f38aaa5de
3 changed files with 47 additions and 1 deletions
|
@ -41,7 +41,9 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter {
|
|||
array(
|
||||
self::FIELD_BODY,
|
||||
self::FIELD_AUTHOR,
|
||||
self::FIELD_AUTHOR_RAW,
|
||||
self::FIELD_COMMITTER,
|
||||
self::FIELD_COMMITTER_RAW,
|
||||
self::FIELD_BRANCHES,
|
||||
self::FIELD_DIFF_FILE,
|
||||
self::FIELD_DIFF_CONTENT,
|
||||
|
@ -99,8 +101,12 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter {
|
|||
return $this->getCommitRef()->getMessage();
|
||||
case self::FIELD_AUTHOR:
|
||||
return $this->getAuthorPHID();
|
||||
case self::FIELD_AUTHOR_RAW:
|
||||
return $this->getAuthorRaw();
|
||||
case self::FIELD_COMMITTER:
|
||||
return $this->getCommitterPHID();
|
||||
case self::FIELD_COMMITTER_RAW:
|
||||
return $this->getCommitterRaw();
|
||||
case self::FIELD_BRANCHES:
|
||||
return $this->getBranches();
|
||||
case self::FIELD_DIFF_FILE:
|
||||
|
@ -278,6 +284,40 @@ final class HeraldPreCommitContentAdapter extends HeraldAdapter {
|
|||
}
|
||||
}
|
||||
|
||||
private function getAuthorRaw() {
|
||||
$repository = $this->hookEngine->getRepository();
|
||||
$vcs = $repository->getVersionControlSystem();
|
||||
switch ($vcs) {
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
|
||||
$ref = $this->getCommitRef();
|
||||
return $ref->getAuthor();
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
|
||||
// In Subversion, the pusher is always the author.
|
||||
return $this->hookEngine->getViewer()->getUsername();
|
||||
}
|
||||
}
|
||||
|
||||
private function getCommitterRaw() {
|
||||
$repository = $this->hookEngine->getRepository();
|
||||
$vcs = $repository->getVersionControlSystem();
|
||||
switch ($vcs) {
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
|
||||
// Here, if there's no committer, we're going to return the author
|
||||
// instead.
|
||||
$ref = $this->getCommitRef();
|
||||
$committer = $ref->getCommitter();
|
||||
if (strlen($committer)) {
|
||||
return $committer;
|
||||
}
|
||||
return $ref->getAuthor();
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
|
||||
// In Subversion, the pusher is always the committer.
|
||||
return $this->hookEngine->getViewer()->getUsername();
|
||||
}
|
||||
}
|
||||
|
||||
private function lookupUser($author) {
|
||||
return id(new DiffusionResolveUserQuery())
|
||||
->withName($author)
|
||||
|
|
|
@ -34,6 +34,8 @@ abstract class HeraldAdapter {
|
|||
const FIELD_DIFFERENTIAL_ACCEPTED = 'differential-accepted';
|
||||
const FIELD_IS_MERGE_COMMIT = 'is-merge-commit';
|
||||
const FIELD_BRANCHES = 'branches';
|
||||
const FIELD_AUTHOR_RAW = 'author-raw';
|
||||
const FIELD_COMMITTER_RAW = 'committer-raw';
|
||||
|
||||
const CONDITION_CONTAINS = 'contains';
|
||||
const CONDITION_NOT_CONTAINS = '!contains';
|
||||
|
@ -180,6 +182,8 @@ abstract class HeraldAdapter {
|
|||
=> pht('Accepted Differential revision'),
|
||||
self::FIELD_IS_MERGE_COMMIT => pht('Commit is a merge'),
|
||||
self::FIELD_BRANCHES => pht('Commit\'s branches'),
|
||||
self::FIELD_AUTHOR_RAW => pht('Raw author name'),
|
||||
self::FIELD_COMMITTER_RAW => pht('Raw committer name'),
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -218,6 +222,8 @@ abstract class HeraldAdapter {
|
|||
switch ($field) {
|
||||
case self::FIELD_TITLE:
|
||||
case self::FIELD_BODY:
|
||||
case self::FIELD_COMMITTER_RAW:
|
||||
case self::FIELD_AUTHOR_RAW:
|
||||
return array(
|
||||
self::CONDITION_CONTAINS,
|
||||
self::CONDITION_NOT_CONTAINS,
|
||||
|
|
|
@ -16,7 +16,7 @@ final class HeraldRule extends HeraldDAO
|
|||
protected $ruleType;
|
||||
protected $isDisabled = 0;
|
||||
|
||||
protected $configVersion = 20;
|
||||
protected $configVersion = 21;
|
||||
|
||||
// phids for which this rule has been applied
|
||||
private $ruleApplied = self::ATTACHABLE;
|
||||
|
|
Loading…
Reference in a new issue