Re: [JGIT PATCH 8/8] Define a basic merge API, and a two-way tree merge strategy

From: Shawn O. Pearce <spearce@spearce.org>
Date: Jan 19, 2009, 17:42 UTC
Message-ID: <20090119174254.GC14053@spearce.org>
In-Reply-To: <f299b4f30901171116y216835c9jc11df2d424ee0377@mail.gmail.com>

Tomi Pakarinen <tomi.pakarinen@gmail.com> wrote:

> testTrivialTwoWay_disjointhistories() failed because merge strategy
> didn't handle missing base
> version. Am'i right?

I don't think so.

> @@ -119,13 +120,26 @@ protected boolean mergeImpl() throws IOException {
>  				}
> 
>  				final int modeB = tw.getRawMode(T_BASE);

Under a missing base condition modeB == 0.  So,

> -				if (modeB == modeO && tw.idEqual(T_BASE, T_OURS))
> -					add(T_THEIRS, DirCacheEntry.STAGE_0);

If modeB == 0 and modeO == 0 its not in the base and its not in ours. Both SHA-1s will be 0{40} and are thus idEqual, so we should enter this add(T_THEIRS) block. Which is what you tried to write below in your else block, isn't it?.

> -				else if (modeB == modeT && tw.idEqual(T_BASE, T_THEIRS))
> -					add(T_OURS, DirCacheEntry.STAGE_0);

Again, if modeB == 0 and modeT == 0 both SHA-1s will be 0{40} and are idEqual, so we should enter this add(T_OURS) block if both base and theirs are missing. Which again is what you tried to write in your else block.

If that isn't coming out right then perhaps tw.idEqual() is busted for when FileMode is FileMode.MISSING (aka 0). Granted, doing idEqual on FileMode.MISSING is pointless and just wastes clock cycles, but it shouldn't harm the algorithm's correctness.

Show 28 quoted lines

> -				else {
> -					conflict();
> -					hasConflict = true;
> +				if (!FileMode.MISSING.equals(modeB)) {
> +					if (modeB == modeO && tw.idEqual(T_BASE, T_OURS))
> +						add(T_THEIRS, DirCacheEntry.STAGE_0);
> +					else if (modeB == modeT && tw.idEqual(T_BASE, T_THEIRS))
> +						add(T_OURS, DirCacheEntry.STAGE_0);
> +					else {
> +						conflict();
> +						hasConflict = true;
> +					}
> +				} else {
> +					if (!FileMode.MISSING.equals(modeO)
> +							&& FileMode.MISSING.equals(modeT))
> +						add(T_OURS, DirCacheEntry.STAGE_0);
> +					else if (FileMode.MISSING.equals(modeO)
> +							&& !FileMode.MISSING.equals(modeT))
> +						add(T_THEIRS, DirCacheEntry.STAGE_0);
> +					else {
> +						conflict();
> +						hasConflict = true;
> +					}
>  				}
>  			}
>  			builder.finish();
> -- 
> 1.6.0.4

-- 
Shawn.