Requiring very recent Mercurial may also run into issues described here:

https://discourse.phabricator-community.org/t/prefer-old-mercurial-2-8-2-to-avoid-various-problems/3431

I don't think I ever fully triaged these, but my vague recollection is that Mercurial changed how parameters get passed over HTTP (headers became GET params, or vice versa, or something?), and I made it two or three patches down the rabbit hole and it didn't seem like I'd need to dig much deeper, but I never finished these changes up for whatever reason.

This specific issue is probably the relevant one, the other thing linked from there looks like it's just a --debug problem, which this patch should fix.

https://discourse.phabricator-community.org/t/error-cloning-mercurial-repository-http-after-first-install/1200/2

In D21679#275638, @epriestley wrote:

This specific issue is probably the relevant one, the other thing linked from there looks like it's just a --debug problem, which this patch should fix.

https://discourse.phabricator-community.org/t/error-cloning-mercurial-repository-http-after-first-install/1200/2

Oh I was looking into this post and details a little bit recently and have it in my notes to get to at some point. I might eventually poke around with this but it's bottom-of-my-list. We don't serve vcs over http but that's probably more common for publicly accessible repositories, or as a means to "simplify" CI integrations.

forgot to submit my comments~

Found one issue, which seems to fix the improper graph structure but now it's rendering disjointly

%C ("raw, unescaped comment")

err: raw, unescaped command rather

I'm not immediately sure how this patch could cause the disjoint rendering, miiiiiight be an unrelated display issue.

I setup mercurial to run version 4.4, created a new repository, added some commits, and verified all the above behavior still works properly.

This isn't quite true -- annotations don't appear to be showing up. I'll take a quick look

Hmm this is the error I'm getting when using mercurial 4.4

2021/07/08 22:35:31 [error] 9693#9693: *1 FastCGI sent in stderr: "

[2021-07-08 22:35:31] PHLOG: 'Using template: --debug --changeset' at [/usr/local/phacility/phabricator/src/applications/diffusion/query/blame/DiffusionMercurialBlameQuery.php:18]

[2021-07-08 22:35:31] EXCEPTION: (RuntimeException) min(): Array must contain at least one element at [<arcanist>/src/error/PhutilErrorHandler.php:262]
arcanist(head=origin-master, ref.master=2d6452acb5f1, ref.origin-master=c94c5bbf35f0), phabricator(head=master, ref.master=14c516b7a109, custom=10)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array)
             called at [<arcanist>/src/error/PhutilErrorHandler.php:262]
  #1 <#2> min(array)
             called at [<phabricator>/src/applications/diffusion/controller/DiffusionBlameController.php:201]
  #2 <#2> DiffusionBlameController::handleRequest(AphrontRequest)
             called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
  #3 phlog(RuntimeException)
             called at [<phabricator>/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php:27]
  #4 PhabricatorAjaxRequestExceptionHandler::handleRequestThrowable(AphrontRequest, RuntimeException)
             called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:751]
  #5 AphrontApplicationConfiguration::handleThrowable(RuntimeException)
             called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:296]
  #6 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl)
             called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:204]
  #7 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink)
             called at [<phabricator>/webroot/index.php:35]

" while reading response header from upstream, client: 10.0.50.250, server: repo-testing1.mimsoftware.com, request: "POST /diffusio

I tried adding this at the end of DiffusionMercurialBlameQuery::resolveBlameFuture()

if (empty($result)) {
  return null;
}

Because it looks like the caller is expecting to handle null, but that didn't seem to change any behavior.

Oh that error happens when resolveBlameFuture() returns null because there was a problem invoking the command... looking a little more into what issue is happening

Buh there must be something going on in the environment that's causing issues. I logged out the command it's running and tried running it directly in the repo

