Page MenuHomePhabricator
Files F15427052

D9113.id21651.diff
No OneTemporary
Actions

D9113.id21651.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -753,6 +753,7 @@
'HarbormasterDAO' => 'applications/harbormaster/storage/HarbormasterDAO.php',
'HarbormasterHTTPRequestBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php',
'HarbormasterLeaseHostBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php',
+ 'HarbormasterLintSaveBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLintSaveBuildStepImplementation.php',
'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php',
'HarbormasterManagementUpdateWorkflow' => 'applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php',
'HarbormasterManagementWorkflow' => 'applications/harbormaster/management/HarbormasterManagementWorkflow.php',
@@ -3446,6 +3447,7 @@
'HarbormasterDAO' => 'PhabricatorLiskDAO',
'HarbormasterHTTPRequestBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterLeaseHostBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
+ 'HarbormasterLintSaveBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow',
'HarbormasterManagementUpdateWorkflow' => 'HarbormasterManagementWorkflow',
'HarbormasterManagementWorkflow' => 'PhabricatorManagementWorkflow',
diff --git a/src/applications/harbormaster/step/HarbormasterLintSaveBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLintSaveBuildStepImplementation.php
new file mode 100644
--- /dev/null
+++ b/src/applications/harbormaster/step/HarbormasterLintSaveBuildStepImplementation.php
@@ -0,0 +1,402 @@
+<?php
+
+final class HarbormasterLintSaveBuildStepImplementation
+ extends HarbormasterBuildStepImplementation {
+
+ public function getName() {
+ return pht('Lint and Save');
+ }
+
+ public function getGenericDescription() {
+ return pht('Run `arc lint` and report the results into Diffusion.');
+ }
+
+ public function getDescription() {
+ return pht(
+ 'Run `arc lint` on host %s and report the results into Diffusion.',
+ $this->formatSettingForDescription('hostartifact'));
+ }
+
+ private $inserts = array();
+
+ public function execute(
+ HarbormasterBuild $build,
+ HarbormasterBuildTarget $build_target) {
+
+ $resolve_log = $build->createLog($build_target, "lint", "determine");
+ $resolve_start = $resolve_log->start();
+
+ $settings = $this->getSettings();
+ $artifact = $build->loadArtifact($settings['hostartifact']);
+ $lease = $artifact->loadDrydockLease();
+ $interface = $lease->getInterface('command');
+
+ $buildable_ref = id(new HarbormasterBuildableQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withPHIDs(array($build->getBuildablePHID()))
+ ->needContainerObjects(true)
+ ->executeOne();
+
+ $buildable = $buildable_ref->getBuildableObject();
+
+ // We skip differential revisions here because it doesn't make sense.
+ if (!($buildable instanceof PhabricatorRepositoryCommit)) {
+ $resolve_log->append(pht('The current buildable is not a commit, so lint will be skipped')."\n");
+ $resolve_log->finalize($resolve_start);
+ return;
+ }
+
+ // Alias just to make code a bit more readable. This is always a
+ // commit when we get to here.
+ $commit = $buildable;
+ $repository = $buildable_ref->getContainerObject();
+
+ $all = null;
+ if ($settings['lintall']) {
+ $all = '--everything';
+ }
+
+ // Work out the branch based on the commit identifier and
+ // the repository. If we don't match any branch, just exit now
+ // since we can't generate useful data.
+ $branches = id(new ConduitCall(
+ 'diffusion.branchquery',
+ array(
+ 'callsign' => $repository->getCallsign()
+ )))
+ ->setUser(PhabricatorUser::getOmnipotentUser())
+ ->execute();
+
+ $mapped_branch = null;
+ foreach ($branches as $branch_json) {
+ if ($branch_json["commitIdentifier"] == $commit->getCommitIdentifier()) {
+ $mapped_branch = $branch_json["shortName"];
+ }
+ }
+
+ if ($mapped_branch === null) {
+ // No branch for this commit, which means we can't store lint data.
+ $resolve_log->append(pht('No branch points to commit %s', $commit->getCommitIdentifier())."\n");
+ $resolve_log->append(pht('The following branches are present:')."\n");
+ foreach ($branches as $branch_json) {
+ $resolve_log->append(pht(' - %s: %s', $branch_json["shortName"], $branch_json["commitIdentifier"])."\n");
+ }
+ $resolve_log->finalize($resolve_start);
+ return;
+ }
+
+ $branch = new PhabricatorRepositoryBranch();
+ $conn = $branch->establishConnection('w');
+ $branch = $branch->loadOneWhere(
+ 'repositoryID = %d AND name = %s',
+ $repository->getID(),
+ $mapped_branch);
+ if (!$branch) {
+ $branch = id(new PhabricatorRepositoryBranch())
+ ->setRepositoryID($repository->getID())
+ ->setName($mapped_branch)
+ ->save();
+ }
+
+ // TODO: This might be better solved by having a --working-dir
+ // argument to all Arcanist commands, so we can get Arcanist
+ // to change directory early on without this platform specific
+ // nonsense.
+ if ($lease->getAttribute('platform') === 'windows') {
+ if (substr_count($settings['path'], '&') > 0) {
+ $resolve_log->append(pht('Unsafe Windows path detected')."\n");
+ $resolve_log->finalize($resolve_start);
+ return;
+ }
+
+ $change_directory = csprintf('cd %C &', $settings['path']);
+ } else {
+ $change_directory = csprintf('cd %s &&', $settings['path']);
+ }
+
+ // ===== RUN ARCANIST =====
+ $arcrc = null;
+ if ($settings['arcrcpath']) {
+ if (substr_count($settings['arcrcpath'], '&') > 0) {
+ $resolve_log->append(pht('Unsafe Windows arcrc path detected')."\n");
+ $resolve_log->finalize($resolve_start);
+ return;
+ }
+
+ $arcrc = csprintf('--arcrc-file %C', $settings['arcrcpath']);
+ }
+
+ if (substr_count($settings['severity'], '&') > 0) {
+ $resolve_log->append(pht('Unsafe severity level detected')."\n");
+ $resolve_log->finalize($resolve_start);
+ return;
+ }
+
+ // FIXME: --rev should be since the last lint!
+ $future = $interface->getExecFuture(
+ '%C %C lint %C --lintall --severity %C %C --output json',
+ $change_directory,
+ $settings['arcanist'],
+ $arcrc,
+ $settings['severity'],
+ $all);
+
+ $executed_command = csprintf(
+ '%C %C lint %C --lintall --severity %C %C --output json',
+ $change_directory,
+ $settings['arcanist'],
+ $arcrc,
+ $settings['severity'],
+ $all);
+
+ $resolve_log->append(pht('Starting Arcanist with command: %s', $executed_command)."\n");
+ $resolve_log->finalize($resolve_start);
+
+ $log_stdout = $build->createLog($build_target, "arcanist", "stdout");
+ $log_stderr = $build->createLog($build_target, "arcanist", "stderr");
+
+ $start_stdout = $log_stdout->start();
+ $start_stderr = $log_stderr->start();
+
+ $jsons = '';
+
+ // Read the next amount of available output every second.
+ while (!$future->isReady()) {
+ list($stdout, $stderr) = $future->read();
+ $jsons .= $stdout;
+ $log_stdout->append($stdout);
+ $log_stderr->append($stderr);
+ $future->discardBuffers();
+
+ // Wait one second before querying for more data.
+ sleep(1);
+ }
+
+ // Get the return value so we can log that as well.
+ list($err) = $future->resolve();
+
+ // Retrieve the last few bits of information.
+ list($stdout, $stderr) = $future->read();
+ $jsons .= $stdout;
+ $log_stdout->append($stdout);
+ $log_stderr->append($stderr);
+ $future->discardBuffers();
+
+ $log_stdout->finalize($start_stdout);
+ $log_stderr->finalize($start_stderr);
+
+ // ===== ARCANIST COMPLETE =====
+
+ if ($err !== 0) {
+ throw new Exception(pht('Arcanist failed with error %d.', $err));
+ }
+
+ $inserts = array();
+ $deletes = array();
+
+ $lint_log = $build->createLog($build_target, "lint", "parse");
+ $lint_start = $lint_log->start();
+
+ // NOTE: We can't use --trace and capture STDERR as some sort of
+ // progress indicator, because that doesn't work on Windows (SSH
+ // on Windows routes everything to STDOUT).
+ foreach (phutil_split_lines($jsons) as $json) {
+ if (strlen(trim($json)) === 0) {
+ $lint_log->append(pht('Skipping line: %s', $json)."\n");
+ continue;
+ }
+
+ $lint_log->append(pht('Got JSON line: %s', $json)."\n");
+
+ $paths = phutil_json_decode($json);
+ if (!is_array($paths)) {
+ throw new Exception("Invalid JSON returns from Arcanist");
+ }
+
+ foreach ($paths as $path => $messages) {
+ $path = '/'.trim(str_replace("\\", '/', $path), '/');
+
+ $deletes[] = $path;
+
+ $lint_log->append(pht('%s has %s lint messages', $path, count($messages))."\n");
+
+ foreach ($messages as $message) {
+ $line = idx($message, 'line', 0);
+
+ $inserts[] = qsprintf(
+ $conn,
+ '(%d, %s, %d, %s, %s, %s, %s)',
+ $branch->getID(),
+ $path,
+ $line,
+ idx($message, 'code', ''),
+ idx($message, 'severity', ''),
+ idx($message, 'name', ''),
+ idx($message, 'description', ''));
+
+ // if ($line && $this->needsBlame) {
+ // $this->blame[$path][$line] = true;
+ // }
+ }
+
+ if (count($deletes) >= 1024 || count($inserts) >= 256) {
+ $lint_log->append(pht('Saving messages to database')."\n");
+
+ $this->saveLintMessages($conn, $branch, $deletes, $inserts);
+ $deletes = array();
+ $inserts = array();
+ }
+ }
+ }
+
+ $lint_log->append(pht('Saving messages to database')."\n");
+ $this->saveLintMessages($conn, $branch, $deletes, $inserts);
+
+ $lint_log->append(pht('Saving branch information')."\n");
+ $branch->setLintCommit($commit->getCommitIdentifier());
+ $branch->save();
+
+ /* if ($this->blame) {
+ $this->blameAuthors();
+ $this->blame = array();
+ }*/
+
+ $lint_log->append(pht('Complete')."\n");
+ $lint_log->finalize($lint_start);
+ }
+
+ private function saveLintMessages($conn, $branch, $deletes, $inserts) {
+ $conn->openTransaction();
+
+ foreach (array_chunk($deletes, 1024) as $paths) {
+ queryfx(
+ $conn,
+ 'DELETE FROM %T WHERE branchID = %d AND path IN (%Ls)',
+ PhabricatorRepository::TABLE_LINTMESSAGE,
+ $branch->getID(),
+ $paths);
+ }
+
+ foreach (array_chunk($inserts, 256) as $values) {
+ queryfx(
+ $conn,
+ 'INSERT INTO %T
+ (branchID, path, line, code, severity, name, description)
+ VALUES %Q',
+ PhabricatorRepository::TABLE_LINTMESSAGE,
+ implode(', ', $values));
+ }
+
+ $conn->saveTransaction();
+ }
+
+/*
+ private function blameAuthors() {
+ $repository = id(new PhabricatorRepositoryQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withIDs(array($this->branch->getRepositoryID()))
+ ->executeOne();
+
+ $queries = array();
+ $futures = array();
+ foreach ($this->blame as $path => $lines) {
+ $drequest = DiffusionRequest::newFromDictionary(array(
+ 'user' => PhabricatorUser::getOmnipotentUser(),
+ 'initFromConduit' => false,
+ 'repository' => $repository,
+ 'branch' => $this->branch->getName(),
+ 'path' => $path,
+ 'commit' => $this->lintCommit,
+ ));
+ $query = DiffusionFileContentQuery::newFromDiffusionRequest($drequest)
+ ->setNeedsBlame(true);
+ $queries[$path] = $query;
+ $futures[$path] = $query->getFileContentFuture();
+ }
+
+ $authors = array();
+
+ foreach (Futures($futures)->limit(8) as $path => $future) {
+ $queries[$path]->loadFileContentFromFuture($future);
+ list(, $rev_list, $blame_dict) = $queries[$path]->getBlameData();
+ foreach (array_keys($this->blame[$path]) as $line) {
+ $commit_identifier = $rev_list[$line - 1];
+ $author = idx($blame_dict[$commit_identifier], 'authorPHID');
+ if ($author) {
+ $authors[$author][$path][] = $line;
+ }
+ }
+ }
+
+ if ($authors) {
+ $this->conn->openTransaction();
+
+ foreach ($authors as $author => $paths) {
+ $where = array();
+ foreach ($paths as $path => $lines) {
+ $where[] = qsprintf(
+ $this->conn,
+ '(path = %s AND line IN (%Ld))',
+ $this->svnRoot.'/'.$path,
+ $lines);
+ }
+ queryfx(
+ $this->conn,
+ 'UPDATE %T SET authorPHID = %s WHERE %Q',
+ PhabricatorRepository::TABLE_LINTMESSAGE,
+ $author,
+ implode(' OR ', $where));
+ }
+
+ $this->conn->saveTransaction();
+ }
+ }
+*/
+
+ public function getArtifactInputs() {
+ return array(
+ array(
+ 'name' => pht('Run on Host'),
+ 'key' => $this->getSetting('hostartifact'),
+ 'type' => HarbormasterBuildArtifact::TYPE_HOST,
+ ),
+ );
+ }
+
+ public function getFieldSpecifications() {
+ return array(
+ 'severity' => array(
+ 'name' => pht('Minimum Severity'),
+ 'type' => 'text',
+ 'required' => true,
+ 'default' => 'advice'
+ ),
+ 'arcanist' => array(
+ 'name' => pht('Arcanist Path'),
+ 'type' => 'text',
+ 'required' => true,
+ 'default' => 'arc'
+ ),
+ 'path' => array(
+ 'name' => pht('Project Path'),
+ 'type' => 'text',
+ 'required' => true
+ ),
+ 'hostartifact' => array(
+ 'name' => pht('Host'),
+ 'type' => 'text',
+ 'required' => true,
+ ),
+ 'arcrcpath' => array(
+ 'name' => pht('.arcrc Path'),
+ 'caption' => pht('Specify the .arcrc path on Windows'),
+ 'type' => 'text'
+ ),
+ 'lintall' => array(
+ 'name' => pht('Lint Entire Repository'),
+ 'type' => 'bool'
+ ),
+ );
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Mon, Mar 24, 12:23 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7707850
Default Alt Text
D9113.id21651.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility