[merge][0.91] set default format to dirstate-tags

Martin Pool mbp at sourcefrog.net
Tue Aug 14 14:37:39 BST 2007


On 8/14/07, Robert Collins <robertc at robertcollins.net> wrote:

> On Tue, 2007-08-14 at 21:53 +1000, Martin Pool wrote:
> > This builds on my previous patch that adjusts append_revision and so on.
> >
> >  * set the default branch format and bzrdir format -- the concrete
> > change caused by this is that we now use just a last-revision marker,
> > and have tags
>
> >  * remote branches now support tags -- of course whether it actually
> > works depends on the remote branch format
>
> do you mean RemoteBranch ? or branches over SFTP?

s/supports/advertises to the test suite that it should be tested for/

I meant RemoteBranchFormat.  'real' branch formats work the same
regardless of transport.

> > There is some divergence between formats about whether you are allowed
> > to point a branch to a revision that's not in its repository.  A few
> > methods assume that it is present and look at the graph.  The tests
> > that rely on this being possible seem to do so only accidentally, so I
> > have just adjusted them to use a revision that is present.
>
> I'm not entirely clear what you mean here. I guess you are saying that
> 'tests which are using branches with a revision id *not* present in the
> repository seem to be doing so accidentally so they no longer do'. (My
> confusion is in the binding of 'this').

Yes: there is a situation S where a branch's last revision is set to a
revision not in its repository.  There are no branch impl tests for S,
so I guess it's not specifically supported.  There are some tests
which incidentally depend on S being supported and not supported in
particular cases, but the tests don't really seem to care about S, so
I changed them.

But if someone strongly feels that this should either be supported or
forbidden, we need to add tests for it and update the branch code.  I
think that would not need to block this merge.

> > I'd like to merge this as soon as we branch off 0.90 so that we can
> > get the most testing with this format in place.
>
> Amen :).
>
> I think the news should be more clear about incompatabilities. we've had
> a SURPRISES before. Something like 'Newly inited branchess will not work
> with bzr below 0.15. Existing branches will interoperate fine with bzr
> 0.90, but if upgraded will cease being compatible with bzr below 0.15.'

+1

> I think I'd prefer to see append_revision deleted, or the prior contract
> upheld, because this is less likely to result in 'it did the wrong
> thing' surprises.

I'm ok to delete it, aside from the general principle that we
deprecate things rather than deleting them.  This does not seem like
something many plugins would be using, aside from (as you said)
importers.  Having seen a few inconsistent behaviours I would say that
I haven't changed the contract, just moved within the existing
untested wiggle room.  But maybe it will break callers that depend on
the old code.

> You duplicate the IN DEVELOPMENT heading in NEWS in your patch, as well
> as changing the version number.

ok

> When is branch.supports_tags() called? Would it be better for
> RemoteBranch to proxy the method to the smart server, than to
> unconditionally report True? (Put another way, what happens if it
> reports true and a client tries to use tags, but the underlying branch
> doesn't ? - and if this doesn't cause failures (other than tag commands
> not tagging ;)), why do we have 'supports_tags' as a method?

It's branch_format.supports_tags(), so we can't pass it through.  It's
only used from the test suite, which wants to check that either all
the tag methods work or all raise notsupported.  Maybe it should check
that directly, or maybe it should be renamed to _wants_tag_tests or
similar.

-- 
Martin



More information about the bazaar mailing list