diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7ab5a448..7fbff131 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -42,6 +42,7 @@ phutil_register_library_map(array( 'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'lint/linter/xhpast/rules/__tests__/ArcanistBraceFormattingXHPASTLinterRuleTestCase.php', 'ArcanistBranchWorkflow' => 'workflow/ArcanistBranchWorkflow.php', 'ArcanistBrowseWorkflow' => 'workflow/ArcanistBrowseWorkflow.php', + 'ArcanistBuildRef' => 'ref/ArcanistBuildRef.php', 'ArcanistBundle' => 'parser/ArcanistBundle.php', 'ArcanistBundleTestCase' => 'parser/__tests__/ArcanistBundleTestCase.php', 'ArcanistCSSLintLinter' => 'lint/linter/ArcanistCSSLintLinter.php', @@ -459,6 +460,7 @@ phutil_register_library_map(array( 'ArcanistBraceFormattingXHPASTLinterRuleTestCase' => 'ArcanistXHPASTLinterRuleTestCase', 'ArcanistBranchWorkflow' => 'ArcanistFeatureWorkflow', 'ArcanistBrowseWorkflow' => 'ArcanistWorkflow', + 'ArcanistBuildRef' => 'Phobject', 'ArcanistBundle' => 'Phobject', 'ArcanistBundleTestCase' => 'PhutilTestCase', 'ArcanistCSSLintLinter' => 'ArcanistExternalLinter', diff --git a/src/ref/ArcanistBuildRef.php b/src/ref/ArcanistBuildRef.php new file mode 100644 index 00000000..a173e692 --- /dev/null +++ b/src/ref/ArcanistBuildRef.php @@ -0,0 +1,115 @@ +parameters = $data; + return $ref; + } + + private function getStatusMap() { + // The modern "harbormaster.build.search" API method returns this in the + // "fields" list; the older API method returns it at the root level. + if (isset($this->parameters['fields']['buildStatus'])) { + $status = $this->parameters['fields']['buildStatus']; + } else if (isset($this->parameters['buildStatus'])) { + $status = $this->parameters['buildStatus']; + } else { + $status = 'unknown'; + } + + // We may either have an array or a scalar here. The array comes from + // "harbormaster.build.search", or from "harbormaster.querybuilds" if + // the server is newer than August 2016. The scalar comes from older + // versions of that method. See PHI261. + if (is_array($status)) { + $map = $status; + } else { + $map = array( + 'value' => $status, + ); + } + + // If we don't have a name, try to fill one in. + if (!isset($map['name'])) { + $name_map = array( + 'inactive' => pht('Inactive'), + 'pending' => pht('Pending'), + 'building' => pht('Building'), + 'passed' => pht('Passed'), + 'failed' => pht('Failed'), + 'aborted' => pht('Aborted'), + 'error' => pht('Error'), + 'paused' => pht('Paused'), + 'deadlocked' => pht('Deadlocked'), + 'unknown' => pht('Unknown'), + ); + + $map['name'] = idx($name_map, $map['value'], $map['value']); + } + + // If we don't have an ANSI color code, try to fill one in. + if (!isset($map['color.ansi'])) { + $color_map = array( + 'failed' => 'red', + 'passed' => 'green', + ); + + $map['color.ansi'] = idx($color_map, $map['value'], 'yellow'); + } + + return $map; + } + + public function getID() { + return $this->parameters['id']; + } + + public function getName() { + if (isset($this->parameters['fields']['name'])) { + return $this->parameters['fields']['name']; + } + + return $this->parameters['name']; + } + + public function getStatus() { + $map = $this->getStatusMap(); + return $map['value']; + } + + public function getStatusName() { + $map = $this->getStatusMap(); + return $map['name']; + } + + public function getStatusANSIColor() { + $map = $this->getStatusMap(); + return $map['color.ansi']; + } + + public function getObjectName() { + return pht('Build %d', $this->getID()); + } + + public function getStatusSortVector() { + $status = $this->getStatus(); + + // For now, just sort passed builds first. + if ($this->getStatus() == 'passed') { + $status_class = 1; + } else { + $status_class = 2; + } + + return id(new PhutilSortVector()) + ->addInt($status_class) + ->addString($status); + } + + +} diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php index d1406096..bb60f71e 100644 --- a/src/repository/api/ArcanistGitAPI.php +++ b/src/repository/api/ArcanistGitAPI.php @@ -52,8 +52,12 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { } public function getGitVersion() { - list($stdout) = $this->execxLocal('--version'); - return rtrim(str_replace('git version ', '', $stdout)); + static $version = null; + if ($version === null) { + list($stdout) = $this->execxLocal('--version'); + $version = rtrim(str_replace('git version ', '', $stdout)); + } + return $version; } public function getMetadataPath() { @@ -645,8 +649,70 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { return $this->executeSVNFindRev($hash, 'SVN'); } + private function buildUncommittedStatusViaStatus() { + $status = $this->buildLocalFuture( + array( + 'status --porcelain=2 -z', + )); + list($stdout) = $status->resolvex(); + + $result = new PhutilArrayWithDefaultValue(); + $parts = explode("\0", $stdout); + while (count($parts) > 1) { + $entry = array_shift($parts); + $entry_parts = explode(' ', $entry); + if ($entry_parts[0] == '1') { + $path = $entry_parts[8]; + } else if ($entry_parts[0] == '2') { + $path = $entry_parts[9]; + } else if ($entry_parts[0] == 'u') { + $path = $entry_parts[10]; + } else if ($entry_parts[0] == '?') { + $result[$entry_parts[1]] = self::FLAG_UNTRACKED; + continue; + } + + $result[$path] |= self::FLAG_UNCOMMITTED; + $index_state = substr($entry_parts[1], 0, 1); + $working_state = substr($entry_parts[1], 1, 1); + if ($index_state == 'A') { + $result[$path] |= self::FLAG_ADDED; + } else if ($index_state == 'M') { + $result[$path] |= self::FLAG_MODIFIED; + } else if ($index_state == 'D') { + $result[$path] |= self::FLAG_DELETED; + } + if ($working_state != '.') { + $result[$path] |= self::FLAG_UNSTAGED; + if ($index_state == '.') { + if ($working_state == 'A') { + $result[$path] |= self::FLAG_ADDED; + } else if ($working_state == 'M') { + $result[$path] |= self::FLAG_MODIFIED; + } else if ($working_state == 'D') { + $result[$path] |= self::FLAG_DELETED; + } + } + } + $submodule_tracked = substr($entry_parts[2], 2, 1); + $submodule_untracked = substr($entry_parts[2], 3, 1); + if ($submodule_tracked == 'M' || $submodule_untracked == 'U') { + $result[$path] |= self::FLAG_EXTERNALS; + } + + if ($entry_parts[0] == '2') { + $result[array_shift($parts)] = $result[$path] | self::FLAG_DELETED; + $result[$path] |= self::FLAG_ADDED; + } + } + return $result->toArray(); + } protected function buildUncommittedStatus() { + if (version_compare($this->getGitVersion(), '2.11.0', '>=')) { + return $this->buildUncommittedStatusViaStatus(); + } + $diff_options = $this->getDiffBaseOptions(); if ($this->repositoryHasNoCommits) { @@ -719,7 +785,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI { protected function buildCommitRangeStatus() { list($stdout, $stderr) = $this->execxLocal( - 'diff %C --raw %s --', + 'diff %C --raw %s HEAD --', $this->getDiffBaseOptions(), $this->getBaseCommit()); diff --git a/src/repository/api/ArcanistMercurialAPI.php b/src/repository/api/ArcanistMercurialAPI.php index 5fe14327..d6d10a9e 100644 --- a/src/repository/api/ArcanistMercurialAPI.php +++ b/src/repository/api/ArcanistMercurialAPI.php @@ -372,41 +372,18 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI { } protected function buildCommitRangeStatus() { - // TODO: Possibly we should use "hg status --rev X --rev ." for this - // instead, but we must run "hg diff" later anyway in most cases, so - // building and caching it shouldn't hurt us. + list($stdout) = $this->execxLocal( + 'status --rev %s --rev tip', + $this->getBaseCommit()); - $diff = $this->getFullMercurialDiff(); - if (!$diff) { - return array(); + $results = new PhutilArrayWithDefaultValue(); + + $working_status = ArcanistMercurialParser::parseMercurialStatus($stdout); + foreach ($working_status as $path => $mask) { + $results[$path] |= $mask; } - $parser = new ArcanistDiffParser(); - $changes = $parser->parseDiff($diff); - - $status_map = array(); - foreach ($changes as $change) { - $flags = 0; - switch ($change->getType()) { - case ArcanistDiffChangeType::TYPE_ADD: - case ArcanistDiffChangeType::TYPE_MOVE_HERE: - case ArcanistDiffChangeType::TYPE_COPY_HERE: - $flags |= self::FLAG_ADDED; - break; - case ArcanistDiffChangeType::TYPE_CHANGE: - case ArcanistDiffChangeType::TYPE_COPY_AWAY: // Check for changes? - $flags |= self::FLAG_MODIFIED; - break; - case ArcanistDiffChangeType::TYPE_DELETE: - case ArcanistDiffChangeType::TYPE_MOVE_AWAY: - case ArcanistDiffChangeType::TYPE_MULTICOPY: - $flags |= self::FLAG_DELETED; - break; - } - $status_map[$change->getCurrentPath()] = $flags; - } - - return $status_map; + return $results->toArray(); } protected function didReloadWorkingCopy() { diff --git a/src/repository/api/ArcanistSubversionAPI.php b/src/repository/api/ArcanistSubversionAPI.php index 887fa4f3..d8066e96 100644 --- a/src/repository/api/ArcanistSubversionAPI.php +++ b/src/repository/api/ArcanistSubversionAPI.php @@ -45,9 +45,9 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI { } protected function buildCommitRangeStatus() { - // In SVN, the commit range is always "uncommitted changes", so these - // statuses are equivalent. - return $this->getUncommittedStatus(); + // In SVN, there are never any previous commits in the range -- it is all in + // the uncommitted status. + return array(); } protected function buildUncommittedStatus() { diff --git a/src/repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php b/src/repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php index 689588d7..b6b4c596 100644 --- a/src/repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php +++ b/src/repository/api/__tests__/ArcanistRepositoryAPIStateTestCase.php @@ -5,6 +5,8 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase { public function testGitStateParsing() { if (Filesystem::binaryExists('git')) { $this->parseState('git_basic.git.tgz'); + $this->parseState('git_submodules_dirty.git.tgz'); + $this->parseState('git_submodules_staged.git.tgz'); } else { $this->assertSkipped(pht('Git is not installed')); } @@ -54,6 +56,16 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase { } private function assertCorrectState($test, ArcanistRepositoryAPI $api) { + if ($api instanceof ArcanistGitAPI) { + $version = $api->getGitVersion(); + if (version_compare($version, '2.11.0', '<')) { + // Behavior differs slightly on older versions of git; rather than code + // both variants, skip the tests in the presence of such a git. + $this->assertSkipped(pht('Behavior differs slightly on git < 2.11.0')); + return; + } + } + $f_mod = ArcanistRepositoryAPI::FLAG_MODIFIED; $f_add = ArcanistRepositoryAPI::FLAG_ADDED; $f_del = ArcanistRepositoryAPI::FLAG_DELETED; @@ -68,7 +80,7 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase { switch ($test) { case 'svn_basic.svn.tgz': - $expect = array( + $expect_uncommitted = array( 'ADDED' => $f_add, 'COPIED_TO' => $f_add, 'DELETED' => $f_del, @@ -78,8 +90,22 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase { 'PROPCHANGE' => $f_mod, 'UNTRACKED' => $f_unt, ); - $this->assertEqual($expect, $api->getUncommittedStatus()); - $this->assertEqual($expect, $api->getCommitRangeStatus()); + $this->assertEqual($expect_uncommitted, $api->getUncommittedStatus()); + + $expect_range = array(); + $this->assertEqual($expect_range, $api->getCommitRangeStatus()); + + $expect_working = array( + 'ADDED' => $f_add, + 'COPIED_TO' => $f_add, + 'DELETED' => $f_del, + 'MODIFIED' => $f_mod, + 'MOVED' => $f_del, + 'MOVED_TO' => $f_add, + 'PROPCHANGE' => $f_mod, + 'UNTRACKED' => $f_unt, + ); + $this->assertEqual($expect_working, $api->getWorkingCopyStatus()); break; case 'git_basic.git.tgz': $expect_uncommitted = array( @@ -94,10 +120,41 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase { 'ADDED' => $f_add, 'DELETED' => $f_del, 'MODIFIED' => $f_mod, - 'UNCOMMITTED' => $f_add, 'UNSTAGED' => $f_add, ); $this->assertEqual($expect_range, $api->getCommitRangeStatus()); + + $expect_working = array( + 'ADDED' => $f_add, + 'DELETED' => $f_del, + 'MODIFIED' => $f_mod, + 'UNCOMMITTED' => $f_add | $f_unc, + 'UNSTAGED' => $f_add | $f_mod | $f_uns | $f_unc, + 'UNTRACKED' => $f_unt, + ); + $this->assertEqual($expect_working, $api->getWorkingCopyStatus()); + break; + case 'git_submodules_dirty.git.tgz': + $expect_uncommitted = array( + '.gitmodules' => $f_mod | $f_uns | $f_unc, + 'added/' => $f_unt, + 'deleted' => $f_del | $f_uns | $f_unc, + 'modified-commit' => $f_mod | $f_uns | $f_unc, + 'modified-commit-dirty' => $f_ext | $f_mod | $f_uns | $f_unc, + 'modified-dirty' => $f_ext | $f_mod | $f_uns | $f_unc, + ); + $this->assertEqual($expect_uncommitted, $api->getUncommittedStatus()); + break; + case 'git_submodules_staged.git.tgz': + $expect_uncommitted = array( + '.gitmodules' => $f_mod | $f_unc, + 'added' => $f_add | $f_unc, + 'deleted' => $f_del | $f_unc, + 'modified-commit' => $f_mod | $f_unc, + 'modified-commit-dirty' => $f_ext | $f_mod | $f_uns | $f_unc, + 'modified-dirty' => $f_ext | $f_mod | $f_uns | $f_unc, + ); + $this->assertEqual($expect_uncommitted, $api->getUncommittedStatus()); break; case 'hg_basic.hg.tgz': $expect_uncommitted = array( @@ -113,6 +170,15 @@ final class ArcanistRepositoryAPIStateTestCase extends PhutilTestCase { 'UNCOMMITTED' => $f_add, ); $this->assertEqual($expect_range, $api->getCommitRangeStatus()); + + $expect_working = array( + 'ADDED' => $f_add, + 'DELETED' => $f_del, + 'MODIFIED' => $f_mod, + 'UNCOMMITTED' => $f_add | $f_mod | $f_unc, + 'UNTRACKED' => $f_unt, + ); + $this->assertEqual($expect_working, $api->getWorkingCopyStatus()); break; default: throw new Exception( diff --git a/src/repository/api/__tests__/state/git_submodules_dirty.git.tgz b/src/repository/api/__tests__/state/git_submodules_dirty.git.tgz new file mode 100644 index 00000000..60cf0551 Binary files /dev/null and b/src/repository/api/__tests__/state/git_submodules_dirty.git.tgz differ diff --git a/src/repository/api/__tests__/state/git_submodules_staged.git.tgz b/src/repository/api/__tests__/state/git_submodules_staged.git.tgz new file mode 100644 index 00000000..f90621d7 Binary files /dev/null and b/src/repository/api/__tests__/state/git_submodules_staged.git.tgz differ diff --git a/src/workflow/ArcanistLandWorkflow.php b/src/workflow/ArcanistLandWorkflow.php index ec46aa05..ada0168c 100644 --- a/src/workflow/ArcanistLandWorkflow.php +++ b/src/workflow/ArcanistLandWorkflow.php @@ -1404,13 +1404,12 @@ EOTEXT switch ($buildable['buildableStatus']) { case 'building': $message = pht( - 'Harbormaster is still building the active diff for this revision:'); + 'Harbormaster is still building the active diff for this revision.'); $prompt = pht('Land revision anyway, despite ongoing build?'); break; case 'failed': $message = pht( - 'Harbormaster failed to build the active diff for this revision. '. - 'Build failures:'); + 'Harbormaster failed to build the active diff for this revision.'); $prompt = pht('Land revision anyway, despite build failures?'); break; default: @@ -1418,28 +1417,25 @@ EOTEXT return; } - $builds = $this->getConduit()->callMethodSynchronous( - 'harbormaster.querybuilds', + $builds = $this->queryBuilds( array( 'buildablePHIDs' => array($buildable['phid']), )); $console->writeOut($message."\n\n"); - foreach ($builds['data'] as $build) { - switch ($build['buildStatus']) { - case 'failed': - $color = 'red'; - break; - default: - $color = 'yellow'; - break; - } - $console->writeOut( - " ** %s ** %s: %s\n", - phutil_utf8_strtoupper($build['buildStatusName']), - pht('Build %d', $build['id']), - $build['name']); + $builds = msort($builds, 'getStatusSortVector'); + foreach ($builds as $build) { + $ansi_color = $build->getStatusANSIColor(); + $status_name = $build->getStatusName(); + $object_name = $build->getObjectName(); + $build_name = $build->getName(); + + echo tsprintf( + " ** %s ** %s: %s\n", + $status_name, + $object_name, + $build_name); } $console->writeOut( @@ -1478,4 +1474,34 @@ EOTEXT $mark_workflow->run(); } + private function queryBuilds(array $constraints) { + $conduit = $this->getConduit(); + + // NOTE: This method only loads the 100 most recent builds. It's rare for + // a revision to have more builds than that and there's currently no paging + // wrapper for "*.search" Conduit API calls available in Arcanist. + + try { + $raw_result = $conduit->callMethodSynchronous( + 'harbormaster.build.search', + array( + 'constraints' => $constraints, + )); + } catch (Exception $ex) { + // If the server doesn't have "harbormaster.build.search" yet (Aug 2016), + // try the older "harbormaster.querybuilds" instead. + $raw_result = $conduit->callMethodSynchronous( + 'harbormaster.querybuilds', + $constraints); + } + + $refs = array(); + foreach ($raw_result['data'] as $raw_data) { + $refs[] = ArcanistBuildRef::newFromConduit($raw_data); + } + + return $refs; + } + + }