[MERGE][#164626] Preserve repository format when branching from HPSS

Andrew Bennetts andrew at canonical.com
Fri Nov 30 05:57:16 GMT 2007


Martin Pool wrote:
> Martin Pool has voted tweak.
> Status is now: Conditionally approved
> Comment:
> It does seem like this ought to be applied across all formats (bzrdir,
> branch, wt even) rather than just for the repository.  (Or is that
> already done in some other way?  I doubt it.)  So, unless this is already 
> done, please mark this bug as closed and open a medium-priority one about 
> the others.

I believe there's already branch_implementations tests for branch.sprout
preserving branch format (but not that it preserves bzrdir, wt or repo format).
So I'll file those bugs.

> +        try:
> +            dest_repo = 
> self._real_repository._format.initialize(to_bzrdir,
> + shared=False)
>
> (query)
> I wonder if that should be separated out into a ._real_format() method on
> all the Remote objects?

Perhaps.  Although we almost only care about the features supported by the
remote format (e.g. rich roots), rather than exactly what that format is.
Except of course that if you know the exact format you can then deliver data in
that format and avoid the costs of translating it.  So it's probably a useful
thing to add.

> +        except errors.UninitializableFormat:
> +            dest_repo = to_bzrdir.open_repository()
>
> (tweak)
> I'm not totally sure this is a good way to distinguish all-in-one bzrdirs
> from otherwise uninitializable formats, but I can't think of anything much
> better, and I guess we're not very likely to hit either of them on this
> path.  Especially because you say below that you can't read all in one
> formats over hpss.  So is the except block needed at all?

Good question.  I was trying to replicate the logic in Repository.sprout
closely, but you're right it's not needed.  Thanks.  

> +        if self.repository_format == RepositoryFormat7():
> +            raise KnownFailure(
> +                "Cannot fetch weaves over smart protocol.")
>
> (tweak) These seem more like TestSkipped: the test didn't fail, rather we 
> can't run it yet.

Fair enough.

> With those tweaks, this is still an improvement, please merge it.

Thanks!

-Andrew.




More information about the bazaar mailing list