No output from hooks when pushing through http

Maxim Khitrov mkhitrov at gmail.com
Wed Jun 30 14:13:11 UTC 2010


On Fri, Jun 25, 2010 at 3:46 PM, Maxim Khitrov <mkhitrov at gmail.com> wrote:
> On Fri, Jun 25, 2010 at 3:12 PM, Matt Mackall <mpm at selenic.com> wrote:
>> On Fri, 2010-06-25 at 14:54 -0400, Maxim Khitrov wrote:
>>> On Fri, Jun 25, 2010 at 1:27 PM, Matt Mackall <mpm at selenic.com> wrote:
>>> > On Fri, 2010-06-25 at 13:20 -0400, Maxim Khitrov wrote:
>>> >> On Fri, Jun 25, 2010 at 12:26 PM, Matt Mackall <mpm at selenic.com> wrote:
>>> >> > On Fri, 2010-06-25 at 11:49 -0400, Maxim Khitrov wrote:
>>> >> >> Hello all,
>>> >> >>
>>> >> >> I'm trying to get a Sphinx repository to automatically rebuild
>>> >> >> documentation after I push out a new changegroup to the server. The
>>> >> >> actual rebuild process works fine. The problem is that it isn't
>>> >> >> sending any output back to the client, which is problematic if there
>>> >> >> are any errors during the build.
>>> >> >
>>> >> > I'm afraid that's just how it works currently. stdout and stderr from
>>> >> > http hooks get redirected at the moment, rather than captured and piped
>>> >> > to the client.
>>> >> >
>>> >>
>>> >> I see, guess I should have taken a look at the code first. I made a
>>> >> small patch that seems to do what I need. Any feedback appreciated.
>>> >>
>>> >> http://mercurial.selenic.com/bts/issue2253
>>> >
>>> > Please send patches to the list. Reviewing patches on the BTS is too
>>> > ungainly; sending a patch to the BTS is a good way to have your patch
>>> > ignored.
>>>
>>> Understood, please see updated version below.
>>>
>>> > Your patch is interesting, but can't be accepted as is: util.py is
>>> > designed to be more or less unaware of all the code built on top of it,
>>> > and it should never ever be handed a ui object. Having util.system
>>> > instead take a callback might be acceptable though.
>>>
>>> But don't you already have an option to pass a ui object as onerr
>>> parameter? I think treating the new parameter as a generic file-like
>>> object might be the best solution (just renamed it). If you still
>>> think a callback is better, can you suggest at what point it should be
>>> executed and with what parameters? My guess would be to pass it the
>>> proc object that is returned from Popen, replacing the for loop that's
>>> currently there.
>>>
>>> > A second issue is that we generally want hook output to be unbuffered -
>>> > your patch buffers hook output for all users, not just http.
>>>
>>> I assume you mean the buffering that communicate() does while it waits
>>> for the process to finish? The new version fixes this and restricts
>>> capture to http operations.
>>>
>>> - Max
>>>
>>
>> Interesting. I'm not really thrilled with this answer because now it's
>> hooks.py that knows too much about its callers. But I'm not sure what
>> the alternatives are. One would be to -always- have hooks use this
>> trick, which would let the caller decide whether buffering was needed or
>> not at a much higher level.
>
> I'm open to suggestions. I know very little about Mercurial internals,
> so not sure of a better way to determine if the code is running under
> a web server.
>
>> Should probably add mention that out is assumed to have a write()
>> method.
>
> Done.
>
>> Looks good!
>>
>> Does this work on Windows?
>
> I'm 99% certain that it does, but I don't have a Windows Mercurial
> server to test on. I ran the portion of the code from
> "subprocess.Popen" till "rc = proc.returncode" and didn't run into any
> problems there.
>
> - Max

Matt, is there anything else preventing this patch from being accepted?

- Max



More information about the Mercurial mailing list