[MERGE] fix non-ascii messages handling in 'missing' command

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Alexander Belchenko wrote:
> John Arbash Meinel пишет:
>> Alexander Belchenko wrote:
>>> Attached patch provide blackbox test and fix for missing command.
>> So @display_command is meant for commands that shouldn't print 'bzr:
>> broken-pipe' when piped into 'less' and canceled before they are
>> finished.
>>
>> I don't really think it is needed for missing. Though it could be argued
>> either way.
> 
> Well, this mechanics (at this moment) actually don't work as expected on
> win32 because here it's "Errno.22 Invalid argument" raised instead of
> broken pipe. So, I'm lightly delete this decorator.

Well, this is just a broken implementation that needs to be fixed. I
just tested it myself, and I see exactly what you mentioned. The error
string seems to be: "The process tried to write to a nonexistent pipe."

Looking deeper, it seems we are calling 'sys.stdout.flush' 2 times. Once
inside display_command, and once outside. I'm guessing the writing is
raising an exception which seems to actually have 'errno = 0' (crappy
crappy)

the run_bzr_captured actually has a try/finally: sys.stdout.flush() and
*that* one is causing the EINVAL to be raised.

I tested it a bit, and I got it to absorb the broken pipe with the
attached patch.


> 
> But I think that it does not hurt and sometimes did right job. Per
> example, in my real work sometimes I use missing for comparing two
> branches and sometimes missing log report about 10-30 missing revisions.
> Even 'missing --line' is not good choice sometime. So I think missing
> command is more like compare-log and it's sometimes could be used with
> 'less' pager. So decorator don't hurt.
> 
> But this decorator anyway (at this moment) does not work on win32 so
> here another patch without it.
> 
>>
>> The 'encoding_type = "replace"' + sys.stdout => self.outf is the fix I
>> was looking for, and looks like what you did.
> 
> Can I have +1 for this new patch?
> 
> -- 
> Alexander
> 

I'm actually thinking to accept your first form. Because I think missing
is a display command. It just hasn't been discussed before.
A quick poll on IRC seems to agree. So I'll go ahead and merge your
first bundle.


I really don't like swallowing the exception this way, but if win32
python is going to raise IOError with errno = 0 and text 'Error', there
isn't much we can do. They aren't raising EPIPE, and it seems to be
EINVAL to call flush() on sys.stdout once the pipe is gone.

We could remove the sys.stdout.flush() call, but the whole point is to
force something like EPIPE to be handled inside the exception handlers,
rather than while the stack is unwinding at the end.

I'll have to think of a way to test this. 'less' does exist on win32, so
it might be sufficient to call run_bzr_subprocess with the output piped
into less. It's just tricky to force real win32 errors in the current
test suite.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEouCgJdeBCYSNAAMRAolnAJ49o2WGLiaOWvnmoHOmAZp7ih7jMgCaAjlj
h+IceZl5XpPCh6lrIwb+eGk=
=6YuD
-----END PGP SIGNATURE-----
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: win32-swallow-epipe.diff
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20060628/9ed5687e/attachment.diff