Page MenuHomePhabricator
Files F14216146

D21679.id51621.diff
No OneTemporary
Actions

D21679.id51621.diff
View Options

diff --git a/src/applications/config/check/PhabricatorBinariesSetupCheck.php b/src/applications/config/check/PhabricatorBinariesSetupCheck.php
--- a/src/applications/config/check/PhabricatorBinariesSetupCheck.php
+++ b/src/applications/config/check/PhabricatorBinariesSetupCheck.php
@@ -120,17 +120,11 @@
break;
case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL:
$bad_versions = array(
- // We need 1.9 for HTTP cloning, see T3046.
- '< 1.9' => pht(
- 'The minimum supported version of Mercurial is 1.9, which was '.
- 'released in 2011.'),
- '= 2.1' => pht(
- 'This version of Mercurial returns a bad exit code '.
- 'after a successful pull.'),
- '= 2.2' => pht(
- 'This version of Mercurial has a significant memory leak, fixed '.
- 'in 2.2.1. Pushing fails with this version as well; see %s.',
- 'T3046#54922'),
+ // We need 2.4 for utilizing `{p1node}` keyword in templates, see
+ // D21679 and D21681.
+ '< 2.4' => pht(
+ 'The minimum supported version of Mercurial is 2.4, which was '.
+ 'released in 2012.'),
);
break;
}
diff --git a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php
--- a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php
+++ b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php
@@ -136,28 +136,33 @@
// stop history (this is more consistent with the Mercurial worldview of
// branches).
+ $path_args = array();
if (strlen($path)) {
- $path_arg = csprintf('%s', $path);
+ $path_args[] = $path;
$revset_arg = hgsprintf(
'reverse(ancestors(%s))',
$commit_hash);
} else {
- $path_arg = '';
$revset_arg = hgsprintf(
'reverse(ancestors(%s)) and branch(%s)',
$drequest->getBranch(),
$commit_hash);
}
+ $hg_analyzer = PhutilBinaryAnalyzer::getForBinary('hg');
+ if ($hg_analyzer->isMercurialTemplatePnodeAvailable()) {
+ $hg_log_template = '{node} {p1.node} {p2.node}\\n';
+ } else {
+ $hg_log_template = '{node} {p1node} {p2node}\\n';
+ }
+
list($stdout) = $repository->execxLocalCommand(
- 'log --debug --template %s --limit %d --rev %s -- %C',
- '{node};{parents}\\n',
+ 'log --template %s --limit %d --rev %s -- %Ls',
+ $hg_log_template,
($offset + $limit), // No '--skip' in Mercurial.
$revset_arg,
- $path_arg);
+ $path_args);
- $stdout = DiffusionMercurialCommandEngine::filterMercurialDebugOutput(
- $stdout);
$lines = explode("\n", trim($stdout));
$lines = array_slice($lines, $offset);
@@ -166,37 +171,19 @@
$last = null;
foreach (array_reverse($lines) as $line) {
- // In the event additional log output is included in future mercurial
- // updates, if the line does not contain any semi-colon then log it and
- // ignore it.
- if (strpos($line, ';') === false) {
- phlog(pht(
- 'Unexpected output from mercurial "log --debug" command: %s',
- $line));
- continue;
- }
- list($hash, $parents) = explode(';', $line);
- $parents = trim($parents);
- if (!$parents) {
- if ($last === null) {
- $parent_map[$hash] = array('...');
- } else {
- $parent_map[$hash] = array($last);
- }
- } else {
- $parents = preg_split('/\s+/', $parents);
- foreach ($parents as $parent) {
- list($plocal, $phash) = explode(':', $parent);
- if (!preg_match('/^0+$/', $phash)) {
- $parent_map[$hash][] = $phash;
- }
- }
- // This may happen for the zeroth commit in repository, both hashes
- // are "000000000...".
- if (empty($parent_map[$hash])) {
- $parent_map[$hash] = array('...');
+ $parts = explode(' ', trim($line));
+ $hash = $parts[0];
+ $parents = array_slice($parts, 1, 2);
+ foreach ($parents as $parent) {
+ if (!preg_match('/^0+\z/', $parent)) {
+ $parent_map[$hash][] = $parent;
}
}
+ // This may happen for the zeroth commit in repository, both hashes
+ // are "000000000...".
+ if (empty($parent_map[$hash])) {
+ $parent_map[$hash] = array('...');
+ }
// The rendering code expects the first commit to be "mainline", like
// Git. Flip the order so it does the right thing.
diff --git a/src/applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php b/src/applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php
--- a/src/applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php
+++ b/src/applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php
@@ -3,6 +3,53 @@
final class DiffusionMercurialCommandEngineTests extends PhabricatorTestCase {
public function testFilteringDebugOutput() {
+ $map = array(
+ '' => '',
+
+ "quack\n" => "quack\n",
+
+ "ignoring untrusted configuration option x.y = z\nquack\n" =>
+ "quack\n",
+
+ "ignoring untrusted configuration option x.y = z\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "quack\n" =>
+ "quack\n",
+
+ "ignoring untrusted configuration option x.y = z\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "quack\n" =>
+ "quack\n",
+
+ "quack\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "ignoring untrusted configuration option x.y = z\n" =>
+ "quack\n",
+
+ "ignoring untrusted configuration option x.y = z\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "duck\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "bread\n".
+ "ignoring untrusted configuration option x.y = z\n".
+ "quack\n" =>
+ "duck\nbread\nquack\n",
+
+ "ignoring untrusted configuration option x.y = z\n".
+ "duckignoring untrusted configuration option x.y = z\n".
+ "quack" =>
+ 'duckquack',
+ );
+
+ foreach ($map as $input => $expect) {
+ $actual = DiffusionMercurialCommandEngine::filterMercurialDebugOutput(
+ $input);
+ $this->assertEqual($expect, $actual, $input);
+ }
+
// Output that should be filtered out from the results
$output =
"ignoring untrusted configuration option\n".
diff --git a/src/applications/diffusion/query/blame/DiffusionMercurialBlameQuery.php b/src/applications/diffusion/query/blame/DiffusionMercurialBlameQuery.php
--- a/src/applications/diffusion/query/blame/DiffusionMercurialBlameQuery.php
+++ b/src/applications/diffusion/query/blame/DiffusionMercurialBlameQuery.php
@@ -6,11 +6,18 @@
$repository = $request->getRepository();
$commit = $request->getCommit();
- // NOTE: We're using "--debug" to make "--changeset" give us the full
- // commit hashes.
+ // NOTE: Using "--template" or "--debug" to get the full commit hashes.
+ $hg_analyzer = PhutilBinaryAnalyzer::getForBinary('hg');
+ if ($hg_analyzer->isMercurialAnnotateTemplatesAvailable()) {
+ // See `hg help annotate --verbose` for more info on this template format
+ $template = "--template {lines % '{node}: {line}'}";
+ } else {
+ $template = '--debug --changeset';
+ }
return $repository->getLocalCommandFuture(
- 'annotate --debug --changeset --rev %s -- %s',
+ 'annotate %s --rev %s -- %s',
+ $template,
$commit,
$path);
}
@@ -26,6 +33,21 @@
$lines = phutil_split_lines($stdout);
foreach ($lines as $line) {
+ // If the `--debug` flag was used above instead of `--template` then
+ // there's a good change additional output was included which is not
+ // relevant to the information we want. It should be safe to call this
+ // regardless of whether we used `--debug` or `--template` so there isn't
+ // a need to track which argument was used.
+ $line = DiffusionMercurialCommandEngine::filterMercurialDebugOutput(
+ $line);
+
+ // Just in case new versions of Mercurial add arbitrary output when using
+ // the `--debug`, do a quick sanity check that this line is formatted in
+ // a way we're expecting.
+ if (strpos(':', $line) === false) {
+ phlog(pht('Unexpected output from hg annotate: %s', $line));
+ continue;
+ }
list($commit) = explode(':', $line, 2);
$result[] = $commit;
}
diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
--- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
+++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php
@@ -47,23 +47,23 @@
private function loadMercurialParents() {
$repository = $this->getRepository();
+ $hg_analyzer = PhutilBinaryAnalyzer::getForBinary('hg');
+ if ($hg_analyzer->isMercurialTemplatePnodeAvailable()) {
+ $hg_log_template = '{p1.node} {p2.node}';
+ } else {
+ $hg_log_template = '{p1node} {p2node}';
+ }
+
list($stdout) = $repository->execxLocalCommand(
- 'log --debug --limit 1 --template={parents} --rev %s',
+ 'log --limit 1 --template %s --rev %s',
+ $hg_log_template,
$this->identifier);
- $stdout = DiffusionMercurialCommandEngine::filterMercurialDebugOutput(
- $stdout);
-
$hashes = preg_split('/\s+/', trim($stdout));
foreach ($hashes as $key => $value) {
- // Mercurial parents look like "23:ad9f769d6f786fad9f76d9a" -- we want
- // to strip out the local rev part.
- list($local, $global) = explode(':', $value);
- $hashes[$key] = $global;
-
- // With --debug we get 40-character hashes but also get the "000000..."
- // hash for missing parents; ignore it.
- if (preg_match('/^0+$/', $global)) {
+ // We get 40-character hashes but also get the "000000..." hash for
+ // missing parents; ignore it.
+ if (preg_match('/^0+\z/', $value)) {
unset($hashes[$key]);
}
}
diff --git a/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php b/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php
--- a/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php
+++ b/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php
@@ -100,55 +100,6 @@
}
- public function testFilterMercurialDebugOutput() {
- $map = array(
- '' => '',
-
- "quack\n" => "quack\n",
-
- "ignoring untrusted configuration option x.y = z\nquack\n" =>
- "quack\n",
-
- "ignoring untrusted configuration option x.y = z\n".
- "ignoring untrusted configuration option x.y = z\n".
- "quack\n" =>
- "quack\n",
-
- "ignoring untrusted configuration option x.y = z\n".
- "ignoring untrusted configuration option x.y = z\n".
- "ignoring untrusted configuration option x.y = z\n".
- "quack\n" =>
- "quack\n",
-
- "quack\n".
- "ignoring untrusted configuration option x.y = z\n".
- "ignoring untrusted configuration option x.y = z\n".
- "ignoring untrusted configuration option x.y = z\n" =>
- "quack\n",
-
- "ignoring untrusted configuration option x.y = z\n".
- "ignoring untrusted configuration option x.y = z\n".
- "duck\n".
- "ignoring untrusted configuration option x.y = z\n".
- "ignoring untrusted configuration option x.y = z\n".
- "bread\n".
- "ignoring untrusted configuration option x.y = z\n".
- "quack\n" =>
- "duck\nbread\nquack\n",
-
- "ignoring untrusted configuration option x.y = z\n".
- "duckignoring untrusted configuration option x.y = z\n".
- "quack" =>
- 'duckquack',
- );
-
- foreach ($map as $input => $expect) {
- $actual = DiffusionMercurialCommandEngine::filterMercurialDebugOutput(
- $input);
- $this->assertEqual($expect, $actual, $input);
- }
- }
-
public function testRepositoryShortNameValidation() {
$good = array(
'sensible-repository',

File Metadata

Mime Type
text/plain
Expires
Thu, Dec 12, 1:05 AM (14 h, 32 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6866521
Default Alt Text
D21679.id51621.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility