Page MenuHomePhabricator
Files F91465

D7809.diff
All Users
Actions

D7809.diff
View Options

diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -259,6 +259,7 @@
$engine = new HeraldEngine();
$rules = null;
$blocking_effect = null;
+ $blocked_update = null;
foreach ($updates as $update) {
$adapter = id(clone $adapter_template)
->setPushLog($update);
@@ -275,6 +276,7 @@
foreach ($effects as $effect) {
if ($effect->getAction() == HeraldAdapter::ACTION_BLOCK) {
$blocking_effect = $effect;
+ $blocked_update = $update;
break;
}
}
@@ -300,12 +302,19 @@
$rule_name = pht('Unnamed Herald Rule');
}
+ $blocked_ref_name = coalesce(
+ $blocked_update->getRefName(),
+ $blocked_update->getRefNewShort());
+ $blocked_name = $blocked_update->getRefType().'/'.$blocked_ref_name;
+
throw new DiffusionCommitHookRejectException(
pht(
- "This commit was rejected by Herald pre-commit rule %s.\n".
- "Rule: %s\n".
+ "This push was rejected by Herald push rule %s.\n".
+ "Change: %s\n".
+ " Rule: %s\n".
"Reason: %s",
'H'.$blocking_effect->getRuleID(),
+ $blocked_name,
$rule_name,
$message));
}
diff --git a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
--- a/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
+++ b/src/applications/diffusion/herald/HeraldPreCommitContentAdapter.php
@@ -53,6 +53,7 @@
self::FIELD_DIFFERENTIAL_ACCEPTED,
self::FIELD_DIFFERENTIAL_REVIEWERS,
self::FIELD_DIFFERENTIAL_CCS,
+ self::FIELD_IS_MERGE_COMMIT,
self::FIELD_RULE,
),
parent::getFields());
@@ -141,6 +142,8 @@
return array();
}
return $revision->getCCPHIDs();
+ case self::FIELD_IS_MERGE_COMMIT:
+ return $this->getIsMergeCommit();
}
return parent::getHeraldField($field);
@@ -306,4 +309,24 @@
return $this->revision;
}
+ private function getIsMergeCommit() {
+ $repository = $this->hookEngine->getRepository();
+ $vcs = $repository->getVersionControlSystem();
+ switch ($vcs) {
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
+ $parents = id(new DiffusionLowLevelParentsQuery())
+ ->setRepository($repository)
+ ->withIdentifier($this->log->getRefNew())
+ ->execute();
+
+ return (count($parents) > 1);
+ case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
+ // NOTE: For now, we ignore "svn:mergeinfo" at all levels. We might
+ // change this some day, but it's not nearly as clear a signal as
+ // ancestry is in Git/Mercurial.
+ return false;
+ }
+ }
+
}
diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php
--- a/src/applications/herald/adapter/HeraldAdapter.php
+++ b/src/applications/herald/adapter/HeraldAdapter.php
@@ -31,6 +31,7 @@
const FIELD_DIFFERENTIAL_REVIEWERS = 'differential-reviewers';
const FIELD_DIFFERENTIAL_CCS = 'differential-ccs';
const FIELD_DIFFERENTIAL_ACCEPTED = 'differential-accepted';
+ const FIELD_IS_MERGE_COMMIT = 'is-merge-commit';
const CONDITION_CONTAINS = 'contains';
const CONDITION_NOT_CONTAINS = '!contains';
@@ -52,6 +53,8 @@
const CONDITION_REGEXP_PAIR = 'regexp-pair';
const CONDITION_HAS_BIT = 'bit';
const CONDITION_NOT_BIT = '!bit';
+ const CONDITION_IS_TRUE = 'true';
+ const CONDITION_IS_FALSE = 'false';
const ACTION_ADD_CC = 'addcc';
const ACTION_REMOVE_CC = 'remcc';
@@ -172,6 +175,7 @@
self::FIELD_DIFFERENTIAL_CCS => pht('Differential CCs'),
self::FIELD_DIFFERENTIAL_ACCEPTED
=> pht('Accepted Differential revision'),
+ self::FIELD_IS_MERGE_COMMIT => pht('Commit is a merge'),
);
}
@@ -186,6 +190,8 @@
self::CONDITION_IS => pht('is'),
self::CONDITION_IS_NOT => pht('is not'),
self::CONDITION_IS_ANY => pht('is any of'),
+ self::CONDITION_IS_TRUE => pht('is true'),
+ self::CONDITION_IS_FALSE => pht('is false'),
self::CONDITION_IS_NOT_ANY => pht('is not any of'),
self::CONDITION_INCLUDE_ALL => pht('include all of'),
self::CONDITION_INCLUDE_ANY => pht('include any of'),
@@ -292,6 +298,11 @@
self::CONDITION_EXISTS,
self::CONDITION_NOT_EXISTS,
);
+ case self::FIELD_IS_MERGE_COMMIT:
+ return array(
+ self::CONDITION_IS_TRUE,
+ self::CONDITION_IS_FALSE,
+ );
default:
throw new Exception(
"This adapter does not define conditions for field '{$field}'!");
@@ -362,8 +373,10 @@
array_fuse($field_value),
$condition_value);
case self::CONDITION_EXISTS:
+ case self::CONDITION_IS_TRUE:
return (bool)$field_value;
case self::CONDITION_NOT_EXISTS:
+ case self::CONDITION_IS_FALSE:
return !$field_value;
case self::CONDITION_UNCONDITIONALLY:
return (bool)$field_value;
@@ -515,6 +528,8 @@
case self::CONDITION_UNCONDITIONALLY:
case self::CONDITION_HAS_BIT:
case self::CONDITION_NOT_BIT:
+ case self::CONDITION_IS_TRUE:
+ case self::CONDITION_IS_FALSE:
// No explicit validation for these types, although there probably
// should be in some cases.
break;
@@ -669,6 +684,8 @@
case self::CONDITION_EXISTS:
case self::CONDITION_NOT_EXISTS:
case self::CONDITION_UNCONDITIONALLY:
+ case self::CONDITION_IS_TRUE:
+ case self::CONDITION_IS_FALSE:
return self::VALUE_NONE;
case self::CONDITION_RULE:
case self::CONDITION_NOT_RULE:

File Metadata

Mime Type
text/x-diff
Storage Engine
amazon-s3
Storage Format
Raw Data
Storage Handle
phabricator/ir/rb/nz67itjpcfrhxyun
Default Alt Text
D7809.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility