[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