[MERGE][bug 248932] reconfigure --standalone creates incompatibe repository

[Aaron Bentley]

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

bb:approve

I think this is an improvement with no drawbacks.

Martin von Gagern wrote:
> The attached patch contains a blackbox test and a solution to the
> problem.

Why?

Blackbox tests aren't meant to be thorough, and they're slower than unit
tests.  I would expect a blackbox test to demonstrate that "reconfigure
- --tree" works at all, and a unit test to ensure that Reconfigure.apply()
creates a repository in the correct format.

Your patch changes Reconfigure.apply, but doesn't add tests to prevent
reconfigure.apply from regressing.

> Lifeless there gave me some code snippet to create a repository of a
> given format. With that, I have to access Repository._format as the only
> "private" (i.e. starting with an underscore) variable, and lifeless said
> that's OK as long as it's in bzrlib and gets upgraded as needed.

If I were coding it, I'd try to set self.bzrdir's repository format
correctly, so that self.bzrdir.create_respoitory could be used
unconditionally.  It seems we need a hybrid between BzrDir.__init__ and
BzrDir.make_cloning_metadir.

> -            repo = self.bzrdir.create_repository()
> +            if self.local_branch and not self._destroy_branch:
> +                old_repo = self.local_branch.repository

You are not using self.local_repository here, so in cases where there is
no branch, this will not do the right thing.

Really, though, I think self.repository is The Right Thing.  If there is
a local branch, it will be correct.  If there's no local branch, it will
still be correct.

> +            elif self._create_branch and self.referenced_branch is not None:
> +                old_repo = self.referenced_branch.repository

Similarly here, I don't understand why the condition's on
_create_branch, not _create_repository.  In fact, really, all of this
can be conditional on _create_repository.

If we're deleting a branch reference, then the repository we create
should match the deleted reference's format.  Otherwise, we should match
the existing repository format.

So I would probably do something like:

if self._destroy_reference:
    (self.bzrdir._repository_format =
     self.referenced_branch.repository._format)
elif self._repository is not None:
    self.bzrdir._repository_format = self.repository._format

And then the self.bzrdir.create_repository could be used as-is.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkn2Bw4ACgkQ0F+nu1YWqI2ZvQCcDKR7Qsp0rzZHhIVx2Y5pXOuj
6WEAnjqBtI52QSCJSySqNgx53UrCgjzP
=G2Nn
-----END PGP SIGNATURE-----