Page MenuHomePhabricator
Files F14010331

D13441.id32533.diff
No OneTemporary
Actions

D13441.id32533.diff
View Options

diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -585,6 +585,8 @@
'a race?'));
}
+ // TODO: This can race with diff updates, particularly those from
+ // Harbormaster. See discussion in T8650.
$diff->setRevisionID($object->getID());
$diff->save();
@@ -593,6 +595,8 @@
// the old (`null`) container.
// TODO: This is a bit iffy, maybe we can find a cleaner approach?
+ // In particular, this could (rarely) be overwritten by Harbormaster
+ // workers.
$table = new HarbormasterBuildable();
$conn_w = $table->establishConnection('w');
queryfx(
diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
--- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
+++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
@@ -406,43 +406,59 @@
$should_publish = $did_update &&
$new_status != HarbormasterBuildable::STATUS_BUILDING &&
!$buildable->getIsManualBuildable();
- if ($should_publish) {
- $object = id(new PhabricatorObjectQuery())
- ->setViewer($viewer)
- ->withPHIDs(array($buildable->getBuildablePHID()))
- ->executeOne();
-
- if ($object instanceof PhabricatorApplicationTransactionInterface) {
- $template = $object->getApplicationTransactionTemplate();
- if ($template) {
- $template
- ->setTransactionType(PhabricatorTransactions::TYPE_BUILDABLE)
- ->setMetadataValue(
- 'harbormaster:buildablePHID',
- $buildable->getPHID())
- ->setOldValue($old_status)
- ->setNewValue($new_status);
-
- $harbormaster_phid = id(new PhabricatorHarbormasterApplication())
- ->getPHID();
-
- $daemon_source = PhabricatorContentSource::newForSource(
- PhabricatorContentSource::SOURCE_DAEMON,
- array());
-
- $editor = $object->getApplicationTransactionEditor()
- ->setActor($viewer)
- ->setActingAsPHID($harbormaster_phid)
- ->setContentSource($daemon_source)
- ->setContinueOnNoEffect(true)
- ->setContinueOnMissingFields(true);
-
- $editor->applyTransactions(
- $object->getApplicationTransactionObject(),
- array($template));
- }
- }
+
+ if (!$should_publish) {
+ return;
+ }
+
+ $object = id(new PhabricatorObjectQuery())
+ ->setViewer($viewer)
+ ->withPHIDs(array($buildable->getBuildablePHID()))
+ ->executeOne();
+ if (!$object) {
+ return;
}
+
+ if (!($object instanceof PhabricatorApplicationTransactionInterface)) {
+ return;
+ }
+
+ // TODO: Publishing these transactions is causing a race. See T8650.
+ // We shouldn't be publishing to diffs anyway.
+ if ($object instanceof DifferentialDiff) {
+ return;
+ }
+
+ $template = $object->getApplicationTransactionTemplate();
+ if (!$template) {
+ return;
+ }
+
+ $template
+ ->setTransactionType(PhabricatorTransactions::TYPE_BUILDABLE)
+ ->setMetadataValue(
+ 'harbormaster:buildablePHID',
+ $buildable->getPHID())
+ ->setOldValue($old_status)
+ ->setNewValue($new_status);
+
+ $harbormaster_phid = id(new PhabricatorHarbormasterApplication())
+ ->getPHID();
+
+ $daemon_source = PhabricatorContentSource::newForSource(
+ PhabricatorContentSource::SOURCE_DAEMON,
+ array());
+
+ $editor = $object->getApplicationTransactionEditor()
+ ->setActor($viewer)
+ ->setActingAsPHID($harbormaster_phid)
+ ->setContentSource($daemon_source)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true);
+
+ $editor->applyTransactions(
+ $object->getApplicationTransactionObject(),
+ array($template));
}
private function releaseAllArtifacts(HarbormasterBuild $build) {

File Metadata

Mime Type
text/plain
Expires
Fri, Nov 1, 7:08 AM (2 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6715461
Default Alt Text
D13441.id32533.diff (4 KB)

Event Timeline

Phabricator · Built by Phacility