[MERGE] get rid of os.spawnvp() usage for external diff

John Arbash Meinel john at arbash-meinel.com
Sat Jun 17 02:20:53 BST 2006


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

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>>> The test is skipped in that case.
>>> Do you need the external_code to handle it specifically.
> 
> I think that would be best.  If you look at how we invoke diff3,
> (patch.py) we throw NoDiff3 when it's not present.
> 
> I pity the user who gets 'No such file or directory' when they run 'bzr
> diff --diff-options=-p.

Well, now I'm raising NoDiff, which the test suite can catch.
It is difficult to test that it is definitely raised, since we don't
have a way of specifying the name of the external diff program.

> 
>>>> Sorry,  --binary.
>>>
>>> I would be up for that. It does nothing on posix platforms, but is very
>>> important for windows. Should we always be setting this flag, or just in
>>> the test suite?
> 
> I think always, because we transparently switch to external patch when
> certain options are supplied, and so we should minimize the differences.

Done.

> 
>>>> It's easier than that:
>>>> p = Popen(stdout=PIPE)
>>>> to_file.writelines(p.stdout.readlines())
> 
>>> The problem is if the output is larger than say 8K, then this would
>>> block.
> 
> No, that's not when we'd block.  readlines will block until diff has
> sent us everything.  Which is fine.
> 
> Aaron

Well, I switched to using osutils.pumpfile() so that we don't have to
buffer everything in memory.

Also, I use 'to_file' if I can, rather than always redirecting through
the pipe.

How about now?

John
=:->

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

iD8DBQFEk1j1JdeBCYSNAAMRAsOAAKC34iqhvae01gsMjAe69uDW2JdE8ACcCeYZ
hUSpZLs6pwjYkHqKR7NYGM4=
=Xxbw
-----END PGP SIGNATURE-----
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: external-diff.diff
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20060616/56b6703e/attachment.diff 


More information about the bazaar mailing list