1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-02 09:58:24 +01:00

Merge the "Herald" and "Owners" daemon workers into a single "Publish" worker

Summary:
Depends on D20466. Ref T13277. Currently:

  - The "Owners" worker writes ownership relationships (e.g., commit X affects package Y, because it touches a path in package Y) -- these are just edges.
  - It also triggers audits.
  - Then it queues a "Herald" worker.
  - This formally publishes the commit and triggers Herald.

These aren't really separate steps and can happen more easily in one shot. Merge them.

Test Plan:
  - Ran `bin/repository reparse --publish` to republish various commits, got sensible behavior.
  - Grepped for "IMPORTED_OWNERS", "IMPORTED_HERALD", "--herald", "--owners", and "--force-local" flags.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13277

Differential Revision: https://secure.phabricator.com/D20467
This commit is contained in:
epriestley 2019-04-23 09:06:50 -07:00
parent 0fab41ff3c
commit c0a4d1de13
9 changed files with 197 additions and 291 deletions

View file

@ -4337,12 +4337,11 @@ phutil_register_library_map(array(
'PhabricatorRepositoryCommit' => 'applications/repository/storage/PhabricatorRepositoryCommit.php', 'PhabricatorRepositoryCommit' => 'applications/repository/storage/PhabricatorRepositoryCommit.php',
'PhabricatorRepositoryCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php', 'PhabricatorRepositoryCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php',
'PhabricatorRepositoryCommitData' => 'applications/repository/storage/PhabricatorRepositoryCommitData.php', 'PhabricatorRepositoryCommitData' => 'applications/repository/storage/PhabricatorRepositoryCommitData.php',
'PhabricatorRepositoryCommitHeraldWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php',
'PhabricatorRepositoryCommitHint' => 'applications/repository/storage/PhabricatorRepositoryCommitHint.php', 'PhabricatorRepositoryCommitHint' => 'applications/repository/storage/PhabricatorRepositoryCommitHint.php',
'PhabricatorRepositoryCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php', 'PhabricatorRepositoryCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php',
'PhabricatorRepositoryCommitOwnersWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php',
'PhabricatorRepositoryCommitPHIDType' => 'applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php', 'PhabricatorRepositoryCommitPHIDType' => 'applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php',
'PhabricatorRepositoryCommitParserWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php', 'PhabricatorRepositoryCommitParserWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php',
'PhabricatorRepositoryCommitPublishWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php',
'PhabricatorRepositoryCommitRef' => 'applications/repository/engine/PhabricatorRepositoryCommitRef.php', 'PhabricatorRepositoryCommitRef' => 'applications/repository/engine/PhabricatorRepositoryCommitRef.php',
'PhabricatorRepositoryCommitTestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryCommitTestCase.php', 'PhabricatorRepositoryCommitTestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryCommitTestCase.php',
'PhabricatorRepositoryConfigOptions' => 'applications/repository/config/PhabricatorRepositoryConfigOptions.php', 'PhabricatorRepositoryConfigOptions' => 'applications/repository/config/PhabricatorRepositoryConfigOptions.php',
@ -10606,15 +10605,14 @@ phutil_register_library_map(array(
), ),
'PhabricatorRepositoryCommitChangeParserWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitChangeParserWorker' => 'PhabricatorRepositoryCommitParserWorker',
'PhabricatorRepositoryCommitData' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositoryCommitData' => 'PhabricatorRepositoryDAO',
'PhabricatorRepositoryCommitHeraldWorker' => 'PhabricatorRepositoryCommitParserWorker',
'PhabricatorRepositoryCommitHint' => array( 'PhabricatorRepositoryCommitHint' => array(
'PhabricatorRepositoryDAO', 'PhabricatorRepositoryDAO',
'PhabricatorPolicyInterface', 'PhabricatorPolicyInterface',
), ),
'PhabricatorRepositoryCommitMessageParserWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitMessageParserWorker' => 'PhabricatorRepositoryCommitParserWorker',
'PhabricatorRepositoryCommitOwnersWorker' => 'PhabricatorRepositoryCommitParserWorker',
'PhabricatorRepositoryCommitPHIDType' => 'PhabricatorPHIDType', 'PhabricatorRepositoryCommitPHIDType' => 'PhabricatorPHIDType',
'PhabricatorRepositoryCommitParserWorker' => 'PhabricatorWorker', 'PhabricatorRepositoryCommitParserWorker' => 'PhabricatorWorker',
'PhabricatorRepositoryCommitPublishWorker' => 'PhabricatorRepositoryCommitParserWorker',
'PhabricatorRepositoryCommitRef' => 'Phobject', 'PhabricatorRepositoryCommitRef' => 'Phobject',
'PhabricatorRepositoryCommitTestCase' => 'PhabricatorTestCase', 'PhabricatorRepositoryCommitTestCase' => 'PhabricatorTestCase',
'PhabricatorRepositoryConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorRepositoryConfigOptions' => 'PhabricatorApplicationConfigOptions',

View file

@ -184,7 +184,7 @@ final class PhabricatorAuditEditor
foreach ($xactions as $xaction) { foreach ($xactions as $xaction) {
switch ($xaction->getTransactionType()) { switch ($xaction->getTransactionType()) {
case PhabricatorAuditTransaction::TYPE_COMMIT: case PhabricatorAuditTransaction::TYPE_COMMIT:
$import_status_flag = PhabricatorRepositoryCommit::IMPORTED_HERALD; $import_status_flag = PhabricatorRepositoryCommit::IMPORTED_PUBLISH;
break; break;
} }
} }

View file

@ -67,11 +67,8 @@ final class PhabricatorRepositoryManagementImportingWorkflow
if (!($status & PhabricatorRepositoryCommit::IMPORTED_CHANGE)) { if (!($status & PhabricatorRepositoryCommit::IMPORTED_CHANGE)) {
$need[] = pht('Change'); $need[] = pht('Change');
} }
if (!($status & PhabricatorRepositoryCommit::IMPORTED_OWNERS)) { if (!($status & PhabricatorRepositoryCommit::IMPORTED_PUBLISH)) {
$need[] = pht('Owners'); $need[] = pht('Publish');
}
if (!($status & PhabricatorRepositoryCommit::IMPORTED_HERALD)) {
$need[] = pht('Herald');
} }
$console->writeOut(' %s', implode(', ', $need)); $console->writeOut(' %s', implode(', ', $need));

View file

@ -12,14 +12,12 @@ final class PhabricatorRepositoryManagementReparseWorkflow
'**reparse** __what__ __which_parts__ [--trace] [--force]'."\n\n". '**reparse** __what__ __which_parts__ [--trace] [--force]'."\n\n".
'Rerun the Diffusion parser on specific commits and repositories. '. 'Rerun the Diffusion parser on specific commits and repositories. '.
'Mostly useful for debugging changes to Diffusion.'."\n\n". 'Mostly useful for debugging changes to Diffusion.'."\n\n".
'e.g. enqueue reparse owners in the TEST repo for all commits:'."\n".
'repository reparse --all TEST --owners'."\n\n".
'e.g. do same but exclude before yesterday (local time):'."\n". 'e.g. do same but exclude before yesterday (local time):'."\n".
'repository reparse --all TEST --owners --min-date yesterday'."\n". 'repository reparse --all TEST --change --min-date yesterday'."\n".
'repository reparse --all TEST --owners --min-date "today -1 day".'. 'repository reparse --all TEST --change --min-date "today -1 day".'.
"\n\n". "\n\n".
'e.g. do same but exclude before 03/31/2013 (local time):'."\n". 'e.g. do same but exclude before 03/31/2013 (local time):'."\n".
'repository reparse --all TEST --owners --min-date "03/31/2013"')) 'repository reparse --all TEST --change --min-date "03/31/2013"'))
->setArguments( ->setArguments(
array( array(
array( array(
@ -30,11 +28,7 @@ final class PhabricatorRepositoryManagementReparseWorkflow
'name' => 'all', 'name' => 'all',
'param' => 'repository', 'param' => 'repository',
'help' => pht( 'help' => pht(
'Reparse all commits in the specified repository. This mode '. 'Reparse all commits in the specified repository.'),
'queues parsers into the task queue; you must run taskmasters '.
'to actually do the parses. Use with __%s__ to run '.
'the tasks locally instead of with taskmasters.',
'--force-local'),
), ),
array( array(
'name' => 'min-date', 'name' => 'min-date',
@ -57,19 +51,13 @@ final class PhabricatorRepositoryManagementReparseWorkflow
), ),
array( array(
'name' => 'change', 'name' => 'change',
'help' => pht('Reparse changes.'), 'help' => pht('Reparse source changes.'),
), ),
array( array(
'name' => 'herald', 'name' => 'publish',
'help' => pht( 'help' => pht(
'Reevaluate Herald rules (may send huge amounts of email!)'), 'Publish changes: send email, publish Feed stories, run '.
), 'Herald rules, etc.'),
array(
'name' => 'owners',
'help' => pht(
'Reevaluate related commits for owners packages (may delete '.
'existing relationship entries between your package and some '.
'old commits!)'),
), ),
array( array(
'name' => 'force', 'name' => 'force',
@ -77,16 +65,14 @@ final class PhabricatorRepositoryManagementReparseWorkflow
'help' => pht('Act noninteractively, without prompting.'), 'help' => pht('Act noninteractively, without prompting.'),
), ),
array( array(
'name' => 'force-local', 'name' => 'background',
'help' => pht( 'help' => pht(
'Only used with __%s__, use this to run the tasks locally '. 'Queue tasks for the daemons instead of running them in the '.
'instead of deferring them to taskmaster daemons.', 'foreground.'),
'--all'),
), ),
array( array(
'name' => 'importing', 'name' => 'importing',
'help' => pht( 'help' => pht('Reparse all steps which have not yet completed.'),
'Reparse all steps which have not yet completed.'),
), ),
)); ));
@ -98,11 +84,10 @@ final class PhabricatorRepositoryManagementReparseWorkflow
$all_from_repo = $args->getArg('all'); $all_from_repo = $args->getArg('all');
$reparse_message = $args->getArg('message'); $reparse_message = $args->getArg('message');
$reparse_change = $args->getArg('change'); $reparse_change = $args->getArg('change');
$reparse_herald = $args->getArg('herald'); $reparse_publish = $args->getArg('publish');
$reparse_owners = $args->getArg('owners');
$reparse_what = $args->getArg('revision'); $reparse_what = $args->getArg('revision');
$force = $args->getArg('force'); $force = $args->getArg('force');
$force_local = $args->getArg('force-local'); $background = $args->getArg('background');
$min_date = $args->getArg('min-date'); $min_date = $args->getArg('min-date');
$importing = $args->getArg('importing'); $importing = $args->getArg('importing');
@ -122,17 +107,13 @@ final class PhabricatorRepositoryManagementReparseWorkflow
$commits)); $commits));
} }
$any_step = ($reparse_message || $any_step = ($reparse_message || $reparse_change || $reparse_publish);
$reparse_change ||
$reparse_herald ||
$reparse_owners);
if ($any_step && $importing) { if ($any_step && $importing) {
throw new PhutilArgumentUsageException( throw new PhutilArgumentUsageException(
pht( pht(
'Choosing steps with %s conflicts with flags which select '. 'Choosing steps with "--importing" conflicts with flags which '.
'specific steps.', 'select specific steps.'));
'--importing'));
} else if ($any_step) { } else if ($any_step) {
// OK. // OK.
} else if ($importing) { } else if ($importing) {
@ -140,12 +121,8 @@ final class PhabricatorRepositoryManagementReparseWorkflow
} else if (!$any_step && !$importing) { } else if (!$any_step && !$importing) {
throw new PhutilArgumentUsageException( throw new PhutilArgumentUsageException(
pht( pht(
'Specify which steps to reparse with %s, or %s, %s, %s, or %s.', 'Specify which steps to reparse with "--message", "--change", '.
'--importing', 'and/or "--publish"; or "--importing" to run all missing steps.'));
'--message',
'--change',
'--herald',
'--owners'));
} }
$min_timestamp = false; $min_timestamp = false;
@ -155,9 +132,7 @@ final class PhabricatorRepositoryManagementReparseWorkflow
if (!$all_from_repo) { if (!$all_from_repo) {
throw new PhutilArgumentUsageException( throw new PhutilArgumentUsageException(
pht( pht(
"You must use --all if you specify --min-date\n". 'You must use "--all" if you specify "--min-date".'));
"e.g.\n".
" repository reparse --all TEST --owners --min-date yesterday"));
} }
// previous to PHP 5.1.0 you would compare with -1, instead of false // previous to PHP 5.1.0 you would compare with -1, instead of false
@ -170,19 +145,6 @@ final class PhabricatorRepositoryManagementReparseWorkflow
} }
} }
if ($reparse_owners && !$force) {
$console->writeOut(
"%s\n",
pht(
'You are about to recreate the relationship entries between the '.
'commits and the packages they touch. This might delete some '.
'existing relationship entries for some old commits.'));
if (!phutil_console_confirm(pht('Are you ready to continue?'))) {
throw new PhutilArgumentUsageException(pht('Cancelled.'));
}
}
$commits = array(); $commits = array();
if ($all_from_repo) { if ($all_from_repo) {
$repository = id(new PhabricatorRepositoryQuery()) $repository = id(new PhabricatorRepositoryQuery())
@ -219,14 +181,8 @@ final class PhabricatorRepositoryManagementReparseWorkflow
$commits = $this->loadNamedCommits($reparse_what); $commits = $this->loadNamedCommits($reparse_what);
} }
if ($all_from_repo && !$force_local) { if (!$background) {
$console->writeOut("%s\n", pht( PhabricatorWorker::setRunAllTasksInProcess(true);
"**NOTE**: This script will queue tasks to reparse the data. Once the ".
"tasks have been queued, you need to run Taskmaster daemons to ".
"execute them.\n\n%s",
pht(
'QUEUEING TASKS (%s Commit(s)):',
phutil_count($commits))));
} }
$progress = new PhutilConsoleProgressBar(); $progress = new PhutilConsoleProgressBar();
@ -241,16 +197,13 @@ final class PhabricatorRepositoryManagementReparseWorkflow
// Find the first missing import step and queue that up. // Find the first missing import step and queue that up.
$reparse_message = false; $reparse_message = false;
$reparse_change = false; $reparse_change = false;
$reparse_owners = false; $reparse_publish = false;
$reparse_herald = false;
if (!($status & PhabricatorRepositoryCommit::IMPORTED_MESSAGE)) { if (!($status & PhabricatorRepositoryCommit::IMPORTED_MESSAGE)) {
$reparse_message = true; $reparse_message = true;
} else if (!($status & PhabricatorRepositoryCommit::IMPORTED_CHANGE)) { } else if (!($status & PhabricatorRepositoryCommit::IMPORTED_CHANGE)) {
$reparse_change = true; $reparse_change = true;
} else if (!($status & PhabricatorRepositoryCommit::IMPORTED_OWNERS)) { } else if (!($status & PhabricatorRepositoryCommit::IMPORTED_PUBLISH)) {
$reparse_owners = true; $reparse_publish = true;
} else if (!($status & PhabricatorRepositoryCommit::IMPORTED_HERALD)) {
$reparse_herald = true;
} else { } else {
continue; continue;
} }
@ -285,12 +238,8 @@ final class PhabricatorRepositoryManagementReparseWorkflow
break; break;
} }
if ($reparse_herald) { if ($reparse_publish) {
$classes[] = 'PhabricatorRepositoryCommitHeraldWorker'; $classes[] = 'PhabricatorRepositoryCommitPublishWorker';
}
if ($reparse_owners) {
$classes[] = 'PhabricatorRepositoryCommitOwnersWorker';
} }
// NOTE: With "--importing", we queue the first unparsed step and let // NOTE: With "--importing", we queue the first unparsed step and let
@ -298,20 +247,10 @@ final class PhabricatorRepositoryManagementReparseWorkflow
// all the requested steps explicitly. // all the requested steps explicitly.
$spec = array( $spec = array(
'commitID' => $commit->getID(), 'commitID' => $commit->getID(),
'only' => !$importing, 'only' => !$importing,
); );
if ($all_from_repo && !$force_local) {
$background = true;
} else {
$background = false;
}
if (!$background) {
PhabricatorWorker::setRunAllTasksInProcess(true);
}
foreach ($classes as $class) { foreach ($classes as $class) {
PhabricatorWorker::scheduleTask( PhabricatorWorker::scheduleTask(
$class, $class,

View file

@ -1003,7 +1003,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
$done = 0; $done = 0;
$total = 0; $total = 0;
foreach ($progress as $row) { foreach ($progress as $row) {
$total += $row['N'] * 4; $total += $row['N'] * 3;
$status = $row['importStatus']; $status = $row['importStatus'];
if ($status & PhabricatorRepositoryCommit::IMPORTED_MESSAGE) { if ($status & PhabricatorRepositoryCommit::IMPORTED_MESSAGE) {
$done += $row['N']; $done += $row['N'];
@ -1011,10 +1011,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
if ($status & PhabricatorRepositoryCommit::IMPORTED_CHANGE) { if ($status & PhabricatorRepositoryCommit::IMPORTED_CHANGE) {
$done += $row['N']; $done += $row['N'];
} }
if ($status & PhabricatorRepositoryCommit::IMPORTED_OWNERS) { if ($status & PhabricatorRepositoryCommit::IMPORTED_PUBLISH) {
$done += $row['N'];
}
if ($status & PhabricatorRepositoryCommit::IMPORTED_HERALD) {
$done += $row['N']; $done += $row['N'];
} }
} }

View file

@ -33,9 +33,8 @@ final class PhabricatorRepositoryCommit
const IMPORTED_MESSAGE = 1; const IMPORTED_MESSAGE = 1;
const IMPORTED_CHANGE = 2; const IMPORTED_CHANGE = 2;
const IMPORTED_OWNERS = 4; const IMPORTED_PUBLISH = 8;
const IMPORTED_HERALD = 8; const IMPORTED_ALL = 11;
const IMPORTED_ALL = 15;
const IMPORTED_CLOSEABLE = 1024; const IMPORTED_CLOSEABLE = 1024;
const IMPORTED_UNREACHABLE = 2048; const IMPORTED_UNREACHABLE = 2048;

View file

@ -1,146 +0,0 @@
<?php
final class PhabricatorRepositoryCommitHeraldWorker
extends PhabricatorRepositoryCommitParserWorker {
protected function getImportStepFlag() {
return PhabricatorRepositoryCommit::IMPORTED_HERALD;
}
public function getRequiredLeaseTime() {
// Herald rules may take a long time to process.
return phutil_units('4 hours in seconds');
}
protected function parseCommit(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
$viewer = PhabricatorUser::getOmnipotentUser();
if ($this->shouldSkipImportStep()) {
// This worker has no followup tasks, so we can just bail out
// right away without queueing anything.
return;
}
// Reload the commit to pull commit data and audit requests.
$commit = id(new DiffusionCommitQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
->withIDs(array($commit->getID()))
->needCommitData(true)
->needAuditRequests(true)
->executeOne();
$data = $commit->getCommitData();
if (!$data) {
throw new PhabricatorWorkerPermanentFailureException(
pht(
'Unable to load commit data. The data for this task is invalid '.
'or no longer exists.'));
}
$commit->attachRepository($repository);
$content_source = $this->newContentSource();
$committer_phid = $data->getCommitDetail('committerPHID');
$author_phid = $data->getCommitDetail('authorPHID');
$acting_as_phid = nonempty(
$committer_phid,
$author_phid,
id(new PhabricatorDiffusionApplication())->getPHID());
$editor = id(new PhabricatorAuditEditor())
->setActor($viewer)
->setActingAsPHID($acting_as_phid)
->setContinueOnMissingFields(true)
->setContinueOnNoEffect(true)
->setContentSource($content_source);
$xactions = array();
$xactions[] = id(new PhabricatorAuditTransaction())
->setTransactionType(PhabricatorAuditTransaction::TYPE_COMMIT)
->setDateCreated($commit->getEpoch())
->setNewValue(array(
'description' => $data->getCommitMessage(),
'summary' => $data->getSummary(),
'authorName' => $data->getAuthorName(),
'authorPHID' => $commit->getAuthorPHID(),
'committerName' => $data->getCommitDetail('committer'),
'committerPHID' => $data->getCommitDetail('committerPHID'),
));
try {
$raw_patch = $this->loadRawPatchText($repository, $commit);
} catch (Exception $ex) {
$raw_patch = pht('Unable to generate patch: %s', $ex->getMessage());
}
$editor->setRawPatch($raw_patch);
return $editor->applyTransactions($commit, $xactions);
}
private function loadRawPatchText(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
$viewer = PhabricatorUser::getOmnipotentUser();
$identifier = $commit->getCommitIdentifier();
$drequest = DiffusionRequest::newFromDictionary(
array(
'user' => $viewer,
'repository' => $repository,
));
$time_key = 'metamta.diffusion.time-limit';
$byte_key = 'metamta.diffusion.byte-limit';
$time_limit = PhabricatorEnv::getEnvConfig($time_key);
$byte_limit = PhabricatorEnv::getEnvConfig($byte_key);
$diff_info = DiffusionQuery::callConduitWithDiffusionRequest(
$viewer,
$drequest,
'diffusion.rawdiffquery',
array(
'commit' => $identifier,
'linesOfContext' => 3,
'timeout' => $time_limit,
'byteLimit' => $byte_limit,
));
if ($diff_info['tooSlow']) {
throw new Exception(
pht(
'Patch generation took longer than configured limit ("%s") of '.
'%s second(s).',
$time_key,
new PhutilNumber($time_limit)));
}
if ($diff_info['tooHuge']) {
$pretty_limit = phutil_format_bytes($byte_limit);
throw new Exception(
pht(
'Patch size exceeds configured byte size limit ("%s") of %s.',
$byte_key,
$pretty_limit));
}
$file_phid = $diff_info['filePHID'];
$file = id(new PhabricatorFileQuery())
->setViewer($viewer)
->withPHIDs(array($file_phid))
->executeOne();
if (!$file) {
throw new Exception(
pht(
'Failed to load file ("%s") returned by "%s".',
$file_phid,
'diffusion.rawdiffquery'));
}
return $file->loadFileData();
}
}

View file

@ -1,10 +1,15 @@
<?php <?php
final class PhabricatorRepositoryCommitOwnersWorker final class PhabricatorRepositoryCommitPublishWorker
extends PhabricatorRepositoryCommitParserWorker { extends PhabricatorRepositoryCommitParserWorker {
protected function getImportStepFlag() { protected function getImportStepFlag() {
return PhabricatorRepositoryCommit::IMPORTED_OWNERS; return PhabricatorRepositoryCommit::IMPORTED_PUBLISH;
}
public function getRequiredLeaseTime() {
// Herald rules may take a long time to process.
return phutil_units('4 hours in seconds');
} }
protected function parseCommit( protected function parseCommit(
@ -12,20 +17,15 @@ final class PhabricatorRepositoryCommitOwnersWorker
PhabricatorRepositoryCommit $commit) { PhabricatorRepositoryCommit $commit) {
if (!$this->shouldSkipImportStep()) { if (!$this->shouldSkipImportStep()) {
$this->triggerOwnerAudits($repository, $commit); $this->publishCommit($repository, $commit);
$commit->writeImportStatusFlag($this->getImportStepFlag()); $commit->writeImportStatusFlag($this->getImportStepFlag());
} }
if ($this->shouldQueueFollowupTasks()) { // This is the last task in the sequence, so we don't need to queue any
$this->queueTask( // followup workers.
'PhabricatorRepositoryCommitHeraldWorker',
array(
'commitID' => $commit->getID(),
));
}
} }
private function triggerOwnerAudits( private function publishCommit(
PhabricatorRepository $repository, PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) { PhabricatorRepositoryCommit $commit) {
$viewer = PhabricatorUser::getOmnipotentUser(); $viewer = PhabricatorUser::getOmnipotentUser();
@ -35,6 +35,88 @@ final class PhabricatorRepositoryCommitOwnersWorker
return; return;
} }
$commit_phid = $commit->getPHID();
// Reload the commit to get the commit data, identities, and any
// outstanding audit requests.
$commit = id(new DiffusionCommitQuery())
->setViewer($viewer)
->withPHIDs(array($commit_phid))
->needCommitData(true)
->needIdentities(true)
->needAuditRequests(true)
->executeOne();
if (!$commit) {
throw new PhabricatorWorkerPermanentFailureException(
pht(
'Failed to reload commit "%s".',
$commit_phid));
}
$xactions = array(
$this->newAuditTransactions($commit),
$this->newPublishTransactions($commit),
);
$xactions = array_mergev($xactions);
$acting_phid = $this->getPublishAsPHID($commit);
$content_source = $this->newContentSource();
$editor = $commit->getApplicationTransactionEditor()
->setActor($viewer)
->setActingAsPHID($acting_phid)
->setContinueOnNoEffect(true)
->setContinueOnMissingFields(true)
->setContentSource($content_source);
try {
$raw_patch = $this->loadRawPatchText($repository, $commit);
} catch (Exception $ex) {
$raw_patch = pht('Unable to generate patch: %s', $ex->getMessage());
}
$editor->setRawPatch($raw_patch);
$editor->applyTransactions($commit, $xactions);
}
private function getPublishAsPHID(PhabricatorRepositoryCommit $commit) {
if ($commit->hasCommitterIdentity()) {
return $commit->getCommitterIdentity()->getIdentityDisplayPHID();
}
if ($commit->hasAuthorIdentity()) {
return $commit->getAuthorIdentity()->getIdentityDisplayPHID();
}
return id(new PhabricatorDiffusionApplication())->getPHID();
}
private function newPublishTransactions(PhabricatorRepositoryCommit $commit) {
$data = $commit->getCommitData();
$xactions = array();
$xactions[] = $commit->getApplicationTransactionTemplate()
->setTransactionType(PhabricatorAuditTransaction::TYPE_COMMIT)
->setDateCreated($commit->getEpoch())
->setNewValue(
array(
'description' => $data->getCommitMessage(),
'summary' => $data->getSummary(),
'authorName' => $data->getAuthorName(),
'authorPHID' => $commit->getAuthorPHID(),
'committerName' => $data->getCommitDetail('committer'),
'committerPHID' => $data->getCommitDetail('committerPHID'),
));
return $xactions;
}
private function newAuditTransactions(PhabricatorRepositoryCommit $commit) {
$viewer = PhabricatorUser::getOmnipotentUser();
$repository = $commit->getRepository();
$affected_paths = PhabricatorOwnerPathQuery::loadAffectedPaths( $affected_paths = PhabricatorOwnerPathQuery::loadAffectedPaths(
$repository, $repository,
$commit, $commit,
@ -47,17 +129,7 @@ final class PhabricatorRepositoryCommitOwnersWorker
$commit->writeOwnersEdges(mpull($affected_packages, 'getPHID')); $commit->writeOwnersEdges(mpull($affected_packages, 'getPHID'));
if (!$affected_packages) { if (!$affected_packages) {
return; return array();
}
$commit = id(new DiffusionCommitQuery())
->setViewer($viewer)
->withPHIDs(array($commit->getPHID()))
->needCommitData(true)
->needAuditRequests(true)
->executeOne();
if (!$commit) {
return;
} }
$data = $commit->getCommitData(); $data = $commit->getCommitData();
@ -99,16 +171,11 @@ final class PhabricatorRepositoryCommitOwnersWorker
// If none of the packages are triggering audits, we're all done. // If none of the packages are triggering audits, we're all done.
if (!$auditor_phids) { if (!$auditor_phids) {
return; return array();
} }
$audit_type = DiffusionCommitAuditorsTransaction::TRANSACTIONTYPE; $audit_type = DiffusionCommitAuditorsTransaction::TRANSACTIONTYPE;
$owners_phid = id(new PhabricatorOwnersApplication())
->getPHID();
$content_source = $this->newContentSource();
$xactions = array(); $xactions = array();
$xactions[] = $commit->getApplicationTransactionTemplate() $xactions[] = $commit->getApplicationTransactionTemplate()
->setTransactionType($audit_type) ->setTransactionType($audit_type)
@ -117,14 +184,7 @@ final class PhabricatorRepositoryCommitOwnersWorker
'+' => array_fuse($auditor_phids), '+' => array_fuse($auditor_phids),
)); ));
$editor = $commit->getApplicationTransactionEditor() return $xactions;
->setActor($viewer)
->setActingAsPHID($owners_phid)
->setContinueOnNoEffect(true)
->setContinueOnMissingFields(true)
->setContentSource($content_source);
$editor->applyTransactions($commit, $xactions);
} }
private function shouldTriggerAudit( private function shouldTriggerAudit(
@ -253,4 +313,66 @@ final class PhabricatorRepositoryCommitOwnersWorker
return false; return false;
} }
private function loadRawPatchText(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
$viewer = PhabricatorUser::getOmnipotentUser();
$identifier = $commit->getCommitIdentifier();
$drequest = DiffusionRequest::newFromDictionary(
array(
'user' => $viewer,
'repository' => $repository,
));
$time_key = 'metamta.diffusion.time-limit';
$byte_key = 'metamta.diffusion.byte-limit';
$time_limit = PhabricatorEnv::getEnvConfig($time_key);
$byte_limit = PhabricatorEnv::getEnvConfig($byte_key);
$diff_info = DiffusionQuery::callConduitWithDiffusionRequest(
$viewer,
$drequest,
'diffusion.rawdiffquery',
array(
'commit' => $identifier,
'linesOfContext' => 3,
'timeout' => $time_limit,
'byteLimit' => $byte_limit,
));
if ($diff_info['tooSlow']) {
throw new Exception(
pht(
'Patch generation took longer than configured limit ("%s") of '.
'%s second(s).',
$time_key,
new PhutilNumber($time_limit)));
}
if ($diff_info['tooHuge']) {
$pretty_limit = phutil_format_bytes($byte_limit);
throw new Exception(
pht(
'Patch size exceeds configured byte size limit ("%s") of %s.',
$byte_key,
$pretty_limit));
}
$file_phid = $diff_info['filePHID'];
$file = id(new PhabricatorFileQuery())
->setViewer($viewer)
->withPHIDs(array($file_phid))
->executeOne();
if (!$file) {
throw new Exception(
pht(
'Failed to load file ("%s") returned by "%s".',
$file_phid,
'diffusion.rawdiffquery'));
}
return $file->loadFileData();
}
} }

View file

@ -102,7 +102,7 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker
$commit = $this->commit; $commit = $this->commit;
if ($this->shouldQueueFollowupTasks()) { if ($this->shouldQueueFollowupTasks()) {
$this->queueTask( $this->queueTask(
'PhabricatorRepositoryCommitOwnersWorker', 'PhabricatorRepositoryCommitPublishWorker',
array( array(
'commitID' => $commit->getID(), 'commitID' => $commit->getID(),
)); ));