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 @@ +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' + ), + ); + } + +}