Issue 507: Wrong behaviour with mercurial 1.6 - Issue tracking, code review, source browser for git, mercurial, subversion and monotone.

Reported by Matías Halles, Jul 17, 2010

Hi,

i have a quite unusual setup of indefero but here i go, i might have 
found a bug that renders the system (SCM integration at least) 
useless...

Setup

+ Shared DreamHost account
+ Mercurial 1.6, user install
+ Running through fastCGI using a custom php.ini file
+ using SyncMercurial

After several tweaks (i will document in the correct place) i 
finally got indefero working correctly, managing the password files 
for http authentication, mercurial hgrc configuration, etc., but 
after all that i got stuck on the "source" tab. Indefero 
would try to update the commits log in database but it would try to 
insert empty data. After some tracing and tweaking and trying i 
finally was able to get the damn thing working, and i think i found 
two bugs caused by the usage of mercurial 1.6 rather than 1.5. I say 
this because in my computer i still use 1.5 and the commands issued 
by indefero do work in my local machine.

So basically the issues i found are related on how hg interacts with 
the script, thus the reason why i'm putting this as a single issue.

1. in indefero/src/IDF/Scm/Mercurial.php

when getChangeLog calls parseLog, it passes a number (6) which by 
documentation refers to the "number of header" lines 
(although i think i still don't understand what that means). By 
changing this number to 4, it will parse the hg output correctly and 
would stop the error generated: "Summary cannot be null".

2. in indefero/src/IDF/Scm/Mercurial.php

when getCommit calls exec, the command sent is "hg log -p -r 
'revision-hash' -R '/path/to/repo'". I had to replace -R by 
--cwd in order to get the expected behaviour out of mercurial. With 
-R it would just try to get a patch from that revision comparing 
against the current working directory, which i didn't even care to 
find out. --cwd would make hg work in the correct path.

I'm new to indefero and absolutely have no experience with Pluf, so 
if you need any further info just let me know and tell me how to get 
it :p

Regards,
Matías

Comment 1 by Matías Halles, Jul 17, 2010

Here's the output for "hg log -l 25" which is executed in 
getChangeLog: http://zouppowered.com/logRl25.log

And here are the dumps on how parseLog parses depending or the hdrs 
value:
http://zouppowered.com/parseLog_hdrs4.log
http://zouppowered.com/parseLog_hdrs5.log
http://zouppowered.com/parseLog_hdrs6.log

As you can see, i'm getting this exception (
http://x.halles.cl/e52efafa.png) related to the commit dd2c6db6c3da 
(the last one in the list) which is clearly being wrongly parsed due 
to my happy face in the beginning of the message ":D". He.

This wasn't related to the $hdrs passed to parseLog.

I'm gonna try to fix in the afternoon.

Regards,
Matías

Comment 2 by Matías Halles, Jul 18, 2010

The regexp pattern command used to parse the log has to be changed 
from 

/(\S+)\s*:\s*(.*)/

to

/^(\S+):\s*(.*)/

What this does is match the header name by searching for it within 
the beggining of the line and the first ":", with no 
spaces between the string and the ":".

I'm sorry i can't workout a patch, i don't know how to use git.

Comment 3 by Matías Halles, Jul 19, 2010

Also, i don't know why (might be related to hg version) but hg -R 
/path/ will not work on my install when fetching patches between 
versions. You have to use --cwd or both --cwd & -R to have the 
correct behaviour.

This should go inside the getCommit in Mercurial.php

Comment 4 by Matías Halles, Jul 24, 2010

The behaviour might be happening because of my OS version; i'm 
running this on Debian Sarge, although i can't tell for sure, but i 
suppose it is my config and not indefero.

I'm installing latest debian to give it a try.

Gonna keep you updated so someone can close this for sure.

Comment 5 by Loïc d'Anterroches, Sep 6, 2010

Ok, I will need to review everything here. The Mercurial backend was 
created nearly 2 years ago, maybe the new version has a new 
interface and we need to adapt.

Status: Accepted
Summary: Wrong behaviour with mercurial 1.6

Comment 6 by Matías Halles, Sep 6, 2010

Comment 1 and 2, were fixed with the patch relating Issue #508.

I asked around, and the semantics on mercurial have not changed, and 
since nobody else is reporting this error, i'm inclined to think 
that it is because my environment, but i wouldn't be sure.

Comment 7 by Thomas Keller, Mar 18, 2011

Fixed with commit 5fc3a98.

Labels: Milestone:Release1.1
Status: Fixed