diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -773,6 +773,7 @@ 'DiffusionCommitTimelineEngine' => 'applications/diffusion/engine/DiffusionCommitTimelineEngine.php', 'DiffusionCommitTransactionType' => 'applications/diffusion/xaction/DiffusionCommitTransactionType.php', 'DiffusionCommitVerifyTransaction' => 'applications/diffusion/xaction/DiffusionCommitVerifyTransaction.php', + 'DiffusionCommitWrongBuildsHeraldField' => 'applications/diffusion/herald/DiffusionCommitWrongBuildsHeraldField.php', 'DiffusionCompareController' => 'applications/diffusion/controller/DiffusionCompareController.php', 'DiffusionConduitAPIMethod' => 'applications/diffusion/conduit/DiffusionConduitAPIMethod.php', 'DiffusionController' => 'applications/diffusion/controller/DiffusionController.php', @@ -904,6 +905,7 @@ 'DiffusionPreCommitContentRevisionHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionHeraldField.php', 'DiffusionPreCommitContentRevisionReviewersHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionReviewersHeraldField.php', 'DiffusionPreCommitContentRevisionSubscribersHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentRevisionSubscribersHeraldField.php', + 'DiffusionPreCommitContentWrongBuildsHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitContentWrongBuildsHeraldField.php', 'DiffusionPreCommitRefChangeHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefChangeHeraldField.php', 'DiffusionPreCommitRefHeraldField' => 'applications/diffusion/herald/DiffusionPreCommitRefHeraldField.php', 'DiffusionPreCommitRefHeraldFieldGroup' => 'applications/diffusion/herald/DiffusionPreCommitRefHeraldFieldGroup.php', @@ -6438,6 +6440,7 @@ 'DiffusionCommitTimelineEngine' => 'PhabricatorTimelineEngine', 'DiffusionCommitTransactionType' => 'PhabricatorModularTransactionType', 'DiffusionCommitVerifyTransaction' => 'DiffusionCommitAuditTransaction', + 'DiffusionCommitWrongBuildsHeraldField' => 'DiffusionCommitHeraldField', 'DiffusionCompareController' => 'DiffusionController', 'DiffusionConduitAPIMethod' => 'ConduitAPIMethod', 'DiffusionController' => 'PhabricatorController', @@ -6572,6 +6575,7 @@ 'DiffusionPreCommitContentRevisionHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPreCommitContentRevisionReviewersHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPreCommitContentRevisionSubscribersHeraldField' => 'DiffusionPreCommitContentHeraldField', + 'DiffusionPreCommitContentWrongBuildsHeraldField' => 'DiffusionPreCommitContentHeraldField', 'DiffusionPreCommitRefChangeHeraldField' => 'DiffusionPreCommitRefHeraldField', 'DiffusionPreCommitRefHeraldField' => 'HeraldField', 'DiffusionPreCommitRefHeraldFieldGroup' => 'HeraldFieldGroup', diff --git a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php --- a/src/applications/differential/engine/DifferentialDiffExtractionEngine.php +++ b/src/applications/differential/engine/DifferentialDiffExtractionEngine.php @@ -281,7 +281,11 @@ ->setNewValue($revision->getModernRevisionStatus()); } - $concerning_builds = $this->loadConcerningBuilds($revision); + $concerning_builds = self::loadConcerningBuilds( + $this->getViewer(), + $revision, + $strict = false); + if ($concerning_builds) { $build_list = array(); foreach ($concerning_builds as $build) { @@ -328,8 +332,11 @@ $editor->applyTransactions($revision, $xactions); } - private function loadConcerningBuilds(DifferentialRevision $revision) { - $viewer = $this->getViewer(); + public static function loadConcerningBuilds( + PhabricatorUser $viewer, + DifferentialRevision $revision, + $strict) { + $diff = $revision->getActiveDiff(); $buildables = id(new HarbormasterBuildableQuery()) @@ -342,7 +349,6 @@ return array(); } - $land_key = HarbormasterBuildPlanBehavior::BEHAVIOR_LANDWARNING; $behavior = HarbormasterBuildPlanBehavior::getBehavior($land_key); @@ -391,7 +397,13 @@ // cases where the repository is observed and the fetch pipeline // stalls for a while. - continue; + // If we're in strict mode (from a pre-commit content hook), we do + // not ignore these, since we're doing an instantaneous check against + // the current state. + + if (!$strict) { + continue; + } } if ($build->isPassed()) { diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -62,6 +62,7 @@ const PROPERTY_LINES_ADDED = 'lines.added'; const PROPERTY_LINES_REMOVED = 'lines.removed'; const PROPERTY_BUILDABLES = 'buildables'; + const PROPERTY_WRONG_BUILDS = 'wrong.builds'; public static function initializeNewRevision(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) diff --git a/src/applications/differential/xaction/DifferentialRevisionWrongBuildsTransaction.php b/src/applications/differential/xaction/DifferentialRevisionWrongBuildsTransaction.php --- a/src/applications/differential/xaction/DifferentialRevisionWrongBuildsTransaction.php +++ b/src/applications/differential/xaction/DifferentialRevisionWrongBuildsTransaction.php @@ -13,6 +13,10 @@ return $value; } + public function applyInternalEffects($object, $value) { + $object->setProperty(DifferentialRevision::PROPERTY_WRONG_BUILDS, true); + } + public function getIcon() { return 'fa-exclamation'; } diff --git a/src/applications/diffusion/herald/DiffusionCommitWrongBuildsHeraldField.php b/src/applications/diffusion/herald/DiffusionCommitWrongBuildsHeraldField.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/herald/DiffusionCommitWrongBuildsHeraldField.php @@ -0,0 +1,49 @@ +getAdapter(); + $viewer = $adapter->getViewer(); + + $revision = $adapter->loadDifferentialRevision(); + if (!$revision) { + return false; + } + + if ($revision->isPublished()) { + $wrong_builds = DifferentialRevision::PROPERTY_WRONG_BUILDS; + return !$revision->getProperty($wrong_builds, false); + } + + // Reload the revision to pick up active diffs. + $revision = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withPHIDs(array($revision->getPHID())) + ->needActiveDiffs(true) + ->executeOne(); + + $concerning = DifferentialDiffExtractionEngine::loadConcerningBuilds( + $viewer, + $revision, + $strict = false); + + return (bool)$concerning; + } + + protected function getHeraldFieldStandardType() { + return self::STANDARD_BOOL; + } + +} diff --git a/src/applications/diffusion/herald/DiffusionPreCommitContentWrongBuildsHeraldField.php b/src/applications/diffusion/herald/DiffusionPreCommitContentWrongBuildsHeraldField.php new file mode 100644 --- /dev/null +++ b/src/applications/diffusion/herald/DiffusionPreCommitContentWrongBuildsHeraldField.php @@ -0,0 +1,49 @@ +getAdapter(); + $viewer = $adapter->getViewer(); + + $revision = $adapter->getRevision(); + if (!$revision) { + return false; + } + + if ($revision->isPublished()) { + $wrong_builds = DifferentialRevision::PROPERTY_WRONG_BUILDS; + return !$revision->getProperty($wrong_builds, false); + } + + // Reload the revision to pick up active diffs. + $revision = id(new DifferentialRevisionQuery()) + ->setViewer($viewer) + ->withPHIDs(array($revision->getPHID())) + ->needActiveDiffs(true) + ->executeOne(); + + $concerning = DifferentialDiffExtractionEngine::loadConcerningBuilds( + $viewer, + $revision, + $strict = true); + + return (bool)$concerning; + } + + protected function getHeraldFieldStandardType() { + return self::STANDARD_BOOL; + } + +}