[MERGE] Reduce round trips pushing new branches substantially.

[Robert Collins]

On Mon, 2009-04-27 at 13:40 +1000, Andrew Bennetts wrote:

> > 
> > huh? This code handles that precise combination:
> > We get a JailBreak because we're trying to stack. We're in the server,
> > and thus not actually ever able to _do_ stacking, so we stop. The client
> > configures their proxy RemoteRepository as stacked, and its all good.
> 
> There's “trying to stack” and there's “must stack”.  You're handling the
> “trying to stack” case just fine.  I'm talking about the “must stack” case,
> i.e. self._require_stacking=True.  If some caller (perhaps a plugin, perhaps
> a later change bzrlib) does specify require_stacking=True and encounter a
> JailBreak, then I think silently returning here would be an error.  I don't
> think any such callers exist currently, but that doesn't mean there's no bug
> here.
> 
> So, I expect that if you change the code to be (omitting comments):
> 
>     except errors.JailBreak:
>         if self._require_stacking:
>             raise
>         return
> 
> Then our existing tests will still pass because the server doesn't pass
> require_stacking=True to the repository acquisition policy, but it fixes the
> bug I'm talking about.

So, broadly 'there is something that might break and we don't test'. 

:). I'd like to leave it as is because:
 - it passes automatic and manual tests
 - your analysis might be wrong
 - we can't tell either way without getting deep into that code, and if
   so I think its new development. A good time to do that would be when
   looking at the 'how do I force stacking off' bug I think.

> > > How many places do we hard-code the current default repository formats?  It
> > > seems likely that we next update the default format we'll miss places like
> > > this... why not use 'default'/'default-rich-root' from the format_registry?
> > 
> > They aren't stackable.
> 
> Ugh, that's right.  How about a module-global function in bzrdir.py?  e.g.
> 
>     def get_default_stacking_format(rich_root):
>         if rich_root:
>             repo_format = pack_repo.RepositoryFormatKnitPack6RichRoot()
>         else:
>             repo_format = pack_repo.RepositoryFormatKnitPack6()
> 	branch_format = branch.BzrBranchFormat7()
>         return branch_format, repo_format
> 
> And then update BzrDirMetaFormat1.require_stacking to use it too — it has
> the same basic logic as yours, but uses RepositoryFormatKnitPack5* not 6.
> It seems wrong to let these places get out of sync.

Hmm, yes. How about a follow up branch? I'm betting there will be test
fallout.

-Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090427/5a82845b/attachment.pgp