Rev 6117: (jelmer) Print a warning rather than an error when the current user does not in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Sep 1 06:35:35 UTC 2011


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6117 [merge]
revision-id: pqm at pqm.ubuntu.com-20110901063530-2opindd7ks84297p
parent: pqm at pqm.ubuntu.com-20110901042108-wkoypzpo5m5cbp6i
parent: v.ladeuil+lp at free.fr-20110830124000-8xf33kw0m67ekytv
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2011-09-01 06:35:30 +0000
message:
  (jelmer) Print a warning rather than an error when the current user does not
   have permission to read a configuration file. (Jelmer Vernooij)
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  doc/en/release-notes/bzr-2.5.txt bzr2.5.txt-20110708125756-587p0hpw7oke4h05-1
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-08-29 12:01:49 +0000
+++ b/bzrlib/config.py	2011-08-30 12:40:00 +0000
@@ -1397,7 +1397,7 @@
             return (self.branch._transport.get_bytes("email")
                     .decode(osutils.get_user_encoding())
                     .rstrip("\r\n"))
-        except errors.NoSuchFile, e:
+        except (errors.NoSuchFile, errors.PermissionDenied), e:
             pass
 
         return self._get_best_value('_get_user_id')
@@ -2278,6 +2278,11 @@
             return f
         except errors.NoSuchFile:
             return StringIO()
+        except errors.PermissionDenied, e:
+            trace.warning("Permission denied while trying to open "
+                "configuration file %s.", urlutils.unescape_for_display(
+                urlutils.join(self._transport.base, self._filename), "utf-8"))
+            return StringIO()
 
     def _external_url(self):
         return urlutils.join(self._transport.external_url(), self._filename)
@@ -2690,7 +2695,12 @@
         """Load the store from the associated file."""
         if self.is_loaded():
             return
-        content = self.transport.get_bytes(self.file_name)
+        try:
+            content = self.transport.get_bytes(self.file_name)
+        except errors.PermissionDenied:
+            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)
@@ -2738,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-29 12:01:49 +0000
+++ b/bzrlib/tests/test_config.py	2011-08-30 12:40:00 +0000
@@ -33,18 +33,14 @@
     errors,
     osutils,
     mail_client,
-    mergetools,
     ui,
     urlutils,
-    registry,
     remote,
     tests,
     trace,
-    transport,
     )
 from bzrlib.symbol_versioning import (
     deprecated_in,
-    deprecated_method,
     )
 from bzrlib.transport import remote as transport_remote
 from bzrlib.tests import (
@@ -1968,6 +1964,29 @@
         conf = config.TransportConfig(t, 'foo.conf')
         self.assertRaises(errors.ParseConfigError, conf._get_configobj)
 
+    def test_load_permission_denied(self):
+        """Ensure we get an empty config file if the file is inaccessible."""
+        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, "")
+
+        cfg = config.TransportConfig(
+            DenyingTransport("nonexisting://"), 'control.conf')
+        self.assertIs(None, cfg.get_option('non-existant', 'SECTION'))
+        self.assertEquals(
+            warnings,
+            [u'Permission denied while trying to open configuration file '
+             u'nonexisting:///control.conf.'])
+
     def test_get_value(self):
         """Test that retreiving a value from a section is possible"""
         bzrdir_config = config.TransportConfig(self.get_transport('.'),
@@ -2584,6 +2603,25 @@
         store = config.IniFileStore(t, 'foo.conf')
         self.assertRaises(errors.ParseConfigError, store.load)
 
+    def test_load_permission_denied(self):
+        """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)
+
+        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 load configuration store %s.'
+             % store.external_url()])
+
 
 class TestIniConfigContent(tests.TestCaseWithTransport):
     """Simulate loading a IniBasedConfig without content of various encodings.

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- a/doc/en/release-notes/bzr-2.5.txt	2011-08-31 11:08:14 +0000
+++ b/doc/en/release-notes/bzr-2.5.txt	2011-09-01 06:35:30 +0000
@@ -138,6 +138,10 @@
 * Decode ``BZR_HOME`` with fs encoding on posix platforms to avoid unicode
   errors.  (Vincent Ladeuil, #822571)
 
+* Rather than an error being raised, a warning is now printed when the
+  current user does not have permission to read a configuration file.
+  (Jelmer Vernooij, #837324)
+
 * TreeTransformBase.fixup_new_roots no longer forces trees to have a root, so
   operations that use it, like merge, can now create trees without a root.
   (Aaron Bentley)




More information about the bazaar-commits mailing list