Rev 5412: (mbp) Add check in lazy import ScopeReplacer to ensure it's not trying to in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

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

------------------------------------------------------------
revno: 5412 [merge]
revision-id: pqm at pqm.ubuntu.com-20100908133517-0cuy5kiuvb3dhtyx
parent: pqm at pqm.ubuntu.com-20100908110922-fki7swxcw814v7u0
parent: gzlist at googlemail.com-20100905171546-3ggqhogy23fgoxc8
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2010-09-08 14:35:17 +0100
message:
  (mbp) Add check in lazy import ScopeReplacer to ensure it's not trying to
   replace itself (Martin [gz])
modified:
  bzrlib/lazy_import.py          lazy_import.py-20060910203832-f77c54gf3n232za0-1
  bzrlib/tests/test_lazy_import.py test_lazy_import.py-20060910203832-f77c54gf3n232za0-2
=== modified file 'bzrlib/lazy_import.py'
--- a/bzrlib/lazy_import.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/lazy_import.py	2010-09-05 17:15:46 +0000
@@ -87,6 +87,9 @@
                           " to another variable?",
                 extra=e)
         obj = factory(self, scope, name)
+        if obj is self:
+            raise errors.IllegalUseOfScopeReplacer(name, msg="Object tried"
+                " to replace itself, check it's not using its own scope.")
         if ScopeReplacer._should_proxy:
             object.__setattr__(self, '_real_obj', obj)
         scope[name] = obj

=== modified file 'bzrlib/tests/test_lazy_import.py'
--- a/bzrlib/tests/test_lazy_import.py	2010-01-25 15:55:48 +0000
+++ b/bzrlib/tests/test_lazy_import.py	2010-09-05 17:15:46 +0000
@@ -127,13 +127,9 @@
         else:
             self.fail('test_obj1 was not supposed to exist yet')
 
-        orig_globals = set(globals().keys())
-
         InstrumentedReplacer(scope=globals(), name='test_obj1',
                              factory=factory)
 
-        new_globals = set(globals().keys())
-
         # We can't use isinstance() because that uses test_obj1.__class__
         # and that goes through __getattribute__ which would activate
         # the replacement
@@ -168,13 +164,9 @@
         else:
             self.fail('test_obj6 was not supposed to exist yet')
 
-        orig_globals = set(globals().keys())
-
         lazy_import.ScopeReplacer(scope=globals(), name='test_obj6',
                                   factory=factory)
 
-        new_globals = set(globals().keys())
-
         # We can't use isinstance() because that uses test_obj6.__class__
         # and that goes through __getattribute__ which would activate
         # the replacement
@@ -440,6 +432,36 @@
                           ('foo', 4),
                          ], actions)
 
+    def test_replacing_from_own_scope_fails(self):
+        """If a ScopeReplacer tries to replace itself a nice error is given"""
+        actions = []
+        InstrumentedReplacer.use_actions(actions)
+        TestClass.use_actions(actions)
+
+        def factory(replacer, scope, name):
+            actions.append('factory')
+            # return the name in given scope, which is currently the replacer
+            return scope[name]
+
+        try:
+            test_obj7
+        except NameError:
+            # test_obj7 shouldn't exist yet
+            pass
+        else:
+            self.fail('test_obj7 was not supposed to exist yet')
+
+        InstrumentedReplacer(scope=globals(), name='test_obj7',
+                             factory=factory)
+
+        self.assertEqual(InstrumentedReplacer,
+                         object.__getattribute__(test_obj7, '__class__'))
+        e = self.assertRaises(errors.IllegalUseOfScopeReplacer, test_obj7)
+        self.assertIn("replace itself", e.msg)
+        self.assertEqual([('__call__', (), {}),
+                          '_replace',
+                          'factory'], actions)
+
 
 class ImportReplacerHelper(TestCaseInTempDir):
     """Test the ability to have a lazily imported module or object"""