launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/avoid-importing-bzr-plugins-from-site into lp:launchpad.

Commit message:
Suppress Branch security proxies in lp.codehosting, not lp_sitecustomize.

The latter runs too early to be able to safely import Bazaar plugins.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/avoid-importing-bzr-plugins-from-site/+merge/335379

Python doesn't set up sys.getfilesystemencoding() until shortly after it's finished importing site, so if we try to import Bazaar plugins from a sitecustomize hook then any initialisation calls they make to bzrlib.i18n.load_plugin_translations will fail, causing great confusion.  Anything that needs Bazaar plugins is already supposed to import lp.codehosting, so this should be a safe rearrangement.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/avoid-importing-bzr-plugins-from-site into lp:launchpad.

=== modified file 'lib/lp/codehosting/__init__.py'
--- lib/lp/codehosting/__init__.py	2017-09-27 02:12:20 +0000
+++ lib/lp/codehosting/__init__.py	2017-12-19 13:14:50 +0000
@@ -17,9 +17,11 @@
 import os
 
 import bzrlib
+from bzrlib.branch import Branch
 from bzrlib.plugin import load_plugins
 # This import is needed so that bzr's logger gets registered.
 import bzrlib.trace
+from zope.security import checker
 
 from lp.services.config import config
 
@@ -63,3 +65,16 @@
 
 
 load_bundled_plugin("weave_fmt")
+
+
+def dont_wrap_class_and_subclasses(cls):
+    checker.BasicTypes.update({cls: checker.NoProxy})
+    for subcls in cls.__subclasses__():
+        dont_wrap_class_and_subclasses(subcls)
+
+
+# Don't wrap Branch or its subclasses in Zope security proxies.  Make sure
+# the various LoomBranch classes are present first.
+import bzrlib.plugins.loom.branch
+bzrlib.plugins.loom.branch
+dont_wrap_class_and_subclasses(Branch)

=== modified file 'lib/lp_sitecustomize.py'
--- lib/lp_sitecustomize.py	2017-10-21 19:11:38 +0000
+++ lib/lp_sitecustomize.py	2017-12-19 13:14:50 +0000
@@ -8,8 +8,8 @@
 import itertools
 import logging
 import os
+import sys
 import warnings
-import sys
 
 from twisted.internet.defer import (
     Deferred,
@@ -127,23 +127,6 @@
     logging.getLogger('txn').addHandler(txn_handler)
 
 
-def dont_wrap_class_and_subclasses(cls):
-    checker.BasicTypes.update({cls: checker.NoProxy})
-    for subcls in cls.__subclasses__():
-        dont_wrap_class_and_subclasses(subcls)
-
-
-def dont_wrap_bzr_branch_classes():
-    from bzrlib.branch import Branch
-    # Load bzr plugins
-    import lp.codehosting
-    lp.codehosting
-    # Force LoomBranch classes to be listed as subclasses of Branch
-    import bzrlib.plugins.loom.branch
-    bzrlib.plugins.loom.branch
-    dont_wrap_class_and_subclasses(Branch)
-
-
 def silence_warnings():
     """Silence warnings across the entire Launchpad project."""
     # pycrypto-2.0.1 on Python2.6:
@@ -190,7 +173,6 @@
     customizeMimetypes()
     silence_warnings()
     customize_logger()
-    dont_wrap_bzr_branch_classes()
     checker.BasicTypes.update({defaultdict: checker.NoProxy})
     checker.BasicTypes.update({Deferred: checker.NoProxy})
     checker.BasicTypes.update({DeferredList: checker.NoProxy})