← Back to team overview

duplicity-team team mailing list archive

[Merge] lp:~aaron-whitehouse/duplicity/fix_stat_errors into lp:duplicity

 

Aaron Whitehouse has proposed merging lp:~aaron-whitehouse/duplicity/fix_stat_errors into lp:duplicity.

Commit message:
Only give an error about not being able to access possibly locked file if that file is supposed to be included or scanned (i.e. not excluded). Fixes Bug #1089131

Requested reviews:
  duplicity-team (duplicity-team)
Related bugs:
  Bug #1089131 in Duplicity: "Stat errors on locked (but excluded) files"
  https://bugs.launchpad.net/duplicity/+bug/1089131

For more details, see:
https://code.launchpad.net/~aaron-whitehouse/duplicity/fix_stat_errors/+merge/297141

Only give an error about not being able to access possibly locked file if that file is supposed to be included or scanned (i.e. not excluded). Fixes Bug #1089131

This was difficult to test properly with unit tests, as I couldn't figure out a good way to query the errors returned from a unit test -- they are currently only added to the log and console. Given that it is essentially a UI glitch anyway, I ended up just using this script:
http://paste.ubuntu.com/17273691/
to check that the expected output came up for each situation. Happy for any thoughts on how to do this more correctly.

Note that I considered making it a FatalError if the file is marked to be included (there would be too many false positives to do that for folders marked for scanning), but decided that was too big a change in behaviour.
-- 
Your team duplicity-team is requested to review the proposed merge of lp:~aaron-whitehouse/duplicity/fix_stat_errors into lp:duplicity.
=== modified file 'duplicity/selection.py'
--- duplicity/selection.py	2016-06-06 12:36:01 +0000
+++ duplicity/selection.py	2016-06-12 21:17:14 +0000
@@ -125,8 +125,7 @@
                              util.escape(fullpath))
             except OSError:
                 log.Warn(_("Error accessing possibly locked file %s") % util.ufn(fullpath),
-                         log.WarningCode.cannot_stat,
-                         util.escape(fullpath))
+                         log.WarningCode.cannot_stat, util.escape(fullpath))
             return None
 
         def diryield(path):
@@ -143,20 +142,24 @@
             for filename in robust.listpath(path):
                 new_path = robust.check_common_error(
                     error_handler, Path.append, (path, filename))
-                # make sure file is read accessible
-                if (new_path and new_path.type in ["reg", "dir"]
-                        and not os.access(new_path.name, os.R_OK)):
-                    log.Warn(_("Error accessing possibly locked file %s") % util.ufn(new_path.name),
-                             log.WarningCode.cannot_read,
-                             util.escape(new_path.name))
-                    if diffdir.stats:
-                        diffdir.stats.Errors += 1
-                    new_path = None
-                elif new_path:
+                if new_path:
                     s = self.Select(new_path)
-                    if s == 1:
+                    if (new_path.type in ["reg", "dir"] and
+                        not os.access(new_path.name, os.R_OK)) and \
+                            (s == 1 or s == 2):
+                        # Path is a file or folder that cannot be read, but
+                        # should be included or scanned.
+                        log.Error(_("Error accessing possibly locked file %s") %
+                                  util.ufn(new_path.name),
+                                  log.WarningCode.cannot_read,
+                                  util.escape(new_path.name))
+                        if diffdir.stats:
+                            diffdir.stats.Errors += 1
+                    elif s == 1:
+                        # Should be included
                         yield (new_path, 0)
                     elif s == 2 and new_path.isdir():
+                        # Is a directory that should be scanned
                         yield (new_path, 1)
 
         if not path.type:


Follow ups