TracMercurial hooks.py dies when using TortoiseHg

[wayne@…]

In the ​hooks.py script it uses subprocess.Popen to call trac-admin. If you're running from the console it should be fine, but when you try to run from TortoiseHg on Windows you get a nice "helpful" message:

error: commit hook raised an exception: [Error 6] The handle is invalid

It took me a while but I finally tracked down the problem - apparently it's a more or less ​known issue with the subprocess module.

The fix is surprisingly simple - change this:

            proc = subprocess.Popen(command, close_fds=close_fds,
                                    stdout=subprocess.PIPE,
                                    stderr=subprocess.STDOUT)

to this:

            proc = subprocess.Popen(command, close_fds=close_fds,
                                    stdout=subprocess.PIPE,
                                    stderr=subprocess.STDOUT,
                                    stdin=subprocess.PIPE)

Apparently when there's no console on Windows (e.g. running Mercurial commits by way of TortoiseHg)

subprocess cannot inherit the standard handles when the process has no console, you have to create pipes for all 3 channels, and close those that are not needed. ​Python mailing list

[Remy Blank]

It's probably not quite as simple as that. Creating a pipe for stdout and stderr, but not reading it, risks blocking the subprocess if it outputs too much data.

What we should probably do is define two separate pipes for stdout and stderr, read them while the subprocess is running, and pass the data to ui.write() and ui.write_err(), respectively. That way, TortoiseHg would even get a chance to relay any issues to the user.

[anonymous]

Use Popen.communicate, so it will not deadlock.

And use a file instead a PIPE if you expect too much output, because a PIPE is not buffered.

[sayap <sokann@…>]

I have a hgweb repo with hooks:

commit = python:tracext.hg.hooks.add_changesets
changegroup = python:tracext.hg.hooks.add_changesets

When hosted with Apache mod_wsgi, the hooks script will alternative between success and failure, e.g. push n = success, push n+1 = failure, push n+2 = success, etc.

This problem is fixed after applying the solution suggested here.

[sayap <sokann@…>]

This diff works for me

[Christian Boos]

Thanks for the patch!

[Remy Blank]

Can't you simply do:

ui.write(stdout)

instead of iterating over all characters and writing them one by one?

[sayap <sokann@…>]

Yup, didn't notice that. In this case, I guess we can just do:

ui.write(proc.communicate()[0])

[Peter Suter]

Pushed patch with changes from comment:6 in [60/mercurial-plugin].