Rev 6115: Warn when loading, fail if saving will occur later in file:///home/vila/src/bzr/reviews/config-file-permdenied/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/reviews/config-file-permdenied/

------------------------------------------------------------
revno: 6115
revision-id: v.ladeuil+lp at free.fr-20110830124000-8xf33kw0m67ekytv
parent: jelmer at samba.org-20110830120807-ilnun1xbuilw0oj1
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-file-permdenied
timestamp: Tue 2011-08-30 14:40:00 +0200
message:
  Warn when loading, fail if saving will occur later
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-08-30 12:08:07 +0000
+++ b/bzrlib/config.py	2011-08-30 12:40:00 +0000
@@ -2698,16 +2698,12 @@
         try:
             content = self.transport.get_bytes(self.file_name)
         except errors.PermissionDenied:
-            trace.warning("Permission denied while trying to open "
-                "configuration file %s.", urlutils.unescape_for_display(
-                urlutils.join(self.transport.base, self.file_name),
-                "utf-8"))
-            # Ignore this file
-            self._load_from_string("")
-        else:
-            self._load_from_string(content)
-            for hook in ConfigHooks['load']:
-                hook(self)
+            trace.warning("Permission denied while trying to load "
+                          "configuration store %s.", self.external_url())
+            raise
+        self._load_from_string(content)
+        for hook in ConfigHooks['load']:
+            hook(self)
 
     def _load_from_string(self, bytes):
         """Create a config store from a string.
@@ -2752,8 +2748,8 @@
         # We need a loaded store
         try:
             self.load()
-        except errors.NoSuchFile:
-            # If the file doesn't exist, there is no sections
+        except (errors.NoSuchFile, errors.PermissionDenied):
+            # If the file can't be read, there is no sections
             return
         cobj = self._config_obj
         if cobj.scalars:

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-08-30 12:05:33 +0000
+++ b/bzrlib/tests/test_config.py	2011-08-30 12:40:00 +0000
@@ -2604,27 +2604,23 @@
         self.assertRaises(errors.ParseConfigError, store.load)
 
     def test_load_permission_denied(self):
-        """Ensure we get an empty config file if the file is inaccessible."""
+        """Ensure we get warned when trying to load an inaccessible file."""
         warnings = []
         def warning(*args):
             warnings.append(args[0] % args[1:])
         self.overrideAttr(trace, 'warning', warning)
 
-        class DenyingTransport(object):
-
-            def __init__(self, base):
-                self.base = base
-
-            def get_bytes(self, relpath):
-                raise errors.PermissionDenied(relpath, "")
-
-        store = config.IniFileStore(
-            DenyingTransport("nonexisting://"), 'control.conf')
-        store.load()
+        t = self.get_transport()
+
+        def get_bytes(relpath):
+            raise errors.PermissionDenied(relpath, "")
+        t.get_bytes = get_bytes
+        store = config.IniFileStore(t, 'foo.conf')
+        self.assertRaises(errors.PermissionDenied, store.load)
         self.assertEquals(
             warnings,
-            [u'Permission denied while trying to open configuration file '
-             u'nonexisting:///control.conf.'])
+            [u'Permission denied while trying to load configuration store %s.'
+             % store.external_url()])
 
 
 class TestIniConfigContent(tests.TestCaseWithTransport):