On Fri, May 11, 2018 at 6:42 PM, Yuya Nishihara <y...@tcha.org> wrote:
> On Fri, 11 May 2018 14:08:18 -0700, Gregory Szorc wrote: > > On Mon, May 7, 2018 at 6:11 AM, Yuya Nishihara <y...@tcha.org> wrote: > > > > > # HG changeset patch > > > # User Yuya Nishihara <y...@tcha.org> > > > # Date 1521963557 -32400 > > > # Sun Mar 25 16:39:17 2018 +0900 > > > # Node ID 2be95f3cc4f8a4b59f7c50af093ea033d1d09c5e > > > # Parent 04ceb267271a45ae08352d76a9e91f8037ce53e7 > > > test-ssh: add some flush() to make output deterministic > > > > > > We shouldn't rely on buffering mode/state of file handles. > > > > > > > I'm accepting this and understand why we need a flush() to make behavior > > deterministic. > > Basically I inserted flush() where crossing underlying file objects, e.g. > between print() and ui.write(). Some of the last ui.flush() might be > unnecessary. OTOH, writing to sys.std* in in-process hooks is API > violation, > so we wouldn't have to care seriously about that. > IIRC we only recently made print() and sys.std* writing an API violation. Or maybe it always was and we happened to go out of our way to make it work. For the record, the I/O tests in this file (which I wrote) were to basically tease out behavior so refactoring would uncover changes. If we want to change behavior so e.g. sys.stdout() writes aren't intercepted and treated as hook output, I'm totally fine with that. We may want to issue a deprecation warning for a cycle first though. Or wrap sys.stdout during hook evaluation so any writes result in a warning message being written to the hook output. Alternatively, we could more strongly tie sys.stdout to ui.fout during hook evaluation so we're *not* crossing file objects. But the code here is a bit gnarly. I'm not sure if I trust any more hacks in this area. I'd rather force the use of ui.write() for hook output. > > > However, I think we should consider adding the flush() to the code that > > invokes the hooks - not the hooks themselves. In theory, hook output is > > streamed to clients. So if we want to ensure hook output is seen by > clients > > soon after it is generated, I think we should be flushing output > > automatically when invoking hooks. In the context of wire protocol > > requests, perhaps we should always be making the ui object unbuffered? > > Maybe we can simply do ui.flush() at the end of runhooks()? It wouldn't be > easy to change the buffering mode of open files. > That seems like a reasonable change. Or we could wrap ui.write() so it automatically does a .flush().
_______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
