[MERGE] test_sprout_uses_bzrdir_branch_format

[John Arbash Meinel]

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

Robert Collins wrote:
> On Tue, 2008-11-18 at 13:01 -0600, John Arbash Meinel wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> So it ended up taking more to fix than I thought it would, so I'm
>> posting my updated version for review.
>>
>> 1) Almost none of the BranchFormat objects was setting the branch_format
>> attribute in its _matchingbzrdir. This adds a test for all of the
>> permutations being tested that they do.
>>
>> Andrew and I had already done some of this, but I think he forgot to
>> include it when he split this patch of to be approved (the tests were
>> failing at least.)
>>
>> 2) It also turned out that the RemoteBranch tests were also not using a
>> matching bzrdir. They were using RemoteBzrDir() directly, but again,
>> self._matchingbzrdir.get_branch_format() would then return the default
>> format which is Branch6. Perhaps a better fix would be to hard-code
>> RemoteBzrDirFormat.get_branch_format() to return RemoteBranchFormat?
> 
> This seems to change what used to be 'preserve branch kind' to be 'use
> the kind of the target'. This is good and bad - it's good that it will
> allow 'seamless' upgrades, but its bad that it will downgrade branches
> too.
> 

I agree, but it was the practice we decided to go to based on stacking.
In general we still preserve the source format. It is just up to the
"BzrDir.initialize()" code to create an object with the right attributes
matching the source format. We just trust that whatever BzrDirFormat
object has the formats we want, rather than ignoring its attributes and
using (from the Branch perspective) self._format.initialize().

For "bzr branch --stacked" we don't always preserve the source format.
Which is why this was necessary.

> I am staring at this test now, to make it work with RemoteBranch with
> some other changes going on; what I propose to do in the short term is
> punt on testing with Remote; its no worse that what is in bzr.dev at the
> moment. 

It works with RemoteBranch at the moment. I'm not sure what you are
looking at "fixing". RemoteBranchFormat knows about using
"self._ensure_real" to get the actual remote format, and pass that along.

The point I have above is that I'm setting an attribute of
"RemoteBranchFormat._matchingbzrdir" so that
"_matchingbzrdir.get_branch_format()" returns RemoteBranchFormat, rather
than just hard-coding RemoteBzrDirFormat.get_branch_format() to always
return RemoteBranchFormat.

The end result would be the same either way.
> 
> However, in terms of correct behaviour, I'd like to see branching across
> preserve the format - this helps people moving ahead of stable as much
> as it does laggards that want to read their contributors branches. We
> have tests for stacking working, so I can make sure that stacking isn't
> harmed at all doing this.
> 
> -Rob

My direct concern is that you are misunderstanding the test, and will
punt for RemoteBranch objects when we shouldn't need to.

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

iEYEARECAAYFAkmUTcsACgkQJdeBCYSNAAMkowCePmAaC+M1obhMXQBS6w2HqTec
H3MAn178p8Q7fO7P8AJMj6+vaCONc8fe
=I7hQ
-----END PGP SIGNATURE-----