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

[Andrew Bennetts]

The primary improvement in this patch is that it stops a smart server from
opening any branch not on its backing_transport.  This allows a client to
use a stacked branch on a smart server, even when the smart server can't
itself reach the stacked-on branch.  This also finally gives a way to write
tests that things work when the client can reach a fallback, but the server
can't (without a jail, in our current test environment all URLs that work
for the client work for the server).

The jail is per-thread state, and is set up and torn down by
SmartServerRequestHandler before calling in to the actual request
implementation.  To be paranoid we perhaps should make sure the jail state
propagates to new threads, although very little bzrlib code uses threads at
the moment.

This may cause some trouble for server-side plugins that actually do want to
access branches outside the jail, e.g. to automatically mirror a branch to
another location.  We should spend a bit of time making sure we have a
reasonable API for those plugins to use to do that (a API to extend the
jail, or circumvent it?).

In support of that, this patch also adds an ignore_fallbacks option to
BzrDir.open_branch (and BranchFormat.open, and so on).  This allows the
smart server methods that do need to open a branch to do so without
opening fallback branches that are forbidden (and unnecessary to the
implementation of the server request).  I think this is potentially useful
in other cases as well.

There are some outstanding issues:

 * Tests fail.  If there is a RemoteBzrDir with a branch reference in it,
   sprout on that bzrdir will attempt to open the branch in the server to
   determine the cloning_metadir, which may not be inside the jail.  As far
   as I can tell this isn't a problem for the command-line UI, which always
   calls sprout on open_branch(...).bzrdir, which will always be a
   fully-resolved branch.  But we have tests that expect that calling
   bzrdir.sprout works when bzrdir.get_branch_reference() != None.  The
   obvious trivial fixes fail (hacking sprout to check get_branch_reference
   first adds an extra Branch.get_stacked_on_url RPC, breaking our ratchet
   tests, for instance), but I'm sure there's a reasonably clean solution.
   I believe branch references on a server don't give great results in the
   real world anyway, e.g. if the reference points to a file:/// URL.

 * Robert would rather that open_branch didn't have a new flag.  Instead he
   proposes (Robert please jump in if I'm misrepresenting this in any way)
   that Branch itself should check if it's being opened while in a jail, and
   if so automatically skip opening the fallbacks.  So the server could
   continue to use plain bzrdir.open_branch().  I don't like the idea of a
   method giving a different type of result (a fully working branch vs. a
   partially working branch) based on global state; Robert doesn't like
   adding yet another flag to open_branch, and forcing people like plugin
   authors to decide if they need to open a fallback-enabled branch or not.
   If branches were more lazy about when they opened fallback locations,
   perhaps we'd both be happy, but implementing that would probably be
   non-trivial.

Robert and I would very much like to hear other people's opinions on the
explicit-flag vs. automatic-check-for-jail question, because we haven't yet
been able to convince each other.

I'd also like feedback on the patch in general; despite the open issues it's
actually quite close to being ready to land.

-Andrew.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzrdir-open-gaol-4168.patch
Type: text/x-diff
Size: 26805 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090320/8f5eb489/attachment-0001.bin