Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [jgit-dev] Handle multiple merge points when searching for common ancestor

Dariusz Luksza <dariusz.luksza@xxxxxxxxx> wrote:
> Some time ago Stefan Lay create a bug 317934[1] for synchronization
> view and its common base ancestor. In this bug report he mention about
> 'multiple merge bases', to be honest I'm not centrally sure what it
> means. But I guesses that 'multiple merge bases' are for eg. merge
> commits (when two or more branches are merged) em I right ?
> If yes, then RevWalk with RevFilter.MERGE_BASE will return only one
> RevCommit object for 'normal' (non merge) commit's.

Actually, if you pump the RevWalk again by calling next() a second
time, it will return either null or the 2nd merge base if there are
multiple merge bases available.  So to find all of them, just pump
next() until it returns null.  :-)

> Based on this
> assumptions I've implemented searching of common ancestor in
> GitBaseResourceVariantTree[2] class and it seams that this works fine.

Yes, here you are only pulling up the first merge base.  If you
needed to know all of them, you woul dhave to pump the walker more.
But as I mentioned in the other message in this thread, if you can't
do the full recursive combination when multiple merge bases exist,
you want the first.

FWIW, we prefer parseCommit() rather than lookupCommit() when
converting from a Ref or ObjectId into a RevCommit.  There are two
good reasons for this:

- It permits a tag to be used as though it were a commit.

  If someone passes in to you "v1.4.3" (to use the v1.4.3 tag),
  the Ref is pointing at an annotated tag object.  By doing
  lookupCommit you have promised the walker the tag ObjectId is
  actually a commit ObjectId.  Later on this will explode with an
  IncorrectObjectTypeException.  By using parseCommit() this case
  is magically handled by unfolding the tag to the commit.

- It verifies the object exists.

  If the Ref is dangling, or the input was invalid, we would get
  back a MissingObjectException during the next() call, but then
  its harder to tell where that invalid ObjectId originated from.
  Doing a parseCommit up front allows you to trap the exception
  and tell exactly where the bad input came from.
> But I don't know what I should do when there is more than one
> RevCommit's in RevWalk, could you help me solve this problem ?

Like I said, if you can't do the full recursive merge here (and
you probably can't) use the first.

Back to the top