Fix for batch mode (was Re: batch mode maintainability)

[Jos Backus]

Fwiw, I'm currently working on getting rid of the automatically generated
filenames --write-batch creates, instead --write-batch should accept a suffix,
just like --read-batch. With that working I'll convert the suffix into a
prefix, per Dave's excellent suggestion.

On Wed, Jan 23, 2002 at 11:05:15AM -0600, Dave Dykstra wrote:
> On Wed, Jan 23, 2002 at 04:37:53PM +1100, Martin Pool wrote:
> > On 17 Jan 2002, Jos Backus <josb at cncdsl.com> wrote:
> > When you get a chance, could you please look at writing some more
> > documentation about batch mode?  It would also be good to try to split
> > out the code a little more.   All the if(read_batch) conditionals look
> > highly breakable.

Yes, I'll look at doing some documentation. What about a paragraph titled
``About batch mode'' with a little explanation how it works, how it differs
from normal rsync operation and a small example?

> So, I hadn't gotten a chance to try your patch, but Martin checked in your
> changes into CVS and I pulled them back out of there and tried them out.
> It is definitely working better.

Great. I was happy to see this go in, so now more people can look at it
easily. I do intend to work on it to make it more stable.

> I'm still seeing a rsync_argvs file getting created in my home directory on
> the remote machine; --read-batch is also printing out "batch file extension"
> with two different numbers so that was my clue.

Strange, that doesn't happen in my test setup. I'll poke at it.

> One time I accidentally tried use --read-batch to
> directory that didn't match the destination directory that was present
> when --write-batch created the files, and then it core dumped at batch.c
> line 487:
> 			if ((s->sums[i].sum1 != file_sum1) ||
> 			    (memcmp
> 			     (s->sums[i].sum2, file_sum2,
> 			      csum_length) != 0)) {
> 				*checksums_match = 0;
> and gdb said that s->sums[i] was a bad address.  s->sums[0] was OK, but i
> was 487.  So, more care needs to be taken with making sure that the
> new destination matches the original destination.

Hm, I wonder if we can make this more flexible.

> Suggestion: have the rsync_argvs file take a $1 parameter as the destination
> rather than the original destination, or maybe ${1:-original_dest} so if
> a new destination is not specified it will default to the original one.
 
Yes, if we can fix the above problem that is a good idea.

> Interesting tidbit: it apparently works with -W, because I noticed that
> when I make a small change to a large file the rsync_delta* file is large
> if my original destination is on the same machine as the source (because in
> that case -W is implied) but if my original destination is on a remote
> machine the rsync_delta* file is small, and both cases produce the correct
> result.  Maybe -W should never be implied when --write-batch is on.

Meaning that with --write-batch you'd always get the small delta file (-W
turned off)?

> - Dave Dykstra

-- 
Jos Backus                 _/  _/_/_/        Santa Clara, CA
                          _/  _/   _/
                         _/  _/_/_/             
                    _/  _/  _/    _/
josb at cncdsl.com     _/_/   _/_/_/            use Std::Disclaimer;