From 14c516b7a1097cab000951e3660491a1bfbb3211 Mon Sep 17 00:00:00 2001 From: Christopher Speck Date: Sun, 27 Jun 2021 22:30:11 -0400 Subject: [PATCH] Updating the filtering of Mercurial debug output Summary: With newer versions of Mercurial come newer debug messages which need filtered out. 1. In the scenario of Phabricator observing a hosted Mercurial repository which exists on a server in a multi-user environment it's possible that a repository computes branch cache at a tip revision which is not present. When this happens Mercurial will include in the debug output this information. This message indicates that the cache is going to be re-computed. See https://www.mercurial-scm.org/pipermail/mercurial/2014-June/047239.html. 2. Likely in some version with added or improved support for `pager` the debug info seems to indicate when a pager is being invoked for a command. This seems to print out regularly despite piping the stdout. 3. If the repository on Phabricator ever had the `largefiles` extension enabled then some additional details about "updated patterns" will print out. Test Plan: I verified an observed repository's history could be browsed, specifically the history of files which previously resulted in "Undefined offset: 1". Added a unit test to check the results of `filterMercurialDebugOutput()`. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D21677 --- src/__phutil_library_map__.php | 2 + .../DiffusionHistoryQueryConduitAPIMethod.php | 9 ++++ .../DiffusionMercurialCommandEngine.php | 24 +++++++++++ .../DiffusionMercurialCommandEngineTests.php | 42 +++++++++++++++++++ 4 files changed, 77 insertions(+) create mode 100644 src/applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 663c64fff9..2c5bb3341f 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -904,6 +904,7 @@ phutil_register_library_map(array( 'DiffusionLowLevelResolveRefsQuery' => 'applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php', 'DiffusionMercurialBlameQuery' => 'applications/diffusion/query/blame/DiffusionMercurialBlameQuery.php', 'DiffusionMercurialCommandEngine' => 'applications/diffusion/protocol/DiffusionMercurialCommandEngine.php', + 'DiffusionMercurialCommandEngineTests' => 'applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php', 'DiffusionMercurialFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionMercurialFileContentQuery.php', 'DiffusionMercurialFlagInjectionException' => 'applications/diffusion/exception/DiffusionMercurialFlagInjectionException.php', 'DiffusionMercurialRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php', @@ -7041,6 +7042,7 @@ phutil_register_library_map(array( 'DiffusionLowLevelResolveRefsQuery' => 'DiffusionLowLevelQuery', 'DiffusionMercurialBlameQuery' => 'DiffusionBlameQuery', 'DiffusionMercurialCommandEngine' => 'DiffusionCommandEngine', + 'DiffusionMercurialCommandEngineTests' => 'PhabricatorTestCase', 'DiffusionMercurialFileContentQuery' => 'DiffusionFileContentQuery', 'DiffusionMercurialFlagInjectionException' => 'Exception', 'DiffusionMercurialRawDiffQuery' => 'DiffusionRawDiffQuery', diff --git a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php index 8c53ff0555..5fa74d2e28 100644 --- a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php @@ -166,6 +166,15 @@ final class DiffusionHistoryQueryConduitAPIMethod $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) { diff --git a/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php b/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php index 03705ad49d..dc898f81d4 100644 --- a/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionMercurialCommandEngine.php @@ -71,12 +71,36 @@ final class DiffusionMercurialCommandEngine // // Separately, it may fail to write to a different branch cache, and may // encounter issues reading the branch cache. + // + // When Mercurial repositories are hosted on external systems with + // multi-user environments it's possible that the branch cache is computed + // on a revision which does not end up being published. When this happens it + // will recompute the cache but also print out "invalid branch cache". + // + // https://www.mercurial-scm.org/pipermail/mercurial/2014-June/047239.html + // + // When observing a repository which uses largefiles, the debug output may + // also contain extraneous output about largefile changes. + // + // At some point Mercurial added/improved support for pager used when + // command output is large. It includes printing out debug information that + // the pager is being started for a command. This seems to happen despite + // the output of the command being piped/read from another process. + // + // When printing color output Mercurial may run into some issue with the + // terminal info. This should never happen in Phabricator since color + // output should be turned off, however in the event it shows up we should + // filter it out anyways. $ignore = array( 'ignoring untrusted configuration option', "couldn't write revision branch cache:", "couldn't write branch cache:", 'invalid branchheads cache', + 'invalid branch cache', + 'updated patterns: .hglf', + 'starting pager for command', + 'no terminfo entry for', ); foreach ($ignore as $key => $pattern) { diff --git a/src/applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php b/src/applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php new file mode 100644 index 0000000000..e89125c5d9 --- /dev/null +++ b/src/applications/diffusion/protocol/__tests__/DiffusionMercurialCommandEngineTests.php @@ -0,0 +1,42 @@ +assertEqual('', $filtered_output); + + // The output that should make it through the filtering + $output = + "0b33a9e5ceedba14b03214f743957357d7bb46a9;694". + ":8b39f63eb209dd2bdfd4bd3d0721a9e38d75a6d3". + "-1:0000000000000000000000000000000000000000\n". + "8b39f63eb209dd2bdfd4bd3d0721a9e38d75a6d3;693". + ":165bce9ce4ccc97024ba19ed5a22f6a066fa6844". + "-1:0000000000000000000000000000000000000000\n". + "165bce9ce4ccc97024ba19ed5a22f6a066fa6844;692:". + "2337bc9e3cf212b3b386b5197801b1c81db64920". + "-1:0000000000000000000000000000000000000000\n"; + + $filtered_output = + DiffusionMercurialCommandEngine::filterMercurialDebugOutput($output); + + $this->assertEqual($output, $filtered_output); + } + +}