[PATCH v2] Submodule merge support

Subsystems: the rest

DORMANTno replies

5 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH v2] Submodule merge support

From: Martin Waitz <hidden>
Date: 2016-06-15 22:43:11

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.

Submodule merges are done solely in the submodules' history, without taking
the supermodule (and it's merge base) into account.  If the submodule merge
is successful then the new submodule version will be used in the merged
supermodule.

If one side of the merge removed any submodule commits (e.g. by switching to
a different branch) then the automatic merge is stopped so that the user can
take a closer look on what happened.

Signed-off-by: Martin Waitz <redacted>
---

This patch is based on my previous submodule checkout patch and the
start-commands-in-submodule patch.

This version takes index_only into account and does not need a new
helper script as all code is done in C now.

The entire ll_merge code in merge-recursive still should be moved to
some generic place, but that is for another patch.

 merge-recursive.c |  122 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 122 insertions(+), 0 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 8f72b2c..72562a8 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);
@@ -1025,6 +1041,105 @@ static int ll_merge(mmbuffer_t *result_buf,
 	return merge_status;
 }
 
+
+static int ll_dirlink_merge_base(const char *path,
+                            const unsigned char *a,
+                            const unsigned char *b,
+                            unsigned char *result)
+{
+	const char *merge_base[] = {
+		"merge-base",
+		sha1_to_hex(a),
+		sha1_to_hex(b),
+		NULL
+	};
+	struct child_process cmd = {
+		.argv = merge_base,
+		.submodule = path,
+		.git_cmd = 1,
+		.out = -1,
+	};
+	char hex[40];
+	int status;
+
+	status = start_command(&cmd);
+	if (status) return status;
+
+	status = read(cmd.out, hex, sizeof(hex));
+	if (status != 40) return status;
+
+	status = finish_command(&cmd);
+	if (status) return status;
+
+	status = get_sha1_hex(hex, result);
+
+	return status;
+}
+
+static int ll_dirlink_merge(const char *path,
+                            const unsigned char *o,
+                            const unsigned char *a,
+                            const unsigned char *b,
+                            unsigned char *result)
+{
+	char b_hex[40+1];
+	const char *merge[] = {
+		"merge", b_hex, NULL
+	};
+	struct child_process cmd = {
+		.argv = merge,
+		.submodule = path,
+		.git_cmd = 1,
+	};
+	int status;
+	unsigned char base[20];
+	unsigned char test[20];
+
+	if (index_only)  {
+		/* as submodules have their own history we don't have to   */
+		/* try to do the index_only intermediate merges.           */
+		/* however we still want to get a submodule version        */
+		/* which is suitable as merge-base, just to make sure that */
+		/* all merge parents contain this base.                    */
+		/* The real merge (below) aborts if this check fails       */
+		return ll_dirlink_merge_base(path, a, b, result);
+	}
+
+	strcpy(b_hex, sha1_to_hex(b));
+	output(3, "merging submodule %s:", path);
+	output(3, " o=%s", sha1_to_hex(o));
+	output(3, " a=%s", sha1_to_hex(a));
+	output(3, " b=%s", sha1_to_hex(b));
+
+	/* first check that the submodule is in the current state  */
+	/* so that it can be merged.                               */
+	status = resolve_gitlink_ref(path, "HEAD", test);
+	if (hashcmp(test, a)) {
+		return error("can't merge submodule %s: not up to date.", path);
+	}
+
+	/* check that both sides of the superproject only did a    */
+	/* fast forward of the subproject so that it can be merged */
+	/* automatically.                                          */
+	status = ll_dirlink_merge_base(path, a, b, base);
+	if (status) return status;
+	status = ll_dirlink_merge_base(path, o, base, test);
+	if (status) return status;
+	if (hashcmp(test, o)) {
+		return error("can't merge submodule %s: conflicting history",
+		             path);
+	}
+
+	/* now start another merge process for the submodule */
+	status = run_command(&cmd);
+	if (status) return status;
+
+	/* get the new merged version */
+	status = resolve_gitlink_ref(path, "HEAD", result);
+
+	return status;
+}
+
 static struct merge_file_info merge_file(struct diff_filespec *o,
 		struct diff_filespec *a, struct diff_filespec *b,
 		const char *branch1, const char *branch2)
@@ -1069,6 +1184,13 @@ 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)) {
+			int merge_status;
+
+			merge_status = ll_dirlink_merge(a->path,
+				o->sha1, a->sha1, b->sha1, result.sha);
+
+			result.clean = (merge_status == 0);
 		} else {
 			if (!(S_ISLNK(a->mode) || S_ISLNK(b->mode)))
 				die("cannot merge modes?");
-- 
1.5.2.2.g081e


-- 
Martin Waitz

Re: [PATCH v2] Submodule merge support

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:43:11

quoted hunk
@@ -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,
+			};
My Solaris 9 system cannot compile this syntax, even though it is
a clean way to initalize the child_process.  That's why I've always
used something more like:

	struct child_process cmd;
	memset(&cmd, 0, sizeof(cmd));
	cmd.argv = dirlink_checkout;
	cmd.git_cmd = 1;

and actually that raises another point, does the compiler 0 fill
the stack-allocated struct that is initalized like you write, or
does it avoid filling the other fields that aren't mentioned in
the initialization?
+	status = read(cmd.out, hex, sizeof(hex));
+	if (status != 40) return status;
OK, this is probably just never trusting the OS, but shouldn't that
read be wrapped up in a loop, like our read_in_full?  We want 40
bytes here, and expect it, and the read call is allowed to return
as few as 1 byte....

-- 
Shawn.

Re: [PATCH v2] Submodule merge support

From: Martin Waitz <hidden>
Date: 2016-06-15 22:43:11

hoi :)

On Mon, May 21, 2007 at 02:20:05AM -0400, Shawn O. Pearce wrote:
quoted
@@ -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,
+			};
My Solaris 9 system cannot compile this syntax, even though it is
a clean way to initalize the child_process.
any special thing it does not like in the above code or does it just
not support structs that are initialized that way?
quoted
+	status = read(cmd.out, hex, sizeof(hex));
+	if (status != 40) return status;
OK, this is probably just never trusting the OS, but shouldn't that
read be wrapped up in a loop, like our read_in_full?  We want 40
bytes here, and expect it, and the read call is allowed to return
as few as 1 byte....
right.

-- 
Martin Waitz

Re: [PATCH v2] Submodule merge support

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:43:11

Martin Waitz [off-list ref] wrote:
On Mon, May 21, 2007 at 02:20:05AM -0400, Shawn O. Pearce wrote:
quoted
quoted
@@ -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,
+			};
My Solaris 9 system cannot compile this syntax, even though it is
a clean way to initalize the child_process.
any special thing it does not like in the above code or does it just
not support structs that are initialized that way?
Its a very old Sun C compiler, and it doesn't like structs to be
initialized that way.  Yes, newer compilers are better, and gcc is
also better, but I'm unable to get our UNIX admins to actually do
their job and keep systems usable by the users.

/me starts to wonder why he continues with this day-job thing...

-- 
Shawn.

Re: [PATCH v2] Submodule merge support

From: Alex Riesen <hidden>
Date: 2016-06-15 22:43:11

On 5/21/07, Shawn O. Pearce [off-list ref] wrote:
Its a very old Sun C compiler, and it doesn't like structs to be
initialized that way.  Yes, newer compilers are better, and gcc is
also better, but I'm unable to get our UNIX admins to actually do
their job and keep systems usable by the users.

/me starts to wonder why he continues with this day-job thing...
Because the new job may involve windows
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help