| Submitter | Pierre-Yves David |
|---|---|
| Date | April 23, 2015, 11:56 p.m. |
| Message ID | <8aea112e68b1f90f8b29.1429833382@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/8774/ |
| State | Accepted |
| Headers | show |
Comments
On Fri, 24 Apr 2015 00:56:22 +0100, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1429797459 -3600 > # Thu Apr 23 14:57:39 2015 +0100 > # Branch stable > # Node ID 8aea112e68b1f90f8b2917f06fd4081116502c64 > # Parent 61a2951a30f7c18501027501f7642dd632e72fcd > ui: allow capture of subprocess output > > We want to capture hooks output during bundle2 processing. For this purpose we > introduce a new 'subproc' argument to 'ui.pushbuffer'. When set, the output of > sub process created through 'ui.system' will be captured in the buffer too. > > This will be used in the next changeset. > def write_err(self, *args, **opts): > try: > - if self._bufferstates and self._bufferstates[-1]: > + if self._bufferstates and self._bufferstates[-1][0]: > return self.write(*args, **opts) > if not getattr(self.fout, 'closed', False): > self.fout.flush() > for a in args: > self.ferr.write(str(a)) > @@ -832,12 +836,15 @@ class ui(object): > > def system(self, cmd, environ={}, cwd=None, onerr=None, errprefix=None): > '''execute shell command with appropriate output stream. command > output will be redirected if fout is not stdout. > ''' > + out = self.fout > + if util.any(s[1] for s in self._bufferstates): > + out = self Nitpick, why does it check any bufferstates unlike write_error() ?
On 04/24/2015 02:49 PM, Yuya Nishihara wrote: > On Fri, 24 Apr 2015 00:56:22 +0100, Pierre-Yves David wrote: >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@fb.com> >> # Date 1429797459 -3600 >> # Thu Apr 23 14:57:39 2015 +0100 >> # Branch stable >> # Node ID 8aea112e68b1f90f8b2917f06fd4081116502c64 >> # Parent 61a2951a30f7c18501027501f7642dd632e72fcd >> ui: allow capture of subprocess output >> >> We want to capture hooks output during bundle2 processing. For this purpose we >> introduce a new 'subproc' argument to 'ui.pushbuffer'. When set, the output of >> sub process created through 'ui.system' will be captured in the buffer too. >> >> This will be used in the next changeset. > >> def write_err(self, *args, **opts): >> try: >> - if self._bufferstates and self._bufferstates[-1]: >> + if self._bufferstates and self._bufferstates[-1][0]: >> return self.write(*args, **opts) >> if not getattr(self.fout, 'closed', False): >> self.fout.flush() >> for a in args: >> self.ferr.write(str(a)) >> @@ -832,12 +836,15 @@ class ui(object): >> >> def system(self, cmd, environ={}, cwd=None, onerr=None, errprefix=None): >> '''execute shell command with appropriate output stream. command >> output will be redirected if fout is not stdout. >> ''' >> + out = self.fout >> + if util.any(s[1] for s in self._bufferstates): >> + out = self > > Nitpick, why does it check any bufferstates unlike write_error() ? The idea is to turn a "flag" on saying "now I want subprocess being capture". So buffer push at lower level should not disable the subprocess output capture. That said write_error should probably do something similar. That said², we should probably capture the errors output in the buffer that request it (in both err and subprocess case but it does not seems too critical for now/stable.
On Fri, 24 Apr 2015 15:46:08 +0100, Pierre-Yves David wrote: > On 04/24/2015 02:49 PM, Yuya Nishihara wrote: > > On Fri, 24 Apr 2015 00:56:22 +0100, Pierre-Yves David wrote: > >> # HG changeset patch > >> # User Pierre-Yves David <pierre-yves.david@fb.com> > >> # Date 1429797459 -3600 > >> # Thu Apr 23 14:57:39 2015 +0100 > >> # Branch stable > >> # Node ID 8aea112e68b1f90f8b2917f06fd4081116502c64 > >> # Parent 61a2951a30f7c18501027501f7642dd632e72fcd > >> ui: allow capture of subprocess output > >> > >> We want to capture hooks output during bundle2 processing. For this purpose we > >> introduce a new 'subproc' argument to 'ui.pushbuffer'. When set, the output of > >> sub process created through 'ui.system' will be captured in the buffer too. > >> > >> This will be used in the next changeset. > > > >> def write_err(self, *args, **opts): > >> try: > >> - if self._bufferstates and self._bufferstates[-1]: > >> + if self._bufferstates and self._bufferstates[-1][0]: > >> return self.write(*args, **opts) > >> if not getattr(self.fout, 'closed', False): > >> self.fout.flush() > >> for a in args: > >> self.ferr.write(str(a)) > >> @@ -832,12 +836,15 @@ class ui(object): > >> > >> def system(self, cmd, environ={}, cwd=None, onerr=None, errprefix=None): > >> '''execute shell command with appropriate output stream. command > >> output will be redirected if fout is not stdout. > >> ''' > >> + out = self.fout > >> + if util.any(s[1] for s in self._bufferstates): > >> + out = self > > > > Nitpick, why does it check any bufferstates unlike write_error() ? > > The idea is to turn a "flag" on saying "now I want subprocess being > capture". So buffer push at lower level should not disable the > subprocess output capture. > > That said write_error should probably do something similar. I see. > That said², we should probably capture the errors output in the buffer > that request it (in both err and subprocess case but it does not seems > too critical for now/stable. For default branch, maybe we can do ui.fin = ui.fout = StringIO() to capture all outputs during bundle2 processing.
On 04/24/2015 04:39 PM, Yuya Nishihara wrote: > On Fri, 24 Apr 2015 15:46:08 +0100, Pierre-Yves David wrote: >> On 04/24/2015 02:49 PM, Yuya Nishihara wrote: >>> On Fri, 24 Apr 2015 00:56:22 +0100, Pierre-Yves David wrote: >>>> # HG changeset patch >>>> # User Pierre-Yves David <pierre-yves.david@fb.com> >>>> # Date 1429797459 -3600 >>>> # Thu Apr 23 14:57:39 2015 +0100 >>>> # Branch stable >>>> # Node ID 8aea112e68b1f90f8b2917f06fd4081116502c64 >>>> # Parent 61a2951a30f7c18501027501f7642dd632e72fcd >>>> ui: allow capture of subprocess output >>>> >>>> We want to capture hooks output during bundle2 processing. For this purpose we >>>> introduce a new 'subproc' argument to 'ui.pushbuffer'. When set, the output of >>>> sub process created through 'ui.system' will be captured in the buffer too. >>>> >>>> This will be used in the next changeset. >>> >>>> def write_err(self, *args, **opts): >>>> try: >>>> - if self._bufferstates and self._bufferstates[-1]: >>>> + if self._bufferstates and self._bufferstates[-1][0]: >>>> return self.write(*args, **opts) >>>> if not getattr(self.fout, 'closed', False): >>>> self.fout.flush() >>>> for a in args: >>>> self.ferr.write(str(a)) >>>> @@ -832,12 +836,15 @@ class ui(object): >>>> >>>> def system(self, cmd, environ={}, cwd=None, onerr=None, errprefix=None): >>>> '''execute shell command with appropriate output stream. command >>>> output will be redirected if fout is not stdout. >>>> ''' >>>> + out = self.fout >>>> + if util.any(s[1] for s in self._bufferstates): >>>> + out = self >>> >>> Nitpick, why does it check any bufferstates unlike write_error() ? >> >> The idea is to turn a "flag" on saying "now I want subprocess being >> capture". So buffer push at lower level should not disable the >> subprocess output capture. >> >> That said write_error should probably do something similar. > > I see. > >> That said², we should probably capture the errors output in the buffer >> that request it (in both err and subprocess case but it does not seems >> too critical for now/stable. > > For default branch, maybe we can do ui.fin = ui.fout = StringIO() to capture > all outputs during bundle2 processing. I was trying to say something else, let me give you an example: ui.pushbuffer(error=True) ui.pushbuffer() Idoerr() inner = ui.popbuffer() outer = ui.popbuffer() In that case, the err output from "idoerr()" should be returned in "outer". (currently not captured, current subprocs behavior return it in "inner")
On Fri, 24 Apr 2015 16:49:43 +0100, Pierre-Yves David wrote: > On 04/24/2015 04:39 PM, Yuya Nishihara wrote: > > On Fri, 24 Apr 2015 15:46:08 +0100, Pierre-Yves David wrote: > >> On 04/24/2015 02:49 PM, Yuya Nishihara wrote: > >>> On Fri, 24 Apr 2015 00:56:22 +0100, Pierre-Yves David wrote: > >>>> # HG changeset patch > >>>> # User Pierre-Yves David <pierre-yves.david@fb.com> > >>>> # Date 1429797459 -3600 > >>>> # Thu Apr 23 14:57:39 2015 +0100 > >>>> # Branch stable > >>>> # Node ID 8aea112e68b1f90f8b2917f06fd4081116502c64 > >>>> # Parent 61a2951a30f7c18501027501f7642dd632e72fcd > >>>> ui: allow capture of subprocess output > >>>> > >>>> We want to capture hooks output during bundle2 processing. For this purpose we > >>>> introduce a new 'subproc' argument to 'ui.pushbuffer'. When set, the output of > >>>> sub process created through 'ui.system' will be captured in the buffer too. > >>>> > >>>> This will be used in the next changeset. > >>> > >>>> def write_err(self, *args, **opts): > >>>> try: > >>>> - if self._bufferstates and self._bufferstates[-1]: > >>>> + if self._bufferstates and self._bufferstates[-1][0]: > >>>> return self.write(*args, **opts) > >>>> if not getattr(self.fout, 'closed', False): > >>>> self.fout.flush() > >>>> for a in args: > >>>> self.ferr.write(str(a)) > >>>> @@ -832,12 +836,15 @@ class ui(object): > >>>> > >>>> def system(self, cmd, environ={}, cwd=None, onerr=None, errprefix=None): > >>>> '''execute shell command with appropriate output stream. command > >>>> output will be redirected if fout is not stdout. > >>>> ''' > >>>> + out = self.fout > >>>> + if util.any(s[1] for s in self._bufferstates): > >>>> + out = self > >>> > >>> Nitpick, why does it check any bufferstates unlike write_error() ? > >> > >> The idea is to turn a "flag" on saying "now I want subprocess being > >> capture". So buffer push at lower level should not disable the > >> subprocess output capture. > >> > >> That said write_error should probably do something similar. > > > > I see. > > > >> That said², we should probably capture the errors output in the buffer > >> that request it (in both err and subprocess case but it does not seems > >> too critical for now/stable. > > > > For default branch, maybe we can do ui.fin = ui.fout = StringIO() to capture > > all outputs during bundle2 processing. > > I was trying to say something else, let me give you an example: > > ui.pushbuffer(error=True) > ui.pushbuffer() > Idoerr() > inner = ui.popbuffer() > outer = ui.popbuffer() > > In that case, the err output from "idoerr()" should be returned in > "outer". (currently not captured, current subprocs behavior return it in > "inner") Got it, thanks. My point is that push/popbuffer isn't always necessary to capture output because now we have ui.fout and .ferr. Regards,
Patch
diff --git a/hgext/color.py b/hgext/color.py --- a/hgext/color.py +++ b/hgext/color.py @@ -443,11 +443,11 @@ class colorui(uimod.ui): def write_err(self, *args, **opts): if self._colormode is None: return super(colorui, self).write_err(*args, **opts) label = opts.get('label', '') - if self._bufferstates and self._bufferstates[-1]: + if self._bufferstates and self._bufferstates[-1][0]: return self.write(*args, **opts) if self._colormode == 'win32': for a in args: win32print(a, super(colorui, self).write_err, **opts) else: diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -74,11 +74,12 @@ default = %s class ui(object): def __init__(self, src=None): # _buffers: used for temporary capture of output self._buffers = [] - # _bufferstates: Should the temporary capture includes stderr + # _bufferstates: + # Should the temporary capture includes stderr and subprocess output self._bufferstates = [] self.quiet = self.verbose = self.debugflag = self.tracebackflag = False self._reportuntrusted = True self._ocfg = config.config() # overlay self._tcfg = config.config() # trusted @@ -538,16 +539,19 @@ class ui(object): @util.propertycache def paths(self): return paths(self) - def pushbuffer(self, error=False): + def pushbuffer(self, error=False, subproc=False): """install a buffer to capture standard output of the ui object - If error is True, the error output will be captured too.""" + If error is True, the error output will be captured too. + + If subproc is True, output from subprocess (typically hooks) will be + captured too.""" self._buffers.append([]) - self._bufferstates.append(error) + self._bufferstates.append((error, subproc)) def popbuffer(self, labeled=False): '''pop the last buffer and return the buffered output If labeled is True, any labels associated with buffered @@ -583,11 +587,11 @@ class ui(object): for a in args: self.fout.write(str(a)) def write_err(self, *args, **opts): try: - if self._bufferstates and self._bufferstates[-1]: + if self._bufferstates and self._bufferstates[-1][0]: return self.write(*args, **opts) if not getattr(self.fout, 'closed', False): self.fout.flush() for a in args: self.ferr.write(str(a)) @@ -832,12 +836,15 @@ class ui(object): def system(self, cmd, environ={}, cwd=None, onerr=None, errprefix=None): '''execute shell command with appropriate output stream. command output will be redirected if fout is not stdout. ''' + out = self.fout + if util.any(s[1] for s in self._bufferstates): + out = self return util.system(cmd, environ=environ, cwd=cwd, onerr=onerr, - errprefix=errprefix, out=self.fout) + errprefix=errprefix, out=out) def traceback(self, exc=None, force=False): '''print exception traceback if traceback printing enabled or forced. only to call in exception handler. returns true if traceback printed.'''