1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-26 00:32:41 +01:00

Merge branch "master" into "experimental".

This commit is contained in:
epriestley 2018-01-16 13:58:09 -08:00
commit e64cbd3ba7
18 changed files with 465 additions and 88 deletions

View file

@ -51,6 +51,7 @@ phutil_register_library_map(array(
'ArcanistBrowseURIHardpointLoader' => 'browse/loader/ArcanistBrowseURIHardpointLoader.php',
'ArcanistBrowseURIRef' => 'browse/ref/ArcanistBrowseURIRef.php',
'ArcanistBrowseWorkflow' => 'browse/workflow/ArcanistBrowseWorkflow.php',
'ArcanistBuildRef' => 'ref/ArcanistBuildRef.php',
'ArcanistBundle' => 'parser/ArcanistBundle.php',
'ArcanistBundleTestCase' => 'parser/__tests__/ArcanistBundleTestCase.php',
'ArcanistCSSLintLinter' => 'lint/linter/ArcanistCSSLintLinter.php',
@ -497,6 +498,7 @@ phutil_register_library_map(array(
'ArcanistBrowseURIHardpointLoader' => 'ArcanistHardpointLoader',
'ArcanistBrowseURIRef' => 'ArcanistRef',
'ArcanistBrowseWorkflow' => 'ArcanistWorkflow',
'ArcanistBuildRef' => 'Phobject',
'ArcanistBundle' => 'Phobject',
'ArcanistBundleTestCase' => 'PhutilTestCase',
'ArcanistCSSLintLinter' => 'ArcanistExternalLinter',

View file

@ -167,19 +167,17 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
$old_impact--;
$new_impact--;
if ($old_impact < 0 || $new_impact < 0) {
throw new Exception(
pht(
'Modified prefix line range has become negative '.
'(old = %d, new = %d).',
$old_impact,
$new_impact));
// We can end up here if a patch removes a line which occurs before
// another identical line.
if ($old_impact <= 0 || $new_impact <= 0) {
break;
}
} while (true);
// If the lines at the end of the changed line range are actually the
// same, shrink the range. This happens when a patch just removes a
// line.
if ($old_impact > 0 && $new_impact > 0) {
do {
$old_suffix = idx($old_lines, $start + $old_impact - 2, null);
$new_suffix = idx($new_lines, $start + $new_impact - 2, null);
@ -197,6 +195,7 @@ final class ArcanistConsoleLintRenderer extends ArcanistLintRenderer {
break;
}
} while (true);
}
} else {

View file

@ -142,6 +142,20 @@ EOTEXT;
'replacement' => "\nX\nY\n",
),
'rmmulti' => array(
'line' => 2,
'char' => 1,
'original' => "\n",
'replacement' => '',
),
'rmmulti2' => array(
'line' => 1,
'char' => 2,
'original' => "\n",
'replacement' => '',
),
);
$defaults = array(

View file

@ -0,0 +1,10 @@
>>> Lint for path/to/example.c:
Warning (WARN123) Lint Warning
Consider this.
1 A
2 ~
>>> - 3 ~
4 B

View file

@ -0,0 +1,4 @@
A
B

View file

@ -0,0 +1,10 @@
>>> Lint for path/to/example.c:
Warning (WARN123) Lint Warning
Consider this.
1 A
>>> - 2 ~
3 ~
4 B

View file

@ -0,0 +1,4 @@
A
B

View file

@ -424,6 +424,9 @@ final class ArcanistBundle extends Phobject {
$cur_target = 'b/'.$cur_path;
}
$old_target = $this->encodeGitTargetPath($old_target);
$cur_target = $this->encodeGitTargetPath($cur_target);
$result[] = "diff --git {$old_index} {$cur_index}".$eol;
if ($type == ArcanistDiffChangeType::TYPE_ADD) {
@ -591,6 +594,24 @@ final class ArcanistBundle extends Phobject {
return $results;
}
private function encodeGitTargetPath($path) {
// See T8768. If a target path contains spaces, it must be terminated with
// a tab. If we don't do this, Mercurial has the wrong behavior when
// applying the patch. This results in a semantic trailing whitespace
// character:
//
// +++ b/X Y.txt\t
//
// Everyone is at fault here and there are no winners.
if (strpos($path, ' ') !== false) {
$path = $path."\t";
}
return $path;
}
private function getOldPath(ArcanistDiffChange $change) {
$old_path = $change->getOldPath();
$type = $change->getType();

View file

@ -33,6 +33,37 @@ final class ArcanistBundleTestCase extends PhutilTestCase {
return ArcanistBundle::newFromDiff($diff);
}
public function testTabEncoding() {
// See T8768. Test that we add semantic trailing tab literals to diffs
// touching files with spaces in them. This is a pain to encode using the
// support toolset here so just do it manually.
// Note that the "b/X Y.txt" line has a trailing tab literal.
$diff = <<<EODIFF
diff --git a/X Y.txt b/X Y.txt
new file mode 100644
--- /dev/null
+++ b/X Y.txt\t
@@ -0,0 +1 @@
+quack
EODIFF;
$bundle = ArcanistBundle::newFromDiff($diff);
$changes = $bundle->getChanges();
$this->assertEqual(1, count($changes));
// The path should parse as "X Y.txt" despite the trailing tab.
$change = head($changes);
$this->assertEqual('X Y.txt', $change->getCurrentPath());
// The tab should be restored when the diff is output again.
$this->assertEqual($diff, $bundle->toGitPatch());
}
/**
* Unarchive a saved git repository and apply each commit as though via
* "arc patch", verifying that the resulting tree hash is identical to the

View file

@ -0,0 +1,115 @@
<?php
final class ArcanistBuildRef
extends Phobject {
private $parameters;
public static function newFromConduit(array $data) {
$ref = new self();
$ref->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);
}
}

View file

@ -52,8 +52,12 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
}
public function getGitVersion() {
static $version = null;
if ($version === null) {
list($stdout) = $this->execxLocal('--version');
return rtrim(str_replace('git version ', '', $stdout));
$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());

View file

@ -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() {

View file

@ -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() {

View file

@ -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(

View file

@ -694,6 +694,8 @@ EOTEXT
$rev_title = $this->revision['title'];
$rev_auxiliary = idx($this->revision, 'auxiliary', array());
$full_name = pht('D%d: %s', $rev_id, $rev_title);
if ($this->revision['authorPHID'] != $this->getUserPHID()) {
$other_author = $this->getConduit()->callMethodSynchronous(
'user.query',
@ -706,17 +708,46 @@ EOTEXT
"This %s has revision '%s' but you are not the author. Land this ".
"revision by %s?",
$this->branchType,
"D{$rev_id}: {$rev_title}",
$full_name,
$other_author));
if (!$ok) {
throw new ArcanistUserAbortException();
}
}
if ($rev_status != ArcanistDifferentialRevisionStatus::ACCEPTED) {
$ok = phutil_console_confirm(pht(
"Revision '%s' has not been accepted. Continue anyway?",
"D{$rev_id}: {$rev_title}"));
$state_warning = null;
$state_header = null;
if ($rev_status == ArcanistDifferentialRevisionStatus::CHANGES_PLANNED) {
$state_header = pht('REVISION HAS CHANGES PLANNED');
$state_warning = pht(
'The revision you are landing ("%s") is currently in the "%s" state, '.
'indicating that you expect to revise it before moving forward.'.
"\n\n".
'Normally, you should resubmit it for review and wait until it is '.
'"%s" by reviewers before you continue.'.
"\n\n".
'To resubmit the revision for review, either: update the revision '.
'with revised changes; or use "Request Review" from the web interface.',
$full_name,
pht('Changes Planned'),
pht('Accepted'));
} else if ($rev_status != ArcanistDifferentialRevisionStatus::ACCEPTED) {
$state_header = pht('REVISION HAS NOT BEEN ACCEPTED');
$state_warning = pht(
'The revision you are landing ("%s") has not been "%s" by reviewers.',
$full_name,
pht('Accepted'));
}
if ($state_warning !== null) {
$prompt = pht('Land revision in the wrong state?');
id(new PhutilConsoleBlock())
->addParagraph(tsprintf('<bg:yellow>** %s **</bg>', $state_header))
->addParagraph(tsprintf('%B', $state_warning))
->draw();
$ok = phutil_console_confirm($prompt);
if (!$ok) {
throw new ArcanistUserAbortException();
}
@ -1373,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:
@ -1387,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(
" **<bg:".$color."> %s </bg>** %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(
" **<bg:".$ansi_color."> %s </bg>** %s: %s\n",
$status_name,
$object_name,
$build_name);
}
$console->writeOut(
@ -1447,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;
}
}

View file

@ -331,7 +331,8 @@ EOTEXT
pht('Automatically amending HEAD with lint patches.'));
$amend = true;
} else {
$amend = phutil_console_confirm(pht('Amend HEAD with lint patches?'));
$amend = phutil_console_confirm(pht('Amend HEAD with lint patches?'),
false);
}
if ($amend) {