Commit 4d22e0f8 authored by epriestley's avatar epriestley

(stable) Promote 2019 Week 10

parents b4a30268 9830c931
......@@ -42,6 +42,7 @@ phutil_register_library_map(array(
'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBraceFormattingXHPASTLinterRuleTestCase.php',
'ArcanistBranchWorkflow' => 'workflow/ArcanistBranchWorkflow.php',
'ArcanistBrowseWorkflow' => 'workflow/ArcanistBrowseWorkflow.php',
'ArcanistBuildPlanRef' => 'ref/ArcanistBuildPlanRef.php',
'ArcanistBuildRef' => 'ref/ArcanistBuildRef.php',
'ArcanistBundle' => 'parser/ArcanistBundle.php',
'ArcanistBundleTestCase' => 'parser/__tests__/ArcanistBundleTestCase.php',
......@@ -464,6 +465,7 @@ phutil_register_library_map(array(
'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase',
'ArcanistBranchWorkflow' => 'ArcanistFeatureWorkflow',
'ArcanistBrowseWorkflow' => 'ArcanistWorkflow',
'ArcanistBuildPlanRef' => 'Phobject',
'ArcanistBuildRef' => 'Phobject',
'ArcanistBundle' => 'Phobject',
'ArcanistBundleTestCase' => 'PhutilTestCase',
......
<?php
final class ArcanistBuildPlanRef
extends Phobject {
private $parameters;
public static function newFromConduit(array $data) {
$ref = new self();
$ref->parameters = $data;
return $ref;
}
public function getPHID() {
return $this->parameters['phid'];
}
public function getBehavior($behavior_key, $default = null) {
return idxv(
$this->parameters,
array('fields', 'behaviors', $behavior_key, 'value'),
$default);
}
}
......@@ -69,6 +69,10 @@ final class ArcanistBuildRef
return $this->parameters['id'];
}
public function getPHID() {
return $this->parameters['phid'];
}
public function getName() {
if (isset($this->parameters['fields']['name'])) {
return $this->parameters['fields']['name'];
......@@ -96,11 +100,32 @@ final class ArcanistBuildRef
return pht('Build %d', $this->getID());
}
public function getBuildPlanPHID() {
return idxv($this->parameters, array('fields', 'buildPlanPHID'));
}
public function isComplete() {
switch ($this->getStatus()) {
case 'passed':
case 'failed':
case 'aborted':
case 'error':
case 'deadlocked':
return true;
default:
return false;
}
}
public function isPassed() {
return ($this->getStatus() === 'passed');
}
public function getStatusSortVector() {
$status = $this->getStatus();
// For now, just sort passed builds first.
if ($this->getStatus() == 'passed') {
if ($this->isPassed()) {
$status_class = 1;
} else {
$status_class = 2;
......
......@@ -2164,8 +2164,8 @@ EOTEXT
// we always get this 100% right, we're just trying to do something
// reasonable.
$local = $this->loadActiveLocalCommitInfo();
$hashes = ipull($local, null, 'commit');
$hashes = $this->loadActiveDiffLocalCommitHashes();
$hashes = array_fuse($hashes);
$usable = array();
foreach ($commit_messages as $message) {
......@@ -2212,8 +2212,8 @@ EOTEXT
return null;
}
$local = $this->loadActiveLocalCommitInfo();
$hashes = ipull($local, null, 'commit');
$hashes = $this->loadActiveDiffLocalCommitHashes();
$hashes = array_fuse($hashes);
$usable = array();
foreach ($messages as $rev => $message) {
......@@ -2261,7 +2261,63 @@ EOTEXT
return implode('', $default);
}
private function loadActiveLocalCommitInfo() {
private function loadActiveDiffLocalCommitHashes() {
// The older "differential.querydiffs" method includes the full diff text,
// which can be very slow for large diffs. If we can, try to use
// "differential.diff.search" instead.
// We expect this to fail if the Phabricator version on the server is
// older than April 2018 (D19386), which introduced the "commits"
// attachment for "differential.revision.search".
// TODO: This can be optimized if we're able to learn the "revisionPHID"
// before we get here. See PHI1104.
try {
$revisions_raw = $this->getConduit()->callMethodSynchronous(
'differential.revision.search',
array(
'constraints' => array(
'ids' => array(
$this->revisionID,
),
),
));
$revisions = $revisions_raw['data'];
$revision = head($revisions);
if ($revision) {
$revision_phid = $revision['phid'];
$diffs_raw = $this->getConduit()->callMethodSynchronous(
'differential.diff.search',
array(
'constraints' => array(
'revisionPHIDs' => array(
$revision_phid,
),
),
'attachments' => array(
'commits' => true,
),
'limit' => 1,
));
$diffs = $diffs_raw['data'];
$diff = head($diffs);
if ($diff) {
$commits = idxv($diff, array('attachments', 'commits', 'commits'));
if ($commits !== null) {
$hashes = ipull($commits, 'identifier');
return array_values($hashes);
}
}
}
} catch (Exception $ex) {
// If any of this fails, fall back to the older method below.
}
$current_diff = $this->getConduit()->callMethodSynchronous(
'differential.querydiffs',
array(
......@@ -2270,7 +2326,10 @@ EOTEXT
$current_diff = head($current_diff);
$properties = idx($current_diff, 'properties', array());
return idx($properties, 'local:commits', array());
$local = idx($properties, 'local:commits', array());
$hashes = ipull($local, 'commit');
return array_values($hashes);
}
......
......@@ -1369,6 +1369,19 @@ EOTEXT
* before landing if it does.
*/
private function checkForBuildables($diff_phid) {
// Try to use the more modern check which respects the "Warn on Land"
// behavioral flag on build plans if we can. This newer check won't work
// unless the server is running code from March 2019 or newer since the
// API methods we need won't exist yet. We'll fall back to the older check
// if this one doesn't work out.
try {
$this->checkForBuildablesWithPlanBehaviors($diff_phid);
} catch (ArcanistUserAbortException $abort_ex) {
throw $abort_ex;
} catch (Exception $ex) {
// Continue with the older approach, below.
}
// NOTE: Since Harbormaster is still beta and this stuff all got added
// recently, just bail if we can't find a buildable. This is just an
// advisory check intended to prevent human error.
......@@ -1449,6 +1462,166 @@ EOTEXT
}
}
private function checkForBuildablesWithPlanBehaviors($diff_phid) {
// TODO: These queries should page through all results instead of fetching
// only the first page, but we don't have good primitives to support that
// in "master" yet.
$this->writeInfo(
pht('BUILDS'),
pht('Checking build status...'));
$raw_buildables = $this->getConduit()->callMethodSynchronous(
'harbormaster.buildable.search',
array(
'constraints' => array(
'objectPHIDs' => array(
$diff_phid,
),
'manual' => false,
),
));
if (!$raw_buildables['data']) {
return;
}
$buildables = $raw_buildables['data'];
$buildable_phids = ipull($buildables, 'phid');
$raw_builds = $this->getConduit()->callMethodSynchronous(
'harbormaster.build.search',
array(
'constraints' => array(
'buildables' => $buildable_phids,
),
));
if (!$raw_builds['data']) {
return;
}
$builds = array();
foreach ($raw_builds['data'] as $raw_build) {
$build_ref = ArcanistBuildRef::newFromConduit($raw_build);
$build_phid = $build_ref->getPHID();
$builds[$build_phid] = $build_ref;
}
$plan_phids = mpull($builds, 'getBuildPlanPHID');
$plan_phids = array_values($plan_phids);
$raw_plans = $this->getConduit()->callMethodSynchronous(
'harbormaster.buildplan.search',
array(
'constraints' => array(
'phids' => $plan_phids,
),
));
$plans = array();
foreach ($raw_plans['data'] as $raw_plan) {
$plan_ref = ArcanistBuildPlanRef::newFromConduit($raw_plan);
$plan_phid = $plan_ref->getPHID();
$plans[$plan_phid] = $plan_ref;
}
$ongoing_builds = array();
$failed_builds = array();
$builds = msort($builds, 'getStatusSortVector');
foreach ($builds as $build_ref) {
$plan = idx($plans, $build_ref->getBuildPlanPHID());
if (!$plan) {
continue;
}
$plan_behavior = $plan->getBehavior('arc-land', 'always');
$if_building = ($plan_behavior == 'building');
$if_complete = ($plan_behavior == 'complete');
$if_never = ($plan_behavior == 'never');
// If the build plan "Never" warns when landing, skip it.
if ($if_never) {
continue;
}
// If the build plan warns when landing "If Complete" but the build is
// not complete, skip it.
if ($if_complete && !$build_ref->isComplete()) {
continue;
}
// If the build plan warns when landing "If Building" but the build is
// complete, skip it.
if ($if_building && $build_ref->isComplete()) {
continue;
}
// Ignore passing builds.
if ($build_ref->isPassed()) {
continue;
}
if (!$build_ref->isComplete()) {
$ongoing_builds[] = $build_ref;
} else {
$failed_builds[] = $build_ref;
}
}
if (!$ongoing_builds && !$failed_builds) {
return;
}
if ($failed_builds) {
$this->writeWarn(
pht('BUILD FAILURES'),
pht(
'Harbormaster failed to build the active diff for this revision:'));
$prompt = pht('Land revision anyway, despite build failures?');
} else if ($ongoing_builds) {
$this->writeWarn(
pht('ONGOING BUILDS'),
pht(
'Harbormaster is still building the active diff for this revision:'));
$prompt = pht('Land revision anyway, despite ongoing build?');
}
$show_builds = array_merge($failed_builds, $ongoing_builds);
echo "\n";
foreach ($show_builds as $build_ref) {
$ansi_color = $build_ref->getStatusANSIColor();
$status_name = $build_ref->getStatusName();
$object_name = $build_ref->getObjectName();
$build_name = $build_ref->getName();
echo tsprintf(
" **<bg:".$ansi_color."> %s </bg>** %s: %s\n",
$status_name,
$object_name,
$build_name);
}
echo tsprintf(
"\n%s\n\n",
pht('You can review build details here:'));
foreach ($buildables as $buildable) {
$buildable_uri = id(new PhutilURI($this->getConduitURI()))
->setPath(sprintf('/B%d', $buildable['id']));
echo tsprintf(
" **%s**: __%s__\n",
pht('Buildable %d', $buildable['id']),
$buildable_uri);
}
if (!phutil_console_confirm($prompt)) {
throw new ArcanistUserAbortException();
}
}
public function buildEngineMessage(ArcanistLandEngine $engine) {
// TODO: This is oh-so-gross.
$this->findRevision();
......
......@@ -307,6 +307,14 @@ EOTEXT
$repository_api->execxLocal('checkout -b %s', $branch_name);
}
// Synchronize submodule state, since the checkout may have modified
// submodule references. See PHI1083.
// Note that newer versions of "git checkout" include a
// "--recurse-submodules" flag which accomplishes this goal a little
// more simply. For now, use the more compatible form.
$repository_api->execPassthru('submodule update --init --recursive');
echo phutil_console_format(
"%s\n",
pht(
......@@ -714,35 +722,67 @@ EOTEXT
throw new ArcanistUsageException(pht('Unable to apply patch!'));
}
// See PHI1083 and PHI648. If the patch applied changes to submodules,
// it only updates the submodule pointer, not the actual submodule. We're
// left with the pointer update staged in the index, and the unmodified
// submodule on disk.
// If we then "git commit --all" or "git add --all", the unmodified
// submodule on disk is added to the index as a change, which effectively
// undoes the patch we just applied and reverts the submodule back to
// the previous state.
// To avoid this, do a submodule update before we continue.
// We could also possibly skip the "--all" flag so we don't have to do
// this submodule update, but we want to leave the working copy in a
// clean state anyway, so we're going to have to do an update at some
// point. This usually doesn't cost us anything.
$repository_api->execPassthru('submodule update --init --recursive');
if ($this->shouldCommit()) {
$flags = array();
if ($bundle->getFullAuthor()) {
$author_cmd = csprintf('--author=%s', $bundle->getFullAuthor());
} else {
$author_cmd = '';
$flags[] = csprintf('--author=%s', $bundle->getFullAuthor());
}
$commit_message = $this->getCommitMessage($bundle);
$future = $repository_api->execFutureLocal(
'commit -a %C -F - --no-verify',
$author_cmd);
'commit -a %Ls -F - --no-verify',
$flags);
$future->write($commit_message);
$future->resolvex();
$verb = pht('committed');
$this->writeOkay(
pht('COMMITTED'),
pht('Successfully committed patch.'));
} else {
$verb = pht('applied');
$this->writeOkay(
pht('APPLIED'),
pht('Successfully applied patch.'));
}
if ($this->canBranch() &&
!$this->shouldBranch() &&
$this->shouldCommit() && $has_base_revision) {
$repository_api->execxLocal('checkout %s', $original_branch);
// See PHI1083 and PHI648. Synchronize submodule state after mutating
// the working copy.
$repository_api->execxLocal('checkout %s --', $original_branch);
$repository_api->execPassthru('submodule update --init --recursive');
$ex = null;
try {
$repository_api->execxLocal('cherry-pick %s', $new_branch);
$repository_api->execxLocal('cherry-pick -- %s', $new_branch);
$repository_api->execPassthru('submodule update --init --recursive');
} catch (Exception $ex) {
// do nothing
}
$repository_api->execxLocal('branch -D %s', $new_branch);
$repository_api->execxLocal('branch -D -- %s', $new_branch);
if ($ex) {
echo phutil_console_format(
"\n<bg:red>** %s**</bg>\n",
......@@ -751,15 +791,6 @@ EOTEXT
}
}
// Synchronize submodule state, since the patch may have made changes
// to ".gitmodules". We do this after we finish managing branches so
// the behavior is correct under "--nobranch"; see PHI648.
$repository_api->execPassthru('submodule update --init --recursive');
echo phutil_console_format(
"<bg:green>** %s **</bg> %s\n",
pht('OKAY'),
pht('Successfully %s patch.', $verb));
} else if ($repository_api instanceof ArcanistMercurialAPI) {
$future = $repository_api->execFutureLocal('import --no-commit -');
$future->write($bundle->toGitPatch());
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment