[PATCH] submodule merge support

[PATCH] submodule merge support

[Martin Waitz]

When merge-recursive gets to a dirlink, it starts an automatic submodule merge
and then uses the resulting merge commit for the top-level tree.
The submodule merge is done in another process to decouple object databases.

Signed-off-by: Martin Waitz <tali@admingilde.org>
---
 .gitignore           |    1 +
 Makefile             |    2 +-
 git-dirlink-merge.sh |   28 ++++++++++++++++++++++++++++
 merge-recursive.c    |   36 ++++++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 1 deletions(-)
 create mode 100644 git-dirlink-merge.sh

diff --git a/.gitignore b/.gitignore
index 8436a83..b076e2f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,7 @@ git-diff-files
 git-diff-index
 git-diff-tree
 git-dirlink-checkout
+git-dirlink-merge
 git-describe
 git-fast-import
 git-fetch
diff --git a/Makefile b/Makefile
index fcd0125..ff24477 100644
--- a/Makefile
+++ b/Makefile
@@ -196,7 +196,7 @@ SCRIPT_SH = \
 	git-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \
 	git-pull.sh git-rebase.sh \
 	git-repack.sh git-request-pull.sh git-reset.sh \
-	git-sh-setup.sh git-dirlink-checkout.sh \
+	git-sh-setup.sh git-dirlink-checkout.sh git-dirlink-merge.sh \
 	git-tag.sh git-verify-tag.sh \
 	git-applymbox.sh git-applypatch.sh git-am.sh \
 	git-merge.sh git-merge-stupid.sh git-merge-octopus.sh \
diff --git a/git-dirlink-merge.sh b/git-dirlink-merge.sh
new file mode 100644
index 0000000..e719b1a
--- /dev/null
+++ b/git-dirlink-merge.sh
@@ -0,0 +1,28 @@
+#!/bin/sh -e
+# Merge a submodule
+# (c) 2006 Martin Waitz
+
+USAGE="submodule orig-sha1 a-sha1 b-sha1"
+
+unset GIT_DIR
+cd "$1"
+
+. git-sh-setup
+
+test $# -eq 4 || usage
+
+orig="$2"
+ours="$3"
+theirs="$4"
+
+base=`git-merge-base "$ours" "$theirs"`
+
+if test `git-merge-base "$orig" "$base"` != "$orig"; then
+	die "$1 cannot be merged: other side switched branches"
+fi
+
+if test `git-rev-parse --verify HEAD` != "$ours"; then
+	die "$1: HEAD != ours"
+fi
+
+exec git-merge $theirs
diff --git a/merge-recursive.c b/merge-recursive.c
index 8f72b2c..4b67cd0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -11,6 +11,7 @@
 #include "diff.h"
 #include "diffcore.h"
 #include "run-command.h"
+#include "refs.h"
 #include "tag.h"
 #include "unpack-trees.h"
 #include "path-list.h"
@@ -574,6 +575,21 @@ static void update_file_flags(const unsigned char *sha,
 		void *buf;
 		unsigned long size;
 
+		if (S_ISDIRLNK(mode)) {
+			/* defer dirlinks to another process, don't try to */
+			/* read the object "sha" here */
+			const char *dirlink_checkout[] = {
+				"dirlink-checkout", path, sha1_to_hex(sha), NULL
+			};
+			struct child_process cmd = {
+				.argv = dirlink_checkout,
+				.git_cmd = 1,
+			};
+
+			run_command(&cmd);
+			goto update_index;
+		}
+
 		buf = read_sha1_file(sha, &type, &size);
 		if (!buf)
 			die("cannot read object %s '%s'", sha1_to_hex(sha), path);
@@ -1069,6 +1085,26 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
 
 			free(result_buf.ptr);
 			result.clean = (merge_status == 0);
+		} else if (S_ISDIRLNK(a->mode)) {
+			const char *dirlink_merge[] = {
+				"dirlink-merge", a->path,
+				sha1_to_hex(o->sha1),
+				sha1_to_hex(a->sha1),
+				sha1_to_hex(b->sha1),
+				NULL
+			};
+			struct child_process cmd = {
+				.argv = dirlink_merge,
+				.git_cmd = 1,
+			};
+			/* recurse into the submodule in a different process */
+			result.clean = !run_command(&cmd);
+			if (result.clean) {
+				/* get the new merged version */
+				if (resolve_gitlink_ref(a->path, "HEAD",
+				                        result.sha) < 0)
+					result.clean = 0;
+			}
 		} else {
 			if (!(S_ISLNK(a->mode) || S_ISLNK(b->mode)))
 				die("cannot merge modes?");
-- 
1.5.1.2.247.gaef5a


-- 
Martin Waitz

Re: [PATCH] submodule merge support

[Alex Riesen]

Martin Waitz, Sun, May 06, 2007 21:02:24 +0200:
> When merge-recursive gets to a dirlink, it starts an automatic
> submodule merge and then uses the resulting merge commit for the
> top-level tree.

merge-recursive is a mess already, you just made even more so.
Besides, you completely forgot all other merge strategies.

How about making all existing strategies just ignore submodules, and
move recursive merge in the merge driver (git-merge.sh)?

Re: [PATCH] submodule merge support

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

hoi :)

On Mon, May 07, 2007 at 12:07:45AM +0200, Alex Riesen wrote:
> Martin Waitz, Sun, May 06, 2007 21:02:24 +0200:
> > When merge-recursive gets to a dirlink, it starts an automatic
> > submodule merge and then uses the resulting merge commit for the
> > top-level tree.
> 
> merge-recursive is a mess already, you just made even more so.
> Besides, you completely forgot all other merge strategies.
> 
> How about making all existing strategies just ignore submodules, and
> move recursive merge in the merge driver (git-merge.sh)?

Well, I don't think it can be done in the merge driver but it does make
sense to consolidate all the low-level file-based merging (and the new
submodule merge is exactly that) between merge strategies.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] submodule merge support

[Linus Torvalds]

On Mon, 7 May 2007, Alex Riesen wrote:

> Martin Waitz, Sun, May 06, 2007 21:02:24 +0200:
> > When merge-recursive gets to a dirlink, it starts an automatic
> > submodule merge and then uses the resulting merge commit for the
> > top-level tree.
> 
> merge-recursive is a mess already, you just made even more so.
> Besides, you completely forgot all other merge strategies.
> 
> How about making all existing strategies just ignore submodules, and
> move recursive merge in the merge driver (git-merge.sh)?

Yes, I think that's the right thing to do.

I think it's the right thing for another reason: in a true "recursive" 
merge, the submodules shouldn't be recursively merged anyway. *THEIR* 
merge will have its own history, and doing it based on some random history 
of the superproject is actually wrong anyway!

		Linus

Re: [PATCH] submodule merge support

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

hoi :)

On Sun, May 06, 2007 at 03:18:53PM -0700, Linus Torvalds wrote:
> On Mon, 7 May 2007, Alex Riesen wrote:
> > merge-recursive is a mess already, you just made even more so.
> > Besides, you completely forgot all other merge strategies.
> > 
> > How about making all existing strategies just ignore submodules, and
> > move recursive merge in the merge driver (git-merge.sh)?
> 
> Yes, I think that's the right thing to do.
> 
> I think it's the right thing for another reason: in a true "recursive" 
> merge, the submodules shouldn't be recursively merged anyway. *THEIR* 
> merge will have its own history, and doing it based on some random history 
> of the superproject is actually wrong anyway!

Of course the submodule has to get its own history, it's not possible
to do otherwise.  But you have to trigger the submodule merge when you
find a submodule-level conflict in the supermodule merge, just as
you trigger file-level three-way merges, too.
This is really all which my patch does -- it starts a new instance of
the merge driver which only merges the submodule commit from the other
supermodule tree.
So it's not recursive in the merge-recursive way, but in a
call-git-merge-again way.

The comment about merge-recursive not being the best place is correct of
course.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] submodule merge support

[Johannes Sixt]

Martin Waitz wrote:
> On Sun, May 06, 2007 at 03:18:53PM -0700, Linus Torvalds wrote:
> > On Mon, 7 May 2007, Alex Riesen wrote:
> > > How about making all existing strategies just ignore submodules, and
> > > move recursive merge in the merge driver (git-merge.sh)?
> >
> > Yes, I think that's the right thing to do.
> >
> > I think it's the right thing for another reason: in a true "recursive"
> > merge, the submodules shouldn't be recursively merged anyway. *THEIR*
> > merge will have its own history, and doing it based on some random history
> > of the superproject is actually wrong anyway!
> 
> Of course the submodule has to get its own history, it's not possible
> to do otherwise.  But you have to trigger the submodule merge when you
> find a submodule-level conflict in the supermodule merge, just as
> you trigger file-level three-way merges, too.

I think you missed Linus's point: If the supermodule's merge leads to a
conflict in the submodule links, it is not appropriate to merge the
submodule.

Say you are merging commits A and B in the supermodule, and A uses v1.0
of the submodule and B uses v2.0 of submodule, then you can't just merge
v1.0 and v2.0 - instead, you have to make a decision whether the
supermodule's merge result is going to use v1.0 or v2.0 or even
something different like v2.1. An automatic merge cannot make this
decision for you (unless there was no conflict in the first place).

-- Hannes

Re: [PATCH] submodule merge support

[Linus Torvalds]

On Mon, 7 May 2007, Johannes Sixt wrote:
> 
> I think you missed Linus's point: If the supermodule's merge leads to a
> conflict in the submodule links, it is not appropriate to merge the
> submodule.

That is true, but no, that wasn't what I was trying to say.

