2.1.1 is blocked on issue 891

[Steve Borho]

https://bitbucket.org/tortoisehg/thg/issue/891

Apparently Mercurial 1.9 will no longer launch external merge tools
when called from thgw.exe (which has no console, and thus no
stdout/stderr file handles).

--
Steve Borho

[Steve Borho]

FYI: I've found I can debug this problem by running: pythonw thg resolve

This opens thg with the "Windows" (non-console) version of python and
correctly models the situation of having no valid sys.stdout, stderr
file handles.

This patch to the thg script makes the resolve tool work, but it also
makes my skin crawl.

@@ -77,6 +77,9 @@
mystderr = cStringIO.StringIO()
origstderr = sys.stderr
sys.stderr = mystderr
+ sys.stdout = cStringIO.StringIO()
+ sys.__stdout__ = sys.stdout
+ sys.__stderr__ = sys.stderr
ret = 0
try:
ret = tortoisehg.hgqt.run.dispatch(sys.argv[1:])

--
Steve Borho

[Wagner Bruna]

This broke "thg help" from the command line.

fileno() seems to correctly distinguish pythonw.exe from
python.exe, but it makes things even uglier... Could anyone
try the following on Windows? :

diff --git a/thg b/thg

--- a/thg
+++ b/thg
@@ -77,9 +77,10 @@ else:

mystderr = cStringIO.StringIO()
origstderr = sys.stderr
sys.stderr = mystderr

- sys.stdout = cStringIO.StringIO()
- sys.__stdout__ = sys.stdout
- sys.__stderr__ = sys.stderr
+ if getattr(origstderr, 'fileno', lambda: -1)() < 0:

[David Wilhelm]

Based on Idan's comments on the Mercurial list, it seems to be enough
to ensure that sys.stdout == sys.__stdout__.

After reproducing the error, this fixes for me on Win 7:

diff --git a/thg b/thg
--- a/thg
+++ b/thg

@@ -77,6 +77,8 @@ else:

mystderr = cStringIO.StringIO()
origstderr = sys.stderr
sys.stderr = mystderr

[Adrian Buehlmann]

I'm wondering how stupid this would be (untested):

diff --git a/tortoisehg/hgqt/thread.py b/tortoisehg/hgqt/thread.py
--- a/tortoisehg/hgqt/thread.py
+++ b/tortoisehg/hgqt/thread.py
@@ -102,6 +102,8 @@
self.setconfig('ui', 'interactive', 'on')
self.setconfig('progress', 'disable', 'True')

+ self.fout = None
+
def write(self, *args, **opts):
if self._buffers:
self._buffers[-1].extend([str(a) for a in args])

[Idan K]

It might make this problem go away but I don't think this is the
solution.

The issue here is that prior to 1.9, there were certain code paths
(such as util.system)
that wrote to sys.stdout directly without going through the ui class.

This is bad for the command server since output written to the client
needs to obey
the command protocol. So we added the I/O descriptors to the ui class,
and now everything
goes through them, including potential output by a util.system call.

When thg gave dispatch a subclass of ui.ui with its own write(), it
didn't capture the filemerge
code path that uses util.system. So theoretically if the merge tool
generated any kind of output,
it would be lost.

Setting the I/O descriptors on the ui should make this problem go
away,
and will also capture any output from util.system. You might even not
have
to subclass ui anymore, depending on what you're doing in write().

[Idan Kamara]

On Jul 7, 2:39 am, Adrian Buehlmann wrote:
> I'm wondering how stupid this would be (untested):
>
> diff --git a/tortoisehg/hgqt/thread.py b/tortoisehg/hgqt/thread.py
> --- a/tortoisehg/hgqt/thread.py
> +++ b/tortoisehg/hgqt/thread.py
> @@ -102,6 +102,8 @@
>          self.setconfig('ui', 'interactive', 'on')
>          self.setconfig('progress', 'disable', 'True')
>
> +        self.fout = None
> +
>      def write(self, *args, **opts):
>          if self._buffers:
>              self._buffers[-1].extend([str(a) for a in args])

That will probably raise an exception when util.system tries to write to ui.fout.

[Adrian Buehlmann]

Why? util.system checks for None:

http://selenic.com/repo/hg/file/09c9c120a817/mercurial/util.py#l357

