[MERGE][#291046] Fix pushing an unstackable branch + stackable repo when there is a default stacking policy.

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Andrew Bennetts wrote:
> Hi,
> 
> This patch fixes a bug when pushing a new stacked branch.  The bug causes a push
> to a newly created branch to trip over Martin's change to raise BzrCheckError
> when finishing a pack.
> 
> The specific issue is that if a source branch is not in a stackable format,
> *and* the source repo is in a stackable format, *and* the destination has a
> default stacking branch, then the repository is created with only the stacked
> data, but the new branch is still an unstacked branch.  This initial push
> succeeds, then the next push that modifies a file not modified in the stacked
> repository fails, because the compression parent is missing.
> 
> The correct fix I think is to behave as if the command-line had said --stacked
> --stacked-on=foo, i.e. upgrade the choice of branch format we create if stacking
> is requested.  So this patch does this.
> 
> It adds a branch_implementation test *and* a blackbox test.  These tests are
> essentially the same, so perhaps the blackbox one is redundant.  But for some
> reason I feel a bit nervous about this (the BzrDir.clone code is such a mess),
> so I left it in.  If you have any comments on appropriate tests for this, I'd be
> happy to hear them.  I also feel like the tests could be a little clearer, but
> it is a somewhat complex set of circumstances that is required to exercise this
> bug.
> 
> I also had to change BzrBranch (i.e. format 5) to not do a full history sync in
> _synchronize_history if the destination isn't also format 5.  This is because
> the target branch at that point *doesn't* have the fallback repositories set
> (even though it is format 7), so the histories won't match because the
> destination is mostly ghosts.  This in turn is because the destination branch
> has reopened the destination repository, rather than re-using the already
> constructed one made by the BzrDir earlier (which has the fallback set).  Fixing
> this would be great (as it will remove a wasted reconnection and other
> duplicated work), but is fairly involved.  The tweak to _synchronize_history was
> much simpler, and probably has positive performance implications anyway, even
> though it does fix this issue more by accident than design.
> 
> -Andrew.
> 
> 

BB:tweak


+    def require_stacking(self):
+        if not self.get_branch_format().supports_stacking():
+            # We need to make a stacked branch, but the default format
for the
+            # target doesn't support stacking.  So force a branch that
*can*
+            # support stacking.
+            from bzrlib.branch import BzrBranchFormat7
+            self._branch_format = BzrBranchFormat7()
+            mutter("using %r for stacking" % (self._branch_format,))
+            from bzrlib.repofmt import pack_repo
+            if self.repository_format.rich_root_data:
+                bzrdir_format_name = '1.6.1-rich-root'
+                repo_format = pack_repo.RepositoryFormatKnitPack5RichRoot()
+            else:
+                bzrdir_format_name = '1.6'
+                repo_format = pack_repo.RepositoryFormatKnitPack5()
+            note('Source format does not support stacking, using format:'
+                 ' \'%s\'\n  %s\n',
+                 bzrdir_format_name, repo_format.get_format_description())
+            self.repository_format = repo_format
+
+
     def get_converter(self, format=None):


^- you have some extra whitespace here.

I'm a bit concerned about automatically changing the repository format.
Specifically, if I have a 1.9 format repository, and I accidentally have
a Branch6 branch, (because 'bzr upgrade repo' doesn't upgrading all of
my branches), when I push this, it will actually downgrade the target to
a 1.6 format repository.


As *I* am planning to upgrade to 1.9 and start dogfooding launchpad as
soon as it supports it, I have the feeling this is going to explicitly
happen for me several times.

So I'd like a test dealing with a KnitPack6 format repository that shows
that it preserves the repository format as long as it supports stacking
already.

Otherwise I like the patch.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAklH9o4ACgkQJdeBCYSNAAOhgQCgtKQKm+ivUfGbpu6Ci8UdNG/p
NkkAoMuTFQ1cOLQMPEZ4TgJgKtTMTCYT
=hSJT
-----END PGP SIGNATURE-----