[PATCH 0 of 9] improvements for named branches

Alexis S. L. Carvalho alexis at cecm.usp.br
Sun Mar 2 15:17:45 CST 2008


Thus spake Brendan Cully:
> On Sunday, 02 March 2008 at 16:01, Alexis S. L. Carvalho wrote:
> > Hi
> > 
> > This patch series implements two features for named branches:
> > 
> > - multi-headed branches (patches 1-4)
> > 
> > - branch closing (patches 5-9)
> > 
> > 
> > Just like a repo can have multiple heads, a named branch can easily grow
> > multiple heads (two users commit stuff in their private repos and then
> > pull the other).  So I think it makes sense to keep track of all the
> > heads of a named branch.
> > 
> > UI-wise this means that commands that want a single revision (e.g. diff)
> > will complain that the name is ambiguous; commands that take multiple
> > revisions (pull, push, log) will use all of the heads.  The exception is
> > update, which will still update to the tip of the branch (but I think
> > I'll add a "warning: branch has multiple heads").  merge will also want
> > some extra smarts to be more user-friendly.
> 
> this makes a lot of sense to me.
> 
> > "Closing a branch" essentially means to remove it from the branch cache,
> > which means that its name won't be recognized anymore, effectively
> > hiding it.  I like "close" better than "remove" or "delete", since we're
> > not actually removing anything from the repo itself.
> > 
> > I used a field in extra ("closebranch") to save the name of the branch
> > being closed and another ("closebranchnodes") to save the list of branch
> > heads being closed.
> > 
> > hg branch --close foo   schedules the branch "foo" to be closed on the
> > next commit.  hg merge --close  is equivalent to "hg merge; hg branch
> > --close <branch-of-second-parent>".
> > 
> > 
> > It's interesting to note that saving just the name of the branch being
> > closed would make it hard to figure out what heads are being closed,
> > especially if someone reopens the branch later.  So I think we need some
> > pointer to the heads being closed.  It can be either an explicit list
> > (which is my extra['closebranchnodes']) or an implicit one (probably the
> > parents of the revision closing the branch).
> > 
> > Having an implicit list would require less code, but having an explicit
> > one allows the ui to be more forgiving at least in the case where
> > somebody wants to merge and close another branch, but makes an error:
> > 
> >     hg merge another-branch  #Oops, forgot --close
> >     hg commit -m 'merge another-branch'
> >     hg push
> > 
> > After this sequence, with an explicit list one can do just (well, it
> > will be possible after I allow an empty commit that just closes a
> > branch)
> > 
> >     hg branch --close another-branch
> >     hg commit -m 'close another-branch'
> >     hg push
> > 
> > While with an implicit list, you'd need something like
> > 
> >     hg up -C another-branch
> >     hg branch default        #or some other branch
> >     hg branch --close another-branch
> >     hg commit -m 'close another-branch'
> >     hg up -C -- -2           #assuming no other revision was committed
> >     hg merge tip             #merge the new head of this branch
> >     hg commit -m 'merge closing of another-branch'
> >     hg push
> > 
> > Which is a much more complicated and error-prone dance.
> > 
> > It also makes it easier to close ancient branches that are still opened
> > just because we didn't have a way to close them.
> > 
> > The nodes in this explicit list are automatically calculated by hg.

One thing I forgot to mention - "hg branch --close foo; hg commit" will
close all the heads of branch foo that are ancestors of this new
revision.  If there are branch heads that are not ancestors of it, they
will stay opened.

At first I thought this was needed because of the way we scan the
changelog while building the branch cache, but now I can see another way
that would allow this to work.

But I still think it's better to avoid messing with branch heads that
are not ancestors of the parents of the working dir.

> I haven't actually read your patch set yet. I think the feature is badly needed,
> but my instinct is that the implicit list is fine. I would have expected something
> like
> 
> hg branch --close

I used this command to print the branch that will be closed.  We could
change this to close the branch of the (last non-null) parent of the
working dir, but then I don't know how the user would be able to query
the name of the branch that will be closed (I don't think "cat
.hg/closebranch" counts...).

IOW, I think this is mostly a UI bit that can (mostly?) be made to work
even with the implementation I've sent.

> closing old branches would just be
> 
> hg up -C oldbranch
> hg branch --close
> hg commit -m'close branch'

Except that this leaves you with an extra repo head - and as you mentioned
later, what branch will this new commit be in?

> and fixing up merges wouldn't have any extra logic, just
> 
> hg up -C oldbranch
> hg branch --close
> hg commit -m'close branch'
> hg merge main
> hg commit

Compared to the sequence that I mentioned, I did two extra things:

- explicitly set a branch for the 'close branch' commit

- an additional up -C just to swap the order of the merge parents.

Also note that if you just "hg merge main", the merge commit will be in
whatever branch you're currently on.

> I don't know -- is this implicit style more confusing? I think the main drawback is
> that it becomes too easy to accidentally close the main branch,

Right now I disallow closing the current branch - you'd have to change
the working dir branch before the "hg branch --close".  (Hmm...
thinking a bit more I can see a few ways to escape my check, which means
I'll have to improve it a bit)

>                                                                 and it's not obvious
> which branch the working directory should move to after the close is committed. I'd
> guess default. In the case of a merge, I'd say the non-closed branch.

As a rule of thumb I'd prefer it if we made "default" less special.  So
I prefer requiring the user to choose another branch.  (But maybe I can
move my "don't close the branch of the new revision" check to commit, so
that a user would be able to call branch --close before changing
branches).

> I'm not sure what a merge --close should do if both parents are on the branch to
> be closed. Possibly just warn (and move to default after the commit).

Right now I just print a warning and don't close anything.

Alexis


More information about the Mercurial-devel mailing list