if out is None or out == sys.__stdout__:
rc = subprocess.call(cmd, shell=True, close_fds=closefds,
env=env, cwd=cwd)
else:
proc = subprocess.Popen(cmd, shell=True, close_fds=closefds,
env=env, cwd=cwd, stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
for line in proc.stdout:
out.write(line)
proc.wait()
rc = proc.returncode

[Idan Kamara]

Oh, right.

This is still bad because 1) the output is gone, 2) there might be other places that (rightfully) assume ui.fout is a valid file-like object.

[Adrian Buehlmann]

I must say I rather prefer losing that output (we don't care anyway
about stdout of merge tools on TortoiseHg) instead of aborting because
of an invalid handle.

Perhaps we should recommend that people stay on 2.0 until this handle
mess has been sorted out.

[Adrian Buehlmann]

Ok. So we probably have to be a bit more explicit that we don't care
about stuff written to stdout/stderr directly:

$ hg diff

diff --git a/tortoisehg/hgqt/thread.py b/tortoisehg/hgqt/thread.py
--- a/tortoisehg/hgqt/thread.py
+++ b/tortoisehg/hgqt/thread.py

@@ -90,6 +90,12 @@
unit = hglib.tounicode(unit or '')
self.progressSignal.emit(topic, pos, item, unit, total)

+class Ignore(object):

+ def write(self, *args, **opts):

+ pass
+ def flush(self):
+ pass
+
class QtUi(uimod.ui):
def __init__(self, src=None, responseq=None):
super(QtUi, self).__init__(src)
@@ -102,6 +108,9 @@

self.setconfig('ui', 'interactive', 'on')
self.setconfig('progress', 'disable', 'True')

+ self.fout = Ignore()
+ self.ferr = Ignore()

+
def write(self, *args, **opts):
if self._buffers:
self._buffers[-1].extend([str(a) for a in args])

We *do* care about ui.write() and friends. But I don't think we need to
care about things written to fout and ferr directly. But apparently we
now need to make sure Mercurial can happily write to fout and ferr.

[Steve Borho]

Not always true, sadly. We do want the output from hooks that write
to stdout so they appear in our output window.

--
Steve Borho

[Adrian Buehlmann]

Does this work on 2.0?

[Steve Borho]

I don't believe it does work in 2.0, but it did in 1.9.

--
Steve Borho

[Adrian Buehlmann]

On 2011-07-07 20:14, Idan Kamara wrote:
> On Thu, Jul 7, 2011 at 7:34 PM, Adrian Buehlmann <adr...@cadifra.com

> <mailto:adr...@cadifra.com>> wrote:
>>
>> On 2011-07-07 16:07, Idan Kamara wrote:
>> > On Thu, Jul 7, 2011 at 5:02 PM, Adrian Buehlmann <adr...@cadifra.com
> <mailto:adr...@cadifra.com>

> There might be some mergetool out there that for god knows why writes
> something that interests someone to stdout. Any reason to treat this any
> differently to say hooks like Steve said?

Ok. I'll leave this to you guys to sort it out. Not my area of expertise
anyway.

> Another place that uses ui.fout directly is the 'cat' command, and there
> are probably more.
>
> This should also work:
>
> diff -r 3ea676a4bfc5 tortoisehg/hgqt/thread.py
> --- a/tortoisehg/hgqt/thread.py Fri Jul 01 21:47:26 2011 -0500
> +++ b/tortoisehg/hgqt/thread.py Thu Jul 07 20:46:47 2011 +0300
> @@ -99,6 +99,8 @@
> else:
> self.sig = UiSignal(responseq)
>
> + self.fout = self.sig
> +

> self.setconfig('ui', 'interactive', 'on')
> self.setconfig('progress', 'disable', 'True')
>

> But without knowing more of what's going on in thg, it might be a hack.
>
> ui.ferr/fin should probably be taken care of as well. For instance,
> ui.fin is used when
> reading a patch from '-', if thg allows that (without using a temporary
> file).

[Idan Kamara]

There might be some mergetool out there that for god knows why writes

something that interests someone to stdout. Any reason to treat this any

differently to say hooks like Steve said?

Another place that uses ui.fout directly is the 'cat' command, and there

are probably more.

This should also work:

diff -r 3ea676a4bfc5 tortoisehg/hgqt/thread.py

--- a/tortoisehg/hgqt/thread.py Fri Jul 01 21:47:26 2011 -0500

+++ b/tortoisehg/hgqt/thread.py Thu Jul 07 20:46:47 2011 +0300

@@ -99,6 +99,8 @@

         else:

             self.sig = UiSignal(responseq)

 

+        self.fout = self.sig

+

         self.setconfig('ui', 'interactive', 'on')

         self.setconfig('progress', 'disable', 'True')