Peter Lundblad <firstname.lastname@example.org> wrote on 02/28/2006 03:09:16 AM:
> Because that'd be a deadlock:-) We create pipes to read the hooks'
> stderr output. We must read that output before waiting for the
> process, else the pipe buffers might fil up. (I think Paul's patch has
> the same problem.)
Philip Martin <email@example.com> wrote on 02/28/2006 05:12:30 PM:
> Paul Burba <firstname.lastname@example.org> writes:
> First impression is that it's ugly. You have replaced the two
> functions svn_io_start_cmd and svn_io_wait_for_cmd with the single
> function svn_io_run_os400_cmd. I assume you did it like that because
> the two functions communicate via an apr_proc_t and that doesn't allow
> you to add extra data.
> Since this is a workaround for an APR bug I'm not sure why you chose
> to put a public function in libsvn_subr when the only caller is in
> libsvn_repos. Does the OS400 port support an external diff or
> external editor? Your patch doesn't seem to have addressed those.
> An alternative would be to modify the existing svn_io_start_cmd and
> svn_io_wait_for_cmd functions to pass svn_io_proc_t instead of
> apr_proc_t, this would allow you to add any extra data you need. I
> think svn_io_proc_t could be opaque and so all the OS400 stuff could
> be private to io.c. It would mean that the external diff/editor code
> get fixed as well, if that makes a difference.
> I feel a bit guilty about suggesting that since I seem to be asking
> for all your patches to be re-written :-/
Thanks for taking a look at this patch, I probably won't have time to
address your comments until Thursday.
Sorry for the delay, I'll get back to this soon, but I didn't want you to
think I was ignoring your comments!
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Wed Mar 1 00:12:47 2006