What I was trying to say was really that the merge-base in the 
super-module is simply totally irrelevant to the sub-module, and any merge 
at all that thinks it is is obviously broken.

Now, for a _normal_ merge (with just a single merge-base), this is not an 
issue, since the proposed submodule merger wouldn't care about the 
supermodule merge base anyway.

But if you have multiple merge-bases and you do a recursive merge to 
create a new *combined* merge-base, trying to do that for the submodule is 
just pointless. You shouldn't. The merge-base for the submodule will be 
irrelevant for the final merge *anyway* (since the submodule history comes 
from itself), so in a recursive sub-merge, you shouldn't even *try* to 
merge the submodule. The end result would never be used anyway, and the 
only thing you can do is make for more complexity.

So not doing it in the low-level merger is right - because it is simply 
irrelevant at that stage. The low-level merger might as well ignore 
submodules.

I think.

			Linus

Re: [PATCH] submodule merge support

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 3480 bytes --]

hoi :)

On Mon, May 07, 2007 at 12:30:30PM +0200, Johannes Sixt wrote:
> Martin Waitz wrote:
> > On Sun, May 06, 2007 at 03:18:53PM -0700, Linus Torvalds wrote:
> > > On Mon, 7 May 2007, Alex Riesen wrote:
> > > > How about making all existing strategies just ignore submodules, and
> > > > move recursive merge in the merge driver (git-merge.sh)?
> > >
> > > Yes, I think that's the right thing to do.
> > >
> > > I think it's the right thing for another reason: in a true "recursive"
> > > merge, the submodules shouldn't be recursively merged anyway. *THEIR*
> > > merge will have its own history, and doing it based on some random history
> > > of the superproject is actually wrong anyway!
> > 
> > Of course the submodule has to get its own history, it's not possible
> > to do otherwise.  But you have to trigger the submodule merge when you
> > find a submodule-level conflict in the supermodule merge, just as
> > you trigger file-level three-way merges, too.
> 
> I think you missed Linus's point: If the supermodule's merge leads to a
> conflict in the submodule links, it is not appropriate to merge the
> submodule.

why?

We do file-level merging, too.
A submodule is not that different, besides being much more complex ;-)
We should try to automatically merge the content if possible and leave
conflict markers if not.

> Say you are merging commits A and B in the supermodule, and A uses v1.0
> of the submodule and B uses v2.0 of submodule, then you can't just merge
> v1.0 and v2.0 - instead, you have to make a decision whether the
> supermodule's merge result is going to use v1.0 or v2.0 or even
> something different like v2.1. An automatic merge cannot make this
> decision for you (unless there was no conflict in the first place).

So when should we try to merge and when should we leave conflict
markers?

Let's look at submodule commits O, A, B which are the commits that are
used by the (supermodule-) merge base, our supermodule version and their
supermodule version.

The trivial cases are:

 O,A -..- B        O,B -..- A

Then there is the fast-forward case:

 O -..- A -..- B      O -..- B -..- A

All of those do not require to create any new submodule commit, just use
the one on the right side of the chain.  I think they are not
controversial.

Now to the more complex parts:

          /-..- A
 O -..- X
          \-..- B

and

   /-..- A
 X
   \-..- O -..- B

Here we don't have any existing submodule commit that can be used.
In the second case an automatic merge cannot be done -- one supermodule
branch choose to rewind the submodule (e.g. by switching to another
branch in the submodule) and there really is no way to automatically
detect what should be done.
But when the submodule got branched after the supermodule merge-base
then an automatic submodule merge can be tried.
And it really makes sense to do so:
If you rely on submodule for your project, you will have people
submitting features which touch several submodules.  They can simply
create a new supermodule version incorporating all their changes and
send it to their supermodule-upstream.  After review,
supermodule-upstream wants all the new submodule versions merged into
her tree and it would be wrong to refuse to do that automatically.

The above behaviour is exactly what is implemented in the patch.
(OK, I could write a more elaborate commit message... ;-)

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] submodule merge support

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

hoi :)

On Mon, May 07, 2007 at 09:02:00AM -0700, Linus Torvalds wrote:
> But if you have multiple merge-bases and you do a recursive merge to 
> create a new *combined* merge-base, trying to do that for the submodule is 
> just pointless.

Ah, now I see your point.
And yes, you are right.

> So not doing it in the low-level merger is right - because it is simply 
> irrelevant at that stage. The low-level merger might as well ignore 
> submodules.

But where to do it then?
Do a first run which simply ignores dirlinks and then do another run
which looks for all unmerged index entries and processes all dirlinks
by merging the submodule?

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] submodule merge support

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 342 bytes --]

hoi :)

On Mon, May 07, 2007 at 10:17:11AM -0700, Linus Torvalds wrote:
> Oh, btw, you should also look out for the case where the superproject 
> doesn't actually have the subprojects in question checked out. What 
> happens then? Just leave them conflicted?

yes, I think that's the only sane thing one can do.

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]