mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-27 01:02:42 +01:00
Apply "enormous changes" rules to pre-commit content rules too
Summary: Fixes T4276. This adds "Change is enormous" to pre-commit content rules so we can, e.g., just reject these and not worry about them elsewhere. Also, use the same numeric limits across the mechanisms so there's a consistent definition of an "enormous" changeset. Test Plan: - Set enormous limit to 15 bytes, pushed some changes, got blocked by a rule. - Set it back, pushed OK. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4276 Differential Revision: https://secure.phabricator.com/D7887
This commit is contained in:
parent
cba959635e
commit
3627e73e5e
4 changed files with 36 additions and 5 deletions
|
@ -1004,6 +1004,9 @@ final class DiffusionCommitHookEngine extends Phobject {
|
||||||
}
|
}
|
||||||
|
|
||||||
public function loadChangesetsForCommit($identifier) {
|
public function loadChangesetsForCommit($identifier) {
|
||||||
|
$byte_limit = HeraldCommitAdapter::getEnormousByteLimit();
|
||||||
|
$time_limit = HeraldCommitAdapter::getEnormousTimeLimit();
|
||||||
|
|
||||||
$vcs = $this->getRepository()->getVersionControlSystem();
|
$vcs = $this->getRepository()->getVersionControlSystem();
|
||||||
switch ($vcs) {
|
switch ($vcs) {
|
||||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
|
case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT:
|
||||||
|
@ -1015,8 +1018,10 @@ final class DiffusionCommitHookEngine extends Phobject {
|
||||||
'user' => $this->getViewer(),
|
'user' => $this->getViewer(),
|
||||||
'commit' => $identifier,
|
'commit' => $identifier,
|
||||||
));
|
));
|
||||||
|
|
||||||
$raw_diff = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest)
|
$raw_diff = DiffusionRawDiffQuery::newFromDiffusionRequest($drequest)
|
||||||
->setTimeout(5 * 60)
|
->setTimeout($time_limit)
|
||||||
|
->setByteLimit($byte_limit)
|
||||||
->setLinesOfContext(0)
|
->setLinesOfContext(0)
|
||||||
->loadRawDiff();
|
->loadRawDiff();
|
||||||
break;
|
break;
|
||||||
|
@ -1027,15 +1032,29 @@ final class DiffusionCommitHookEngine extends Phobject {
|
||||||
// the "--diff-cmd" flag.
|
// the "--diff-cmd" flag.
|
||||||
|
|
||||||
// For subversion, we need to use `svnlook`.
|
// For subversion, we need to use `svnlook`.
|
||||||
list($raw_diff) = execx(
|
$future = new ExecFuture(
|
||||||
'svnlook diff -t %s %s',
|
'svnlook diff -t %s %s',
|
||||||
$this->subversionTransaction,
|
$this->subversionTransaction,
|
||||||
$this->subversionRepository);
|
$this->subversionRepository);
|
||||||
|
|
||||||
|
$future->setTimeout($time_limit);
|
||||||
|
$future->setStdoutSizeLimit($byte_limit);
|
||||||
|
$future->setStderrSizeLimit($byte_limit);
|
||||||
|
|
||||||
|
list($raw_diff) = $future->resolvex();
|
||||||
break;
|
break;
|
||||||
default:
|
default:
|
||||||
throw new Exception(pht("Unknown VCS '%s!'", $vcs));
|
throw new Exception(pht("Unknown VCS '%s!'", $vcs));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (strlen($raw_diff) >= $byte_limit) {
|
||||||
|
throw new Exception(
|
||||||
|
pht(
|
||||||
|
'The raw text of this change is enormous (larger than %d '.
|
||||||
|
'bytes). Herald can not process it.',
|
||||||
|
$byte_limit));
|
||||||
|
}
|
||||||
|
|
||||||
$parser = new ArcanistDiffParser();
|
$parser = new ArcanistDiffParser();
|
||||||
$changes = $parser->parseDiff($raw_diff);
|
$changes = $parser->parseDiff($raw_diff);
|
||||||
$diff = DifferentialDiff::newFromRawChanges($changes);
|
$diff = DifferentialDiff::newFromRawChanges($changes);
|
||||||
|
|
|
@ -34,6 +34,7 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter {
|
||||||
self::FIELD_DIFF_CONTENT,
|
self::FIELD_DIFF_CONTENT,
|
||||||
self::FIELD_DIFF_ADDED_CONTENT,
|
self::FIELD_DIFF_ADDED_CONTENT,
|
||||||
self::FIELD_DIFF_REMOVED_CONTENT,
|
self::FIELD_DIFF_REMOVED_CONTENT,
|
||||||
|
self::FIELD_DIFF_ENORMOUS,
|
||||||
self::FIELD_REPOSITORY,
|
self::FIELD_REPOSITORY,
|
||||||
self::FIELD_REPOSITORY_PROJECTS,
|
self::FIELD_REPOSITORY_PROJECTS,
|
||||||
self::FIELD_PUSHER,
|
self::FIELD_PUSHER,
|
||||||
|
@ -75,6 +76,9 @@ final class HeraldPreCommitContentAdapter extends HeraldPreCommitAdapter {
|
||||||
return $this->getDiffContent('+');
|
return $this->getDiffContent('+');
|
||||||
case self::FIELD_DIFF_REMOVED_CONTENT:
|
case self::FIELD_DIFF_REMOVED_CONTENT:
|
||||||
return $this->getDiffContent('-');
|
return $this->getDiffContent('-');
|
||||||
|
case self::FIELD_DIFF_ENORMOUS:
|
||||||
|
$this->getDiffContent('*');
|
||||||
|
return ($this->changesets instanceof Exception);
|
||||||
case self::FIELD_REPOSITORY:
|
case self::FIELD_REPOSITORY:
|
||||||
return $this->getHookEngine()->getRepository()->getPHID();
|
return $this->getHookEngine()->getRepository()->getPHID();
|
||||||
case self::FIELD_REPOSITORY_PROJECTS:
|
case self::FIELD_REPOSITORY_PROJECTS:
|
||||||
|
|
|
@ -270,6 +270,14 @@ final class HeraldCommitAdapter extends HeraldAdapter {
|
||||||
return $this->affectedRevision;
|
return $this->affectedRevision;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static function getEnormousByteLimit() {
|
||||||
|
return 1024 * 1024 * 1024; // 1GB
|
||||||
|
}
|
||||||
|
|
||||||
|
public static function getEnormousTimeLimit() {
|
||||||
|
return 60 * 15; // 15 Minutes
|
||||||
|
}
|
||||||
|
|
||||||
private function loadCommitDiff() {
|
private function loadCommitDiff() {
|
||||||
$drequest = DiffusionRequest::newFromDictionary(
|
$drequest = DiffusionRequest::newFromDictionary(
|
||||||
array(
|
array(
|
||||||
|
@ -278,7 +286,7 @@ final class HeraldCommitAdapter extends HeraldAdapter {
|
||||||
'commit' => $this->commit->getCommitIdentifier(),
|
'commit' => $this->commit->getCommitIdentifier(),
|
||||||
));
|
));
|
||||||
|
|
||||||
$byte_limit = (1024 * 1024 * 1024); // 1GB
|
$byte_limit = self::getEnormousByteLimit();
|
||||||
|
|
||||||
$raw = DiffusionQuery::callConduitWithDiffusionRequest(
|
$raw = DiffusionQuery::callConduitWithDiffusionRequest(
|
||||||
PhabricatorUser::getOmnipotentUser(),
|
PhabricatorUser::getOmnipotentUser(),
|
||||||
|
@ -286,7 +294,7 @@ final class HeraldCommitAdapter extends HeraldAdapter {
|
||||||
'diffusion.rawdiffquery',
|
'diffusion.rawdiffquery',
|
||||||
array(
|
array(
|
||||||
'commit' => $this->commit->getCommitIdentifier(),
|
'commit' => $this->commit->getCommitIdentifier(),
|
||||||
'timeout' => (60 * 15), // 15 minutes
|
'timeout' => self::getEnormousTimeLimit(),
|
||||||
'byteLimit' => $byte_limit,
|
'byteLimit' => $byte_limit,
|
||||||
'linesOfContext' => 0,
|
'linesOfContext' => 0,
|
||||||
));
|
));
|
||||||
|
|
|
@ -17,7 +17,7 @@ final class HeraldRule extends HeraldDAO
|
||||||
protected $isDisabled = 0;
|
protected $isDisabled = 0;
|
||||||
protected $triggerObjectPHID;
|
protected $triggerObjectPHID;
|
||||||
|
|
||||||
protected $configVersion = 25;
|
protected $configVersion = 26;
|
||||||
|
|
||||||
// phids for which this rule has been applied
|
// phids for which this rule has been applied
|
||||||
private $ruleApplied = self::ATTACHABLE;
|
private $ruleApplied = self::ATTACHABLE;
|
||||||
|
|
Loading…
Reference in a new issue