[MERGE/RFC] Add transport jail to bzr smart server; add ignore_fallbacks option to BzrDir.open_branch

[Vincent Ladeuil]

Sorry for not following that thread more closely, I may lack
context, but still, I encounter a problem.

>>>>> "Andrew" == Andrew Bennetts <andrew.bennetts at canonical.com> writes:

<snip/>

    Andrew> It does, thanks.  The patch has now landed without
    Andrew> magic (but Robert's other tweaks done).  I'm sure
    Andrew> Robert and I will both be keeping a close eye on how
    Andrew> it works out in future server-side bzrlib code and in
    Andrew> plugins.

I lack time to investigate the why, but the following seems
required or 

  bzr selftest -s bp.xmloutput.tests.test_service.TestXmlRpcServer

blows up in the most obscure way for the poor plugin author or me
as a bzr log related patch reviewer wanting to check the patch
against a given plugin in that case.

As I understand it, this could happen as soon as anybody try to
open a bzrdir while in a thread (under the assumption that the
bzrlib.smart.request has been imported by the main thread and not
by the newly created one...

=== modified file 'bzrlib/smart/request.py'
--- bzrlib/smart/request.py	2009-03-24 06:40:26 +0000
+++ bzrlib/smart/request.py	2009-03-25 11:22:38 +0000
@@ -58,7 +58,10 @@
 
 
 def _pre_open_hook(transport):
-    allowed_transports = jail_info.transports
+    try:
+        allowed_transports = jail_info.transports
+    except AttributeError:
+        allowed_transports = None
     if allowed_transports is None:
         return
     abspath = transport.base


Looking from 10.000 feet, I also find the bare '_install_hook()'
call at the module init time in bzrlib.smart.request choking.

The comment in TestCase._clear_hooks() where I went (immediately
of course :) 'this hook should always be installed' didn't
explain *why* either.

Without going further, my feeling is that this hook shouldn't be
installed unconditionally, the tests shouldn't have to reinstall
it either.

It seems to break the test isolation *on purpose* !

It also smells a lot like global state and I thought we hated
that...

I have a note in my TODO list to look on how to test hooks
easily, is it what is missing here ?

    Vincent