Remove MetadataError and make apply error handling finer-grained.

Handle errors that were handled by a blanket MetadataApplyError catch
more selectively.  Rename MetadataApplyError to ApplyError.  Throw an
ApplyError when encountering EACCES in order to skip the (bad) path.
Call add_error() for lchown EPERM.  Allow other exceptions to pass
through.

Catch ApplyError when calling apply_to_path() from extract() and
finish_extract() and redirect the exception to add_error() --
i.e. skip the path.

Add bup.helpers.clear_errors(), and use it in test cases that involve
calls to add_error().

diff --git a/lib/bup/helpers.py b/lib/bup/helpers.py
index 71b0f9a11dab6d3eb9dabe3bbbdb582f0e66a217..2e9d6f1f16d909e4537beabb60a474fb506df4c4 100644 (file)
--- a/lib/bup/helpers.py
+++ b/lib/bup/helpers.py
@@ -355,6 +355,12 @@ def add_error(e):
     saved_errors.append(e)
     log('%-70s\n' % e)
 
+
+def clear_errors():
+    global saved_errors
+    saved_errors = []
+
+
 istty = os.isatty(2) or atoi(os.environ.get('BUP_FORCE_TTY'))
 def progress(s):
     """Calls log(s) if stderr is a TTY.  Does nothing otherwise."""

diff --git a/lib/bup/metadata.py b/lib/bup/metadata.py
index 2e599c969e4c97634b897cb3d1920cfccab17306..798c1156966abdd267f4ca77b01a9cd2faf1f48f 100644 (file)
--- a/lib/bup/metadata.py
+++ b/lib/bup/metadata.py
@@ -146,11 +146,7 @@ _rec_tag_linux_attr = 6       # lsattr(1) chattr(1)
 _rec_tag_linux_xattr = 7      # getfattr(1) setfattr(1)
 
 
-class MetadataError(Exception):
-    pass
-
-
-class MetadataApplyError(MetadataError):
+class ApplyError(Exception):
     # Thrown when unable to apply any given bit of metadata to a path.
     pass
 
@@ -259,10 +255,24 @@ class Metadata:
 
     def _apply_common_rec(self, path, restore_numeric_ids=False):
         # FIXME: S_ISDOOR, S_IFMPB, S_IFCMP, S_IFNWK, ... see stat(2).
+        # EACCES errors at this stage are fatal for the current path.
         if stat.S_ISLNK(self.mode):
-            lutime(path, (self.atime, self.mtime))
+            try:
+                lutime(path, (self.atime, self.mtime))
+            except IOError, e:
+                if e.errno == errno.EACCES:
+                    raise ApplyError('lutime: %s' % e)
+                else:
+                    raise
         else:
-            utime(path, (self.atime, self.mtime))
+            try:
+                utime(path, (self.atime, self.mtime))
+            except IOError, e:
+                if e.errno == errno.EACCES:
+                    raise ApplyError('utime: %s' % e)
+                else:
+                    raise
+
         if stat.S_ISREG(self.mode) \
                 | stat.S_ISDIR(self.mode) \
                 | stat.S_ISCHR(self.mode) \
@@ -304,7 +314,14 @@ class Metadata:
                         gid = -1
                         add_error('bup: ignoring unknown group %s for "%s"\n'
                                   % (self.group, path))
-            os.lchown(path, uid, gid)
+
+            try:
+                os.lchown(path, uid, gid)
+            except OSError, e:
+                if e.errno == errno.EPERM:
+                    add_error('lchown: %s' %  e)
+                else:
+                    raise
 
             if _have_lchmod:
                 os.lchmod(path, stat.S_IMODE(self.mode))
@@ -528,13 +545,13 @@ class Metadata:
         if not path:
             raise Exception('Metadata.apply_to_path() called with no path');
         num_ids = restore_numeric_ids
-        try: # Later we may want to push this down and make it finer grained.
+        try:
             self._apply_common_rec(path, restore_numeric_ids=num_ids)
             self._apply_posix1e_acl_rec(path, restore_numeric_ids=num_ids)
             self._apply_linux_attr_rec(path, restore_numeric_ids=num_ids)
             self._apply_linux_xattr_rec(path, restore_numeric_ids=num_ids)
-        except Exception, e:
-            raise MetadataApplyError(e), None, sys.exc_info()[2]
+        except ApplyError, e:
+            add_error(e)
 
 
 def from_path(path, archive_path=None, save_symlinks=True):
@@ -636,23 +653,15 @@ def finish_extract(file, restore_numeric_ids=False):
             else:
                 if verbose:
                     print >> sys.stderr, meta.path
-                try:
-                    meta.apply_to_path(path=xpath,
-                                       restore_numeric_ids=restore_numeric_ids)
-                except MetadataApplyError, e:
-                    add_error(e)
-
+                meta.apply_to_path(path=xpath,
+                                   restore_numeric_ids=restore_numeric_ids)
     all_dirs.sort(key = lambda x : len(x.path), reverse=True)
     for dir in all_dirs:
         # Don't need to check xpath -- won't be in all_dirs if not OK.
         xpath = _clean_up_extract_path(dir.path)
         if verbose:
             print >> sys.stderr, dir.path
