threads / patch / 8004

patchsubmodule merge support

Subject: [PATCH] submodule merge support

## tl;dr

10 messages between May 6, 2007 and May 7, 2007. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Martin Waitz· May 6, 2007, 19:02 UTC · lore

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
Show changes to 4 files +66 −1

.gitignore, Makefile, git-dirlink-merge.sh, merge-recursive.c

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
Alex Riesen· May 6, 2007, 22:07 UTC · re: Martin Waitz · lore

Re: [PATCH] submodule merge support

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)?

Martin Waitz· May 6, 2007, 22:16 UTC · re: Alex Riesen · lore

Re: [PATCH] submodule merge support

hoi :)
On Mon, May 07, 2007 at 12:07:45AM +0200, Alex Riesen wrote:
Show 10 quoted lines
> 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
Linus Torvalds· May 6, 2007, 22:18 UTC · re: Alex Riesen · lore

Re: [PATCH] submodule merge support

On Mon, 7 May 2007, Alex Riesen wrote:
Show 10 quoted lines
> 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
Martin Waitz· May 7, 2007, 09:03 UTC · re: Linus Torvalds · lore

Re: [PATCH] submodule merge support

hoi :)
On Sun, May 06, 2007 at 03:18:53PM -0700, Linus Torvalds wrote:
Show 13 quoted lines
> 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
Johannes Sixt· May 7, 2007, 10:30 UTC · re: Martin Waitz · lore

Re: [PATCH] submodule merge support

Martin Waitz wrote:
Show 16 quoted lines
> 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
Linus Torvalds· May 7, 2007, 16:02 UTC · re: Johannes Sixt · lore

Re: [PATCH] submodule merge support

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
Martin Waitz· May 7, 2007, 16:44 UTC · re: Linus Torvalds · lore

Re: [PATCH] submodule merge support

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
Martin Waitz· May 7, 2007, 16:37 UTC · re: Johannes Sixt · lore

Re: [PATCH] submodule merge support

hoi :)
On Mon, May 07, 2007 at 12:30:30PM +0200, Johannes Sixt wrote:
Show 21 quoted lines
> 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.

Show 6 quoted lines
> 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
Martin Waitz· May 7, 2007, 19:33 UTC · lore

Re: [PATCH] submodule merge support

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

← back to recent threads