[cspeck@repo-testing1 85]$ hg --version
Mercurial Distributed SCM (version 4.4.2)
(see https://mercurial-scm.org for more information)

Copyright (C) 2005-2017 Matt Mackall and others
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[cspeck@repo-testing1 85]$ hg annotate --debug --changeset --rev d696cbf7fb0edb99a01cd6a1656cb55c495a52a1 -- test.txt
starting pager for command 'annotate'
d696cbf7fb0edb99a01cd6a1656cb55c495a52a1: bloop!
0477d6101efb6bc4d01332bc99765791bac5f673: blah

But when that command is being run from phabricator I phlog() the stdout and get (with exit code 255)

[2021-07-08 22:54:01] PHLOG: 'Error with blame: hg annotate [-r REV] [-f] [-a] [-u] [-d] [-n] [-c] [-l] FILE...

show changeset information by line for each file

options ([+] can be repeated):

 -r --rev REV             annotate the specified revision
    --no-follow           don't follow copies and renames
 -a --text                treat all files as text
 -u --user                list the author (long with -v)
 -f --file                list the filename
 -d --date                list the date (short with -q)
 -n --number              list the revision number (default)
 -c --changeset           list the changeset
 -l --line-number         show line number at the first appearance
 -w --ignore-all-space    ignore white space when comparing lines
 -b --ignore-space-change ignore changes in the amount of white space
 -B --ignore-blank-lines  ignore changes whose lines are all blank
 -Z --ignore-space-at-eol ignore changes in whitespace at EOL
 -I --include PATTERN [+] include names mat...

Phabricator appears to have identified and using the right binary

For figuring out the command it's running I'm effectively recreating the command-line -- is there an easy way to log the command that will run from the CommandFuture?

is there an easy way to log the command that will run from the CommandFuture?

For normal web requests this should appear in DarkConsole if you enable it (in the Services tab), or you can use bin/conduit call --trace ... if it's happening in Conduit.

Best guess is that it's maaaaaaybe because we pass HGPLAIN=1 in the environment (DiffusionMercurialCommandEngine) and your version of Mercurial has annotate set up as an extension? That seems like a big stretch, but wouldn't show up. Otherwise, you can maybe phlog() from DiffusionMercurialCommandEngine->newFormattedCommand().

I added some logs and then removed them but functionality nothing else should have changed (except the --template quoting I mention) and now when running hg 4.4 it no longer errors, but it the blame doesn't list changesets (but does put color next to the lines which I think is just a default if nothing was found?)

Best guess is that it's maaaaaaybe because we pass HGPLAIN=1 in the environment (DiffusionMercurialCommandEngine) and your version of Mercurial has annotate set up as an extension? That seems like a big stretch, but wouldn't show up.

I don't believe annotate is setup as an extension. This is our global .hgrc

# Ansible managed

[extensions]
#largefiles =
[largefiles]
usercache = /opt/hgrepos/.global-largefiles-store
[trusted]
users = phab-phd

Supposedly this will list the active extensions (running while hg 4.4)

$ hg debugextensions
$ 

Here's something interesting

$ hg annotate --debug --changeset --rev 0 -- test.txt
starting pager for command 'annotate'
0477d6101efb6bc4d01332bc99765791bac5f673: blah
$ HGPLAIN=1 hg annotate --debug --changeset --rev 0 -- test.txt
0477d6101efb6bc4d01332bc99765791bac5f673: blah

apparently setting HGPLAIN caused some of that extraneous log output to disappear x_x

I think I got something messed up and I'm not seeing it. I'm making changes which no longer seem to be taking effect (I'm restarting the services)

I'll take another look tomorrow. Somehow I managed to get the code to emit output and found the issue with strpos() but then after fixing that it now seems busted again for both versions of mercurial. No exceptions are happening but it's not actually building up the annotation data properly (which is just the ordered list of the commit hashes I think?)

Is it possible the annotate results are cached somewhere and might be causing some of the inconsistency I’m seeing?

Ah, yes; DiffusionBlameQuery has a cache.

I've been incorporating ./bin/cache purge --all into my test process and things are making a lot more sense now. I'll hopefully have some changes up here shortly. There's one minor bug with annotations using mercurial 4.2 for file lines that contain : not being annotated but it works on 5.8.

There's one minor bug with annotations using mercurial 4.2 for file lines that contain : not being annotated but it works on 5.8.

This was because I had made modifications to the resolveBlameFuture() in my testing. I undid these changes and things are working again. I tested both mercurial 5.8 and 4.4, purging all caches in between and verifying that annotations and history appear correctly, including annotations of files with code lines including ':'.

To sum up what I think the issues I ran into last night were

1. I was not purging caches when switching versions of mercurial or when making changes to DiffusionMercurialBlameQuery
2. I had improperly inverted strpos(':', $line) instead of strpos($line, ':')
3. When using --template I think I was not properly escaping quotes for how they are eventually invoked when launching the process

Both #2 and #3 are fixed and tested now