[BUG] patch.py not portable to windows

[John A Meinel]

Aaron Bentley wrote:

> John A Meinel wrote:
>
> >Anyway, inside of that code, he spawns patch in order to apply patches
> >to files. Unfortunately, he uses popen2.Popen3() which does not exist on
> >windows.
>
>
> Oops.
>
> >(patch may not exist on windows, but I'm not worrying about
> >that case yet).
> >This is one of the motivations to move to using subprocess since it
> >really cleans up process spawning.
>
>
> It's unfortunate that we can't just bundle subprocess as we do
> ElementTree, but the need to compile a C module on Windows kills that.
> Canonical's using a library called 'gnarly' for process spawning that's
> all-Python, and supposed to be subprocess-compatible, so that might work.
>
> http://ddaa.net/arch/2004/gnarly/gnarly--devel/gnarly--devel--0/

I might check into that. But I think you can bundle subprocess, and just
state that for windows you must install subprocess manually for 2.3, or
use 2.4. I honestly don't think it is a big dependency. It is one
package that needs to be installed in 1 platform for 1 version. It's not
something that most people have to install.

>
> >In the long term, it would be nice to switch to perhaps a python
> >implementation of patch, something that could keep everything in memory,
> >rather than having to write out a bunch of temporary files. Right now,
> >you have to decompress the store data, write that to a file, then spawn
> >patch and pipe in the changes and create a new file, the read that back
> >in, and delete the other files.
>
>
> It's rather shocking that Python can produce unified diffs, but can't
> apply them, eh?  (Actually, it's not producing them properly, either.)

Isn't it just that it doesn't handle a missing EOL the same way (and
that it changes the number for /dev/null).

>
> I can kinda see why, though.  People would expect the same behavior for
> fuzzy patch-application, which could be tricky to get right.

I think you would have a very strong possibly of slightly different but
equally correct. Diff and patch in fuzzy mode is obviously not an exact
science. (One of my personal peeves with diff is the case where you
insert an if just before another one. It will frequently latch on to
either an empty line or a line with just {, and then show it as a delete
+ modify + add, rather than just an add).

>
> >Is there interest in writing a built-in patch? It wouldn't have to be a
> >super-fancy one, since most of the use cases are for exact patching. Any
> >fuzzy patching is generally done by the merge code (and diff3 or patch).
> >It seems like you could write a unified diff patcher without too many
> >lines of python. It probably wouldn't try as hard as patch might.
>
>
> If you're interested, you might want to use my unified diff parser,
> which is included in bzrtools.  The issue I see is diff3 itself uses
> diff, so unless we get rid of diff3, we need diff, and patch usually
> comes with it.  Replacing diff3 in Python would be no mean feat.

True. I'm pretty okay with depending on them, though it would be nice to
have all of the functionality builtin.
I know diff3 is pretty important for bzr, since it is the real merge
workhorse.

>
> >I suppose we need to come up with some sort of consensus if we want to
> >have things work under windows. Even if we just recommend installing
> >some version if diff & patch, we still can't use the Popen3/Popen4
> >classes in popen2.
>
>
> Sure.  I'll look for an alternative way of invoking it.

The specific warning from the python documentation is:

The only way to retrieve the return codes for the child processes is by
using the poll() or wait() methods on the Popen3 and Popen4 classes;
these are only available on Unix. This information is not available when
using the popen2(), popen3(), and popen4() functions, or the equivalent
functions in the os
<http://www.python.org/doc/current/lib/module-os.html> module. (Note
that the tuples returned by the os
<http://www.python.org/doc/current/lib/module-os.html> module's
functions are in a different order from the ones returned by the popen2
module.)

So there is no way on windows to get both the return code *and* capture
all of the output.
Also, your earlier comment that 'os.popen allows you to pass a sequence'
is also not true on windows. Just Unix.

I think these sort of things are what inspired subprocess to be created.
Because the functionality *wasn't* there on all platforms, and it was
pretty half-baked elsewhere.
It seems a shame to have to re-invent the solution because we are
worried about supporting an edge case.

>
> Aaron

John
=:->

PS> Heck for unix-y stuff, there has been the command module for quite
some time, but that doesn't exist on windows either.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 253 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20050701/87282477/attachment.pgp