Re: [rdiff-backup-users] --override-chars-to-quote bug fixed and default behaviour change

[Andrew Ferguson]

On Jan 6, 2009, at 12:01 PM, Oliver Mulatz wrote:
> On 2009-01-05 Oliver Mulatz wrote:
>
> > Another issue: there's a bug (RDB aborts) in '--override-chars-to- 
> quote'
> > functionality, (I require for cross-platform filesystem  
> compatibility).
> >
> > It hits each time "--override-chars-to-quote" is used, even with  
> empty
> > set ('') for keeping capital chars from being escaped.
> >
> > The exact error including trace was also posted to the ML on  
> Oct'08 by EliW:
> > 
> http://www.backupcentral.com/phpBB2/two-way-mirrors-of-external-mailing-lists-3/rdiff-backup-23/probleme-with-rdiff-on-smbfs-92963/#287692
>
> I fixed it, but would like to go deeper.

Did you send the patch to me? (If so, I apologize for not noticing  
yet) ... I hope we wrote the same fix to compare_ctq_file()

> Now is a good time to discuss a default behaviour change, since the
> '--override-char-to-quote' (=> OCTQ) option *never* worked, so users
> will not be surprised and no scripts need to be changed.

It worked until a year ago with release 1.1.15.

> Currently, and what I have read on the mailing-list, the OCTQ-option  
> must
> be provided ON EACH commandline issued when used initially to create  
> a backup
> mirror, also for restores. This is non-intuitive IMO, a more  
> sensible approach
> would be:
>
> 1. if no commandline OCTQ option is provided when a backup mirror is  
> created,
>   use FS auto-detected chars to quote (already doing so now)
> 2. if a backup mirror was started with OCTQ, this is assumed valid  
> unless
>   otherwise notice/overridden (default behaviour needs to be changed  
> for this,
>   but breaks nothing AFAICT)

After reading over the code, it looks like this is actually the case.  
Now that I think --override-chars-to-quote is fixed, please test it.

The relevant function is compare_ctq_file() in fs_abilities.py. In it,  
we see:

                ctq_rp = rbdir.append("chars_to_quote")
                if not ctq_rp.lstat():
                        ...
                        return (actual_ctq, None)

                if Globals.chars_to_quote is None: actual_ctq = 
ctq_rp.get_data()
                else: actual_ctq = Globals.chars_to_quote # Globals override

So, it definitely takes the data from the chars_to_quote file.


> 3. if the OCTQ really must be changed after creation of backup  
> mirror with
>   initial OCTQ set, coupling with '--force' option would be  
> naturally most
>   intuitive, otherwise shows error-message hinting about this
>   => so anybody knows this is a potential 'risky/deep' change, along  
> the line
>   of overwriting already existing directories when doing a RDB  
> restore, ...

I think a small logic change in that function would make this  
possible, since the requoting logic is already there. It's a good  
suggestion.


Andrew