Page MenuHomePhabricator
Files F15410860

D9113.diff
No OneTemporary
Actions

D9113.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
@@ -709,6 +709,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',
'HarbormasterManagePlansCapability' => 'applications/harbormaster/capability/HarbormasterManagePlansCapability.php',
'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php',
'HarbormasterManagementUpdateWorkflow' => 'applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php',
@@ -3477,6 +3478,7 @@
'HarbormasterDAO' => 'PhabricatorLiskDAO',
'HarbormasterHTTPRequestBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterLeaseHostBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
+ 'HarbormasterLintSaveBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
'HarbormasterManagePlansCapability' => 'PhabricatorPolicyCapability',
'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow',
'HarbormasterManagementUpdateWorkflow' => 'HarbormasterManagementWorkflow',
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,367 @@
+<?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();
+ $variables = $build_target->getVariables();
+
+ $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();
+ }
+
+ $build_path = $command = $this->mergeVariables(
+ 'vcsprintf',
+ $settings['path'],
+ $variables);
+
+ // 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($build_path, '&') > 0) {
+ $resolve_log->append(pht('Unsafe Windows path detected')."\n");
+ $resolve_log->finalize($resolve_start);
+ return;
+ }
+
+ // Remove quotes that csprintf may have added
+ $build_path = str_replace("'", '', $build_path);
+
+ $prefix = '';
+ $is_absolute_path =
+ strlen($build_path) > 2 &&
+ $build_path[1] == ':' &&
+ $build_path[2] == '\\';
+ if ($is_absolute_path) {
+ // Windows is terrible! It maintains 26 working directories, so just
+ // issuing "cd Z:\the\path\i\want" won't work if the current working
+ // directory is C:\something. In order to change directory to a
+ // different drive, you have to issue a non-command like "Z:" first.
+ // To do this, we take the drive letter and colon from the path when
+ // the path is absolute.
+ $prefix = $build_path[0].$build_path[1].' & ';
+ }
+
+ $change_directory = csprintf('%Ccd %C &', $prefix, $build_path);
+ } else {
+ $change_directory = csprintf('cd %s &&', $build_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 (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();
+
+ $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();
+ }
+
+ 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
Thu, Mar 20, 8:35 AM (1 h, 8 m ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7700509
Default Alt Text
D9113.diff (13 KB)

Event Timeline

Phabricator · Built by Phacility