[PATCH] make write_batch local

[Wayne Davison]

On Wed, Jun 16, 2004 at 07:09:46PM -0400, Chris Shoemaker wrote:
> I hope you have the time to review this patch and comment.

The patch looks good on first inspection.  I don't like the change to
the whole-file default, though -- I'd prefer rsync to not force the
--whole-file option in batch mode (since we want batch files to be
efficient by default).

Also, this comment in pipe.c looks like you meant to say "server" rather
than "sender" in the second instance, correct?

>   		am_server = 1;
>   
> + 		/* There is write_batch code on both the receiver and
> + 		 * sender sides.  In local_child, both are local processes,
> + 		 * so we must make sure that only one actually writes.  It 
> + 		 * shouldn't matter which one -- here we prevent sender from
> + 		 * writing. */
> + 		write_batch = 0; 
> + 		
>   		if (!am_sender) 
>   			filesfrom_fd = -1;

In your comment on your testing between 2.5.7 and your patched version, you
said:

> All four methods produce identical batch files.

I assume you mean excepting the file-list file, which changed radically
in 2.6.1 (the old code used to write out its own file-list data, but the
new code uses the actual file-list- sending code to output this data,
making it much easier to maintain).

> Regarding next steps, I've come to believe that the entire batch mode
> code is too invasive into too many parts of the rest of the code.

Yes, I think I'm inclined to agree with you.

> If you're willing to break backward compatibility of existing batch
> files,

I am.  The code is marked as experimental, and I don't see a great need
for people to keep batch files for long periods of time, so this
shouldn't cause people a problem.

> I think the batch mode code can be significantly simplified and made
> more maintainable and flexible.  Basically, I think batch mode should
> just record whatever hits the socket.  I don't see much benefit to
> splitting up the data into 3 files, essentially creating a batch-mode
> specific protocol on the disk, with special reading and writing
> functions.  Simplify, simplify.

That would be much better.  In fact, that could even be made a separate
helper app since it would be possible to have an app record the data and
forward it.  Then, the --rsh and/or --rsync-path option could be used to
have rsync talk to a batch-replay program instead of a real rsync.  The
one complicating factor I can see is that there are some differences
between a daemon stream and a normal remote-shell stream, but it should
be possible do a different batch-replay command depending on what stream
was recorded (e.g. daemon syntax using a custom program via the --rsh
option).  I haven't fully thought through if this external program method
would be that much better than a simplified internal batch mode, though,
so feel free to comment on any ramifications I haven't thought of.

I've checked-in some of your comment tweaks that were not specific to
batch changes.  I then checked-in my version of the remainder of your
patch into the patches dir (named local-batch.diff).  Just do a cvs
update to check it out.

..wayne..