Page MenuHomePhabricator
Differential D20221 Diff 48282 src/workflow/ArcanistDiffWorkflow.php

Changeset View

Standalone View

View Options

src/workflow/ArcanistDiffWorkflow.php

Show First 20 Lines • Show All 2,158 Lines • ▼ Show 20 Lines private function getGitUpdateMessage() {
} }
// We have more than one message, so figure out which ones are new. We // We have more than one message, so figure out which ones are new. We
// do this by pulling the current diff and comparing commit hashes in the // do this by pulling the current diff and comparing commit hashes in the
// working copy with attached commit hashes. It's not super important that // working copy with attached commit hashes. It's not super important that
// we always get this 100% right, we're just trying to do something // we always get this 100% right, we're just trying to do something
// reasonable. // reasonable.
$local = $this->loadActiveLocalCommitInfo(); $hashes = $this->loadActiveDiffLocalCommitHashes();
$hashes = ipull($local, null, 'commit'); $hashes = array_fuse($hashes);
$usable = array(); $usable = array();
foreach ($commit_messages as $message) { foreach ($commit_messages as $message) {
$text = $message->getMetadata('message'); $text = $message->getMetadata('message');
$parsed = ArcanistDifferentialCommitMessage::newFromRawCorpus($text); $parsed = ArcanistDifferentialCommitMessage::newFromRawCorpus($text);
if ($parsed->getRevisionID()) { if ($parsed->getRevisionID()) {
// If this is an amended commit message with a revision ID, it's // If this is an amended commit message with a revision ID, it's
Show All 30 Lines private function getMercurialUpdateMessage() {
$messages = $repository_api->getCommitMessageLog(); $messages = $repository_api->getCommitMessageLog();
if (count($messages) == 1) { if (count($messages) == 1) {
// If there's only one message, assume this is an amend-based workflow and // If there's only one message, assume this is an amend-based workflow and
// that using it to prefill doesn't make sense. // that using it to prefill doesn't make sense.
return null; return null;
} }
$local = $this->loadActiveLocalCommitInfo(); $hashes = $this->loadActiveDiffLocalCommitHashes();
$hashes = ipull($local, null, 'commit'); $hashes = array_fuse($hashes);
$usable = array(); $usable = array();
foreach ($messages as $rev => $message) { foreach ($messages as $rev => $message) {
if (isset($hashes[$rev])) { if (isset($hashes[$rev])) {
// If this commit is currently part of the active diff on the revision, // If this commit is currently part of the active diff on the revision,
// stop using commit messages, since anything older than this isn't new. // stop using commit messages, since anything older than this isn't new.
break; break;
} }
Show All 31 Lines foreach ($usable as $message) {
$text = trim($message); $text = trim($message);
$text = head(explode("\n", $text)); $text = head(explode("\n", $text));
$default[] = ' - '.$text."\n"; $default[] = ' - '.$text."\n";
} }
return implode('', $default); 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'];
jmeadorUnsubmitted
Not Done
Inline Actions

any reason this cannot be:

$diff_phid = $revision['fields']['diffPHID'];

And then differential.diff.search could use that instead of inferring the diff ID?

jmeador: any reason this cannot be: ```lang=php $diff_phid = $revision['fields']['diffPHID']; ``` And…
$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);
}
amckinleyUnsubmitted
Not Done
Inline Actions

Just out of curiosity, how could this stuff fail? I see how the control flow gets us back to the legacy version if anything goes wrong, and from your test plan, it's not obvious to me that we know if the new version works.

amckinley: Just out of curiosity, how could this stuff fail? I see how the control flow gets us back to…
}
}
} catch (Exception $ex) {
// If any of this fails, fall back to the older method below.
}
$current_diff = $this->getConduit()->callMethodSynchronous( $current_diff = $this->getConduit()->callMethodSynchronous(
'differential.querydiffs', 'differential.querydiffs',
array( array(
'revisionIDs' => array($this->revisionID), 'revisionIDs' => array($this->revisionID),
)); ));
$current_diff = head($current_diff); $current_diff = head($current_diff);
$properties = idx($current_diff, 'properties', array()); $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);
} }
/* -( Diff Specification )------------------------------------------------- */ /* -( Diff Specification )------------------------------------------------- */
/** /**
* @task diffspec * @task diffspec
▲ Show 20 Lines • Show All 661 Lines • Show Last 20 Lines
Phabricator · Built by Phacility