Edgewall Software

Opened 9 years ago

Closed 2 years ago

#10552 closed defect (fixed)

TracMercurial hooks.py dies when using TortoiseHg

Reported by: wayne@… Owned by: Christian Boos
Priority: normal Milestone: plugin - mercurial
Component: plugin/mercurial Version:
Severity: normal Keywords: TracMercurial, subprocess
Cc: franoleg@… Branch:
Release Notes:

Hooks pipe stdin.

API Changes:
Internal Changes:


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,

to this:

            proc = subprocess.Popen(command, close_fds=close_fds,

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

Attachments (1)

TracMercurial-hook.diff (857 bytes ) - added by sayap <sokann@…> 8 years ago.
This diff works for me

Download all attachments as: .zip

Change History (9)

comment:1 by Remy Blank, 9 years ago

Milestone: plugin - mercurial

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.

comment:2 by anonymous, 9 years ago

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.

comment:3 by Oleg Frantsuzov <franoleg@…>, 9 years ago

Cc: franoleg@… added

comment:4 by sayap <sokann@…>, 8 years ago

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.

by sayap <sokann@…>, 8 years ago

Attachment: TracMercurial-hook.diff added

This diff works for me

comment:5 by Christian Boos, 8 years ago

Owner: set to Christian Boos

Thanks for the patch!

comment:6 by Remy Blank, 8 years ago

Can't you simply do:


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

Last edited 8 years ago by Remy Blank (previous) (diff)

comment:7 by sayap <sokann@…>, 8 years ago

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


comment:8 by Peter Suter, 2 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos to the specified user.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.