From b0f7e7a6af2db93619b3ddde1f91598054198830 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 17 Dec 2013 18:04:01 -0800 Subject: [PATCH] Work around `hg` echoing warnings to stdout under --debug Summary: Ref T615. Ref T4237. With `--debug`, Mercurial will echo an "ignoring untrusted configuration option" warning **to stdout** if `.hgrc` has the wrong owner. However, we need `--debug` to make `{parents}` usable, at least until the patches I got into the upstream are widely deployed. So after getting `--debug` output, strip off any leading warnings. These warnings should always be in English, at least, since we set `LANG` explicitly. Test Plan: Unit tests. @asherkin, maybe you can confirm this? I can't actually get the warning, but I think my `hg` in PATH is just a bit out of date. Reviewers: asherkin, btrahan Reviewed By: asherkin CC: asherkin, aran Maniphest Tasks: T615, T4237 Differential Revision: https://secure.phabricator.com/D7784 --- ...nduitAPI_diffusion_historyquery_Method.php | 1 + ...iffusionLowLevelMercurialBranchesQuery.php | 1 + .../DiffusionMercurialCommitParentsQuery.php | 1 + .../storage/PhabricatorRepository.php | 23 +++++++++ .../PhabricatorRepositoryTestCase.php | 47 +++++++++++++++++++ 5 files changed, 73 insertions(+) diff --git a/src/applications/diffusion/conduit/ConduitAPI_diffusion_historyquery_Method.php b/src/applications/diffusion/conduit/ConduitAPI_diffusion_historyquery_Method.php index d6c6aab39e..3d552d0725 100644 --- a/src/applications/diffusion/conduit/ConduitAPI_diffusion_historyquery_Method.php +++ b/src/applications/diffusion/conduit/ConduitAPI_diffusion_historyquery_Method.php @@ -128,6 +128,7 @@ extends ConduitAPI_diffusion_abstractquery_Method { hgsprintf('reverse(%s::%s)', '0', $commit_hash), $path_arg); + $stdout = PhabricatorRepository::filterMercurialDebugOutput($stdout); $lines = explode("\n", trim($stdout)); $lines = array_slice($lines, $offset); diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php index cd1cbcd720..602b2b6123 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelMercurialBranchesQuery.php @@ -12,6 +12,7 @@ final class DiffusionLowLevelMercurialBranchesQuery // NOTE: `--debug` gives us 40-character hashes. list($stdout) = $repository->execxLocalCommand( '--debug branches'); + $stdout = PhabricatorRepository::filterMercurialDebugOutput($stdout); $branches = array(); diff --git a/src/applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php b/src/applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php index ec19f9cbce..dc9f900ce8 100644 --- a/src/applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php +++ b/src/applications/diffusion/query/parents/DiffusionMercurialCommitParentsQuery.php @@ -10,6 +10,7 @@ final class DiffusionMercurialCommitParentsQuery list($stdout) = $repository->execxLocalCommand( 'log --debug --limit 1 --template={parents} --rev %s', $drequest->getStableCommitName()); + $stdout = PhabricatorRepository::filterMercurialDebugOutput($stdout); $hashes = preg_split('/\s+/', trim($stdout)); foreach ($hashes as $key => $value) { diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 2ce03840a0..64ce761e72 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -440,6 +440,29 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return $args; } + /** + * Sanitize output of an `hg` command invoked with the `--debug` flag to make + * it usable. + * + * @param string Output from `hg --debug ...` + * @return string Usable output. + */ + public static function filterMercurialDebugOutput($stdout) { + // When hg commands are run with `--debug` and some config file isn't + // trusted, Mercurial prints out a warning to stdout, twice, after Feb 2011. + // + // http://selenic.com/pipermail/mercurial-devel/2011-February/028541.html + + $lines = preg_split('/(?<=\n)/', $stdout); + $regex = '/ignoring untrusted configuration option .*\n$/'; + + foreach ($lines as $key => $line) { + $lines[$key] = preg_replace($regex, '', $line); + } + + return implode('', $lines); + } + public function getURI() { return '/diffusion/'.$this->getCallsign().'/'; } diff --git a/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php b/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php index 69f18f454a..5b60b2968e 100644 --- a/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php +++ b/src/applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php @@ -107,5 +107,52 @@ final class PhabricatorRepositoryTestCase } + 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 = PhabricatorRepository::filterMercurialDebugOutput($input); + $this->assertEqual($expect, $actual, $input); + } + } }