-        try:
-            dir.apply_to_path(path=xpath,
-                              restore_numeric_ids=restore_numeric_ids)
-        except MetadataApplyError, e:
-            add_error(e)
+        dir.apply_to_path(path=xpath, restore_numeric_ids=restore_numeric_ids)
 
 
 def extract(file, restore_numeric_ids=False, create_symlinks=True):
@@ -673,10 +682,7 @@ def extract(file, restore_numeric_ids=False, create_symlinks=True):
             else:
                 if verbose:
                     print >> sys.stderr, '=', meta.path
-                try:
-                    meta.apply_to_path(restore_numeric_ids=restore_numeric_ids)
-                except MetadataApplyError, e:
-                    add_error(e)
+                meta.apply_to_path(restore_numeric_ids=restore_numeric_ids)
     all_dirs.sort(key = lambda x : len(x.path), reverse=True)
     for dir in all_dirs:
         # Don't need to check xpath -- won't be in all_dirs if not OK.
@@ -684,8 +690,5 @@ def extract(file, restore_numeric_ids=False, create_symlinks=True):
         if verbose:
             print >> sys.stderr, '=', meta.path
         # Shouldn't have to check for risky paths here (omitted above).
-        try:
-            dir.apply_to_path(path=dir.path,
-                              restore_numeric_ids=restore_numeric_ids)
-        except MetadataApplyError, e:
-            add_error(e)
+        dir.apply_to_path(path=dir.path,
+                          restore_numeric_ids=restore_numeric_ids)

diff --git a/lib/bup/t/tmetadata.py b/lib/bup/t/tmetadata.py
index a60dd67329e637614749af61b3e3446cb67889e5..dadd6eed757b417b4ce15342e4574be557b8d72c 100644 (file)
--- a/lib/bup/t/tmetadata.py
+++ b/lib/bup/t/tmetadata.py
@@ -3,8 +3,9 @@ import pwd
 import stat
 import subprocess
 import tempfile
+import bup.helpers as helpers
 from bup import metadata
-from bup.helpers import detect_fakeroot, saved_errors
+from bup.helpers import clear_errors, detect_fakeroot
 from wvtest import *
 
 
@@ -86,26 +87,29 @@ def test_from_path_error():
         WVPASSEQ(m.path, path)
         subprocess.call(['chmod', '000', path])
         metadata.from_path(path, archive_path=path, save_symlinks=True)
-        WVPASS(saved_errors
-               and saved_errors[0].startswith('bup: unable to read Linux attr'))
+        errmsg = helpers.saved_errors[0] if helpers.saved_errors else ''
+        WVPASS(errmsg.startswith('bup: unable to read Linux attr'))
+        clear_errors()
     finally:
         subprocess.call(['rm', '-rf', tmpdir])
 
 
 @wvtest
-def test_apply_to_path_error():
+def test_apply_to_path_restricted_access():
     if os.geteuid == 0 or detect_fakeroot():
         return
     tmpdir = tempfile.mkdtemp(prefix='bup-tmetadata-')
     try:
         path = tmpdir + '/foo'
         subprocess.call(['mkdir', path])
+        clear_errors()
         m = metadata.from_path(path, archive_path=path, save_symlinks=True)
         WVPASSEQ(m.path, path)
         subprocess.call(['chmod', '000', tmpdir])
-        WVEXCEPT(metadata.MetadataApplyError,
-                 m.apply_to_path, path)
-        subprocess.call(['chmod', '700', tmpdir])
+        m.apply_to_path(path)
+        errmsg = str(helpers.saved_errors[0]) if helpers.saved_errors else ''
+        WVPASS(errmsg.startswith('utime: '))
+        clear_errors()
     finally:
         subprocess.call(['rm', '-rf', tmpdir])
 
@@ -123,12 +127,16 @@ def test_restore_restricted_user_group():
         WVPASSEQ(m.apply_to_path(path), None)
         orig_uid = m.uid
         m.uid = 0;
-        WVEXCEPT(metadata.MetadataApplyError,
-                 m.apply_to_path, path, restore_numeric_ids=True)
+        m.apply_to_path(path, restore_numeric_ids=True)
+        errmsg = str(helpers.saved_errors[0]) if helpers.saved_errors else ''
+        WVPASS(errmsg.startswith('lchown: '))
+        clear_errors()
         m.uid = orig_uid
         m.gid = 0;
-        WVEXCEPT(metadata.MetadataApplyError,
-                 m.apply_to_path, path, restore_numeric_ids=True)
+        m.apply_to_path(path, restore_numeric_ids=True)
+        errmsg = str(helpers.saved_errors[0]) if helpers.saved_errors else ''
+        WVPASS(errmsg.startswith('lchown: '))
+        clear_errors()
     finally:
         subprocess.call(['rm', '-rf', tmpdir])