Edgewall Software
Modify

Opened 6 years ago

Last modified 21 months ago

#10552 new defect

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@…
Release Notes:
API Changes:

Description

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

Attachments (1)

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

Download all attachments as: .zip

Change History (8)

comment:1 Changed 6 years ago by Remy Blank

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 Changed 6 years ago by 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.

comment:3 Changed 6 years ago by Oleg Frantsuzov <franoleg@…>

Cc: franoleg@… added

comment:4 Changed 5 years ago by 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.

Changed 5 years ago by sayap <sokann@…>

Attachment: TracMercurial-hook.diff added

This diff works for me

comment:5 Changed 5 years ago by Christian Boos

Owner: set to Christian Boos

Thanks for the patch!

comment:6 Changed 5 years ago by Remy Blank

Can't you simply do:

ui.write(stdout)

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

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

comment:7 Changed 5 years ago by sayap <sokann@…>

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

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain Christian Boos.
The ticket will be disowned.
as The resolution will be set.
The owner will be changed from Christian Boos to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences .

 
Note: See TracTickets for help on using tickets.