rdiff-backup-users
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Oliver Mulatz
Subject: Re: [rdiff-backup-users] --override-chars-to-quote bug fixed and default behaviour change
Date: Wed, 07 Jan 2009 16:49:53 +0100
User-agent: Thunderbird 2.0.0.x (X11) Mnenhy/0.7.6.0

Andrew Ferguson schrieb:

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()

no, sorry to have replicated work
I did not send it yet since it was only a single line added
(besides using other code-comment, our fix is identical), I'd wanted
to ask about/impl. the behaviour change before submitting

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.

I see.

[...]

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.

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.

just finished (+ testing) it, still warm :)
The actual re-quoting of a complete archive I tackle in the next days.

<code>
diff U3 fs_abilities.py fs_abilities.py
--- fs_abilities.py     Sun Jan 04 00:36:20 2009
+++ fs_abilities.py     Wed Jan 07 16:20:38 2009
@@ -556,7 +556,7 @@
        def set_escape_dos_devices(self, subdir):
                """Test if DOS device files can be used as filenames.

-               This test must detect if the underlying OS is Windows, whehter 
we are
+               This test must detect if the underlying OS is Windows, whether 
we are
                running under Cygwin or natively. Cygwin allows these special 
files to
                be stat'd from any directory. Native Windows returns OSError 
(like
                non-Cygwin POSIX), but we can check for that using os.name.
@@ -805,37 +805,57 @@
                return "".join(ctq)

        def compare_ctq_file(self, rbdir, suggested_ctq, force):
-               """Compare ctq file with suggested result, return actual ctq"""
+               """Compares ctq file with commandline-provided overrides with
+               filesystem auto-deleted suggested chars, returns actual ctq"""
                ctq_rp = rbdir.append("chars_to_quote")
+               stored_ctq = None;
+               log.Log("characters-to-quote: ctq_rp: '%s'" % ctq_rp, 4) ## OM: 
temp debug only (line to be stripped later)
+               
                if not ctq_rp.lstat():
                        if Globals.chars_to_quote is None: actual_ctq = 
suggested_ctq
                        else: actual_ctq = Globals.chars_to_quote
                        ctq_rp.write_string(actual_ctq)
+                       log.Log("characters-to-quote: ctq_rp WAS NOT existing, 
created initial CTQ-file written: '%s'" % actual_ctq, 4) ## OM: temp debug only 
(line to be stripped later)
                        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
+               else:
+                       log.Log("characters-to-quote: ctq_rp file IS existing", 
4) ## OM: temp debug only (line to be stripped later)
+               
+                       if Globals.chars_to_quote is None:
+                               actual_ctq = ctq_rp.get_data()
+                               log.Log("characters-to-quote: 
Globals.chars_to_quote' IS None!\n Using existing CTQ-rp: '%s'" % actual_ctq, 4) ## 
OM: temp debug only (line to be stripped later)
+                       else:
+                               stored_ctq = ctq_rp.get_data()
+                               actual_ctq = Globals.chars_to_quote # Globals 
override
+                               log.Log("characters-to-quote: 'actual_ctq' is 
assigned content of Globals.chars_to_quote (passed via CLI option): '%s'" % 
actual_ctq, 4) ## OM: temp debug only (line to be stripped later)

                if actual_ctq == suggested_ctq: return (actual_ctq, None)
+               log.Log("characters-to-quote: actual_ctq != suggested_ctq", 4) 
## OM: temp debug only (line to be stripped later)
+               
                if suggested_ctq == "":
                        log.Log("Warning: File system no longer needs quoting, "
                                        "but we will retain for backwards 
compatibility.", 2)
                        return (actual_ctq, None)
-               if Globals.chars_to_quote is None:
-                       if force:
-                               log.Log("Warning: migrating rdiff-backup repository 
from"
-                                               "old quoting chars %r to new quoting 
chars %r" %
-                                               (actual_ctq, suggested_ctq), 2)
-                               ctq_rp.delete()
-                               ctq_rp.write_string(suggested_ctq)
-                               return  (suggested_ctq, 1)
-                       else:
-                               log.Log.FatalError("""New quoting requirements!
+
+               log.Log("characters-to-quote: suggested_ctq is NOT empty", 4) 
## OM: temp debug only (line to be stripped later)
+                       
+               if Globals.chars_to_quote is not None: suggested_ctq = 
Globals.chars_to_quote
+               if stored_ctq is None: stored_ctq = actual_ctq
+
+               if force:
+                       log.Log("Warning: migrating rdiff-backup repository from 
"
+                                       "old quoting chars %r to new quoting chars 
%r" %
+                                       (stored_ctq, suggested_ctq), 2)
+                       ctq_rp.delete()
+                       ctq_rp.write_string(suggested_ctq)
+                       log.Log("characters-to-quote: Wrote '--force'd new quoting 
requirements, returning with new/updated 'suggested_ctq'", 4)  ## OM: internal debug 
only (line to be stripped)
+                       return  (suggested_ctq, 1)
+               else:
+                       log.Log.FatalError("""New quoting requirements!

 The quoting chars this session needs %r do not match
 the repository settings %r listed in

-%s
+"%s"

 This may be caused when you copy an rdiff-backup repository from a
 normal file system onto a windows one that cannot support the same
@@ -843,10 +863,11 @@
 case-insensitive one that previously only had case-insensitive ones
 backed up onto it.

-By specificying the --force option, rdiff-backup will migrate the
+By specifying the --force option, rdiff-backup will migrate the
 repository from the old quoting chars to the new ones.""" %
-                       (suggested_ctq, actual_ctq, ctq_rp.path))
-
+                       (suggested_ctq, stored_ctq, ctq_rp.path))
+## OM: Not needed anymore, dealt with above.
+##             return (actual_ctq, None) # Maintain Globals override

 class RestoreSetGlobals(SetGlobals):
        """Functions for setting fsa-related globals for restore session"""
diff U3 Main.py Main.py
--- Main.py     Sun Jan 04 00:36:20 2009
+++ Main.py     Wed Jan 07 13:07:13 2009
@@ -199,7 +199,7 @@
                elif opt == "--tempdir": tempfile.tempdir = arg
                elif opt == "--terminal-verbosity": Log.setterm_verbosity(arg)
                elif opt == "--test-server": action = "test-server"
-               elif opt == "use-compatible-timestamps":
+               elif opt == "--use-compatible-timestamps":
                        Globals.set("use_compatible_timestamps", 1)
                elif opt == "--user-mapping-file": user_mapping_filename = arg
                elif opt == "-v" or opt == "--verbosity": Log.setverbosity(arg)
diff U3 robust.py robust.py
--- robust.py   Sun Jan 04 00:36:20 2009
+++ robust.py   Wed Jan 07 13:15:38 2009
@@ -74,7 +74,7 @@
                return "Lost connection to the remote system"
        elif isinstance(exc, SignalException):
                return "Killed with signal %s" % (exc,)
-       elif isinstance(exc, EnvironmentError) and e.errno == errno.ENOTCONN:
+       elif isinstance(exc, EnvironmentError) and exc.errno == errno.ENOTCONN:
                return ("Filesystem reports connection failure:\n%s" % exc)
        return None
</code>

Cheers,
Oliver






reply via email to

[Prev in Thread] Current Thread [Next in Thread]