[MERGE][#251871] Use source_branch.sprout in BzrDir.sprout when possible.
John Arbash Meinel
john at arbash-meinel.com
Thu Jul 31 21:05:22 BST 2008
-----BEGIN PGP SIGNED MESSAGE-----
Andrew Bennetts wrote:
| This patch fixes bzr.dev so that branching with the bzr-svn plugin
| latest tip of the stable branch) works again. When sprouting from an
| calling source._format.initialise(result) was failing because
| simply raises NotImplementedError, which is somewhat reasonable as
| cannot create them at arbitrary locations outside an SVN repository.
| IncompatibleFormat or UninitializableFormat would be a more
| for bzr-svn to raise, but it doesn't really matter.)
| Basically, this patch just reverts BzrDir.sprout to calling
| when it can, which is how things used to work.
| There's a bit of tension in BzrDir.sprout about which format to use
| result_branch. Generally it wants to preserve the source format, except:
| * sometimes we require a stacking-compatible format, even if the
| stacking compatible.
| * sometimes we might be cloning from a bzrdir with no branch
| in the code speculate that this may be a dead code path...)
I know we have an open bug for someone who wants to:
~ bzr branch -r revid:XXX repo repo/new-branch
It works if you are doing:
~ bzr branch -r revid:XXX repo/random_branch repo/new-branch
I agree, though, that we don't really know what to use in that case.
| * sometimes, as with bzr-svn, the source format isn't actually
| This could happen with very old bzr formats too I suppose (those
that are no
| longer supported for writing, just reading).
| This patch preserves the existing logic for the force-stacking and
| no-source-branch cases: they are special-cased and don't call
| source_branch.sprout. The general case now relies on
| make an appropriate choice of format, which is how bzr-svn decides to
| rich-root-pack branches for SvnBranches.
| I'm pretty sure we'll be touching this code again at some point in the
| For instance the duplication of Branch.sprout logic later in the
| a bit (as the comment acknowledges). But this seems like a simple and
| change for now, even if it doesn't get rid of the underlying
messiness. It's a
| bit late in the release cycle for deep changes anyway...
I wonder about passing the stacking information into Branch.sprout . It
might be a nice way to have rich-root-pack create a 1.6-rich-root
target, rather than the default format.
| This patch currently lacks a test: perhaps an appropriate test would
| sprouting a bzrdir works when the a source branch has:
| * an uninitialisable format
| * a working sprout method
| I'm have a niggling feeling that's too specific, but perhaps it's fine.
| I've tested this branch quite a bit manually. With the attached
| this small patch to bzr-svn I can branch and branch --stacked direct
from an SVN
| repo and SVN checkouts:
| === modified file 'workingtree.py'
| --- workingtree.py 2008-07-23 16:06:27 +0000
| +++ workingtree.py 2008-07-31 06:31:24 +0000
| @@ -21,7 +21,7 @@
| from bzrlib.bzrdir import BzrDirFormat, BzrDir
| from bzrlib.errors import (InvalidRevisionId, NotBranchError, NoSuchFile,
| NoRepositoryPresent, BzrError,
| - OutOfDateTree)
| + OutOfDateTree, NoSuchRevisionInTree)
| from bzrlib.inventory import Inventory, InventoryFile, InventoryLink
| from bzrlib.lockable_files import TransportLock, LockableFiles
| from bzrlib.lockdir import LockDir
| @@ -581,6 +581,9 @@
| return self.base_tree
| + def revision_tree(self, revision_id):
| + raise NoSuchRevisionInTree(self, revision_id)
| That patch fixes a traceback when branching from a checkout.
| (Btw, it appears that "bzr info -v" inside a branch stacked on an
| fails, not sure yet if it's a bzr-svn issue or a bzr issue.)
Well, sounds like a bug in bzr-svn, as WorkingTree3 already has a proper
implementation of 'revision_tree':
def revision_tree(self, revision_id):
~ """See Tree.revision_tree.
~ WorkingTree can supply revision_trees for the basis revision only
~ because there is only one cached inventory in the bzr directory.
~ if revision_id == self.last_revision():
~ xml = self.read_basis_inventory()
~ except errors.NoSuchFile:
~ inv = xml7.serializer_v7.read_inventory_from_string(xml)
~ # dont use the repository revision_tree api because we want
~ # to supply the inventory.
~ if inv.revision_id == revision_id:
~ inv, revision_id)
~ except errors.BadInventoryFormat:
~ # raise if there was no inventory, or if we read the wrong inventory.
~ raise errors.NoSuchRevisionInTree(self, revision_id)
Basically, if we have cached the xml, we don't need to extract it from
the repository, which *did* make some things a lot faster.
WorkingTree4 has a newer form which can use the dirstate itself for
this, and doing so enables the faster code-path for 'iter_changes()'.
(Though we also have basis_tree() to do this.)
I'm happy with the patch, I would just like to see a test for it if
I think adding a test for a Branch.sprout() that creates a different
branch format (this could be hacked together in the test suite) would be
a reasonable way to test that BzrDir.sprout() is calling the right thing
when it can.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar