[Fwd: Re: HTTP 500 when using hooks with hgwebdir.cgi in 1.0.2]

[Jesse Long]

Dear Mercurial Developers,

Please include the patch below in the next release. Basically, the hook 
method fails to restore io correctly if either _pythonhook() or 
_exthook() throw and exception. finally fixes it.

Thanks,
Jesse

-------- Original Message --------
Subject: 	Re: HTTP 500 when using hooks with hgwebdir.cgi in 1.0.2
Date: 	Mon, 17 Nov 2008 16:13:40 +0200
From: 	Jesse Long <jpl at unknown.za.net>
To: 	mercurial at selenic.com
References: 	<49216F64.5090302 at unknown.za.net>



Jesse Long wrote:
> Hi All,
>
> To the authors, thanks for Mercurial. Been using it happily for a while.
> Today I tried using hooks with hgwebdir.cgi (a working installation of
> hgwebdir.cgi, which has been working for yonks now).
>
> I enable hooks in the hgrc of one of the repos like:
> [hooks]
> pretxnchangegroup.test = test `hg heads --template '{branches}\n'|wc -l`
> -eq `hg heads --template '{branches}\n'|sort|uniq|wc -l`
>
> When I push to this url (hosted on Linux Apache server) I get a 500
> error from hg client.
>
> Apache error logs have:
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] Status: 200 Script
> output follows\r
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] Content-Type:
> application/mercurial-0.1\r
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] \r
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] 0
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] adding changesets
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] adding manifests
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] adding file changes
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] added 3 changesets
> with 3 changes to 3 files (+1 heads)
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] transaction abort!
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] rollback completed
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] abort:
> pretxnchangegroup.test hook exited with status 1
> [Mon Nov 17 14:42:39 2008] [error] [client 127.0.0.1] Premature end of
> script headers: hgwebdir.cgi
>
> Using hg serve, all work fine, just hgwebdir.cgi (and maybe hgweb.cgi...
> haven't tested that yet).
>
> I'd really love a patch for this if someone would be so kind.
>
> Thanks and cheers,
> Jesse
>   
Well, it looks like I'll be that kind person.

See patch below. Please note, this is the first python code I've written 
in my life.

Thanks,
Jesse

diff -Naur mercurial-1.0.2.orig/mercurial/hook.py mercurial-1.0.2/mercurial/hook.py
--- mercurial-1.0.2.orig/mercurial/hook.py	2008-08-14 00:11:47.000000000 +0200
+++ mercurial-1.0.2/mercurial/hook.py	2008-11-17 16:11:09.000000000 +0200
@@ -96,20 +96,21 @@
        oldstdout = os.dup(sys.__stdout__.fileno())
        os.dup2(sys.__stderr__.fileno(), sys.__stdout__.fileno())

-    hooks = [(hname, cmd) for hname, cmd in ui.configitems("hooks")
-             if hname.split(".", 1)[0] == name and cmd]
-    hooks.sort()
-    for hname, cmd in hooks:
-        if callable(cmd):
-            r = _pythonhook(ui, repo, name, hname, cmd, args, throw) or r
-        elif cmd.startswith('python:'):
-            r = _pythonhook(ui, repo, name, hname, cmd[7:].strip(),
-                            args, throw) or r
-        else:
-            r = _exthook(ui, repo, hname, cmd, args, throw) or r
-
-    if _redirect:
-        os.dup2(oldstdout, sys.__stdout__.fileno())
-        os.close(oldstdout)
+    try:
+        hooks = [(hname, cmd) for hname, cmd in ui.configitems("hooks")
+                if hname.split(".", 1)[0] == name and cmd]
+        hooks.sort()
+        for hname, cmd in hooks:
+            if callable(cmd):
+                r = _pythonhook(ui, repo, name, hname, cmd, args, throw) or r
+            elif cmd.startswith('python:'):
+                r = _pythonhook(ui, repo, name, hname, cmd[7:].strip(),
+                        args, throw) or r
+            else:
+                r = _exthook(ui, repo, hname, cmd, args, throw) or r
+    finally:
+        if _redirect:
+            os.dup2(oldstdout, sys.__stdout__.fileno())
+            os.close(oldstdout)

    return r