Differential D7935 Diff 17980 src/applications/harbormaster/step/WaitForPreviousBuildStepImplementation.php

Changeset View

Standalone View

src/applications/harbormaster/step/WaitForPreviousBuildStepImplementation.php

Show All 24 Lines	public function execute(

// We can only wait when building against commits.		// We can only wait when building against commits.
$buildable = $build->getBuildable();		$buildable = $build->getBuildable();
$object = $buildable->getBuildableObject();		$object = $buildable->getBuildableObject();
if (!($object instanceof PhabricatorRepositoryCommit)) {		if (!($object instanceof PhabricatorRepositoryCommit)) {
return;		return;
}		}

// We are blocked until all previous builds finish.
$build->setBuildStatus(HarbormasterBuild::STATUS_WAITING);
$build->save();

hach-queUnsubmitted Not Done Inline Actions Why is this removed? This will mean the build will not display waiting information to the user. hach-que: Why is this removed? This will mean the build will not display waiting information to the user.
epriestleyAuthorUnsubmitted Not Done Inline Actions Two reasons: I've removed all updates of `$build` from the actual steps. This avoids a bunch of locking and race issues where multiple things are trying to write to `$build` at once: instead, the build update process does all build writes, and it uses locks to guarantee it is the only simultaneous writer. So these direct writes to `$build` violate that (and, e.g., could overwrite changes in other fields which had happened since `$build` loaded). Now that steps can run in parallel, a single step being in the "waiting" state does not mean that the whole build is waiting: other steps may be running. Instead, we should set the step as waiting and fail temporarily. Elsewhere, in the build update process, we should set the whole build as waiting only if all of the current steps are waiting. Since the only thing that not doing this costs us is that statuses say "running" instead of "waiting" and it's somewhat complex, I left it out of this diff (as the other changes represent meaningful unbreaks). I plan to restore it in the near future. epriestley: Two reasons: - I've removed all updates of `$build` from the actual steps. This avoids a…
// Block until all previous builds of the same build plan have		// Block until all previous builds of the same build plan have
// finished.		// finished.
$plan = $build->getBuildPlan();		$plan = $build->getBuildPlan();

$log = null;		$log = null;
$log_start = null;		$log_start = null;
$blockers = $this->getBlockers($object, $plan, $build);		$blockers = $this->getBlockers($object, $plan, $build);
while (count($blockers) > 0) {		while (count($blockers) > 0) {
if ($build->checkForCancellation()) {
if ($log !== null) {
$log->finalize($log_start);
}
return;
}

if ($log === null) {		if ($log === null) {
$log = $build->createLog($build_target, "waiting", "blockers");		$log = $build->createLog($build_target, "waiting", "blockers");
$log_start = $log->start();		$log_start = $log->start();
}		}

$log->append("Blocked by: ".implode(",", $blockers)."\n");		$log->append("Blocked by: ".implode(",", $blockers)."\n");

		// TODO: This should fail temporarily instead after setting the target to
		// waiting, and thereby push the build into a waiting status.
sleep(1);		sleep(1);
$blockers = $this->getBlockers($object, $plan, $build);		$blockers = $this->getBlockers($object, $plan, $build);
}		}
if ($log !== null) {		if ($log !== null) {
$log->finalize($log_start);		$log->finalize($log_start);
}		}

// Move back into building status.
$build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING);
$build->save();
}		}

private function getBlockers(		private function getBlockers(
PhabricatorRepositoryCommit $commit,		PhabricatorRepositoryCommit $commit,
HarbormasterBuildPlan $plan,		HarbormasterBuildPlan $plan,
HarbormasterBuild $source) {		HarbormasterBuild $source) {

$call = new ConduitCall(		$call = new ConduitCall(
▲ Show 20 Lines • Show All 47 Lines • Show Last 20 Lines