[PATCH] Batch-mode rewrite

[Chris Shoemaker]

On Wed, Jul 14, 2004 at 08:07:36PM -0700, Wayne Davison wrote:
> I did some work refining your patch a little, and liked the result so
> much I went ahead and checked it into CVS.  I'd appreciate it if you
> could give my changes a look to see if I messed anything up. 
> 
> The most important changes I made were:
> 
>  - Delay the start of batch writing until after any exclude-list and
>    files-from data gets sent.

If I understand your changes, the files-from stuff you're skipping is
only the flagging of the fd, no actual communication.

The exclude-list change raises a question:  I wasn't paying that close
attention to exclude list stuff, but, if the user uses --exclude and
--delete and then applies the batch with the --delete but not the
exclude, what should happen?  I think it's a bit of an abuse batch-mode,
so maybe undefined behavior is ok.  I think it's ok to have the excluded
file not deleted, but that would only happen if the exclude list is
written to the batch file.

However, the exclude list is another one of those protocol dependancies
on server-ness, isn't it?  I see "send_exclude_list(f_out)" in the
both am_sender and !am_sender paths of client, and
"recv_exclude_list(f_in)" in both server paths.   I guess I can see why
it may be easier to leave it out of the batch.  Otherwise, it has to
fall into the same category as the protocol version and checksum_seed.
:(

> 
>  - Reinstated the final report() so that the end-of-transfer summary
>    shows up when reading a batch.  I had to make the report() code
>    always write out the stats data into the batch file.  Added a big
>    comment at the top to help explain what it is doing.

It's a shame to make report() more complicated than it already is (too).
But, this does give the stats for a read-batch, which is nice.  I guess
it's worth it.

>    
>  - To make the first 2 items easier on the code, I changed the pipe
>    code so that a local-to-local transfer turns off write-batch mode
>    in the "am_server" process (this way the server never writes the
>    batch, just the client).

Ah, oh well, I guess the un-importance of that decision was bound to
change.

> 
>  - Got rid of the --files-from option from the FOO.rsync_argvs.
> 

makes sense.

>  - Got rid of a host: prefix in the destination arg in FOO.rsync_argvs
>    (this got checked in before the new-batch changes since it affected
>    the old batch code too).

good catch.

> 
>  - Added a check to the generator so that it doesn't do a lot of
>    useless checksum-generating work when --read-batch is specified.

check.
> 
>  - Added some text to the manpage that talks about what options change
>    when switching from --write-batch to --read-batch and what options
>    must remain the same.

Yes, that's helpful.

> 
>  - Decided not to include your comments on potential protocol changes.

That's probably appropriate.

> 
>  - Removed some unused variables, and other misc. twiddles.
> 
> If you want to see the changes without resorting to CVS, you can look
> here:
> 
>     http://www.blorf.net/newbatch.patch
> 

I looked at the patch and what you checked into CVS and it all looks
good to me.  I think what we have will be a lot easier to maintain:

batch.c        |  263 ---------------------------------------------------------
clientserver.c |    3
compat.c       |   11 +-
flist.c        |   20 +---
generator.c    |    4
io.c           |   38 ++++++++
main.c         |  122 +++++++++++++++++---------
match.c        |    3
options.c      |   24 +----
pipe.c         |   25 ++++-
proto.h        |   18 +--
rsync.1        |  117 +++++++++++--------------
rsync.yo       |  111 +++++++++++-------------
sender.c       |  149 +++++++++-----------------------
token.c        |   34 -------
 15 files changed, 326 insertions(+), 616 deletions(-)
 

> Thanks for the nice patches!  I think this will make batch mode much
> nicer.  We can consider marking it as less experimental in the man page
> after some more testing.  (I've done just a little so far.)

Agreed, and thank _you_!

-chris

> 
> ..wayne..