[PATCH 0/3] merge -Xindex-only

STALE3715d

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

[PATCH 0/3] merge -Xindex-only

From: Thomas Rast <hidden>
Date: 2016-06-15 22:58:01

[Michael, sorry for the double mail -- I typoed the list address on
the first round.]

I recently looked into making merge-recursive more useful as a modular
piece in various tasks, e.g. Michael's git-imerge and the experiments
I made in showing evil merges.

This miniseries is the extremely low-hanging fruit.  If it makes a
good first step for git-imerge, perhaps it can go in like this.  It's
not a big speedup (about 2.2s vs 2.4s in a sample conflicting merge in
git.git), but it does feel much cleaner to avoid touching the worktree
unless actually necessary.

Otherwise it's probably not worth it just yet; for what I want to do
with it, we need some more reshuffling of things.



Thomas Rast (3):
  merge-recursive: remove dead conditional in update_stages()
  merge-recursive: untangle double meaning of o->call_depth
  merge-recursive: -Xindex-only to leave worktree unchanged

 Documentation/merge-strategies.txt |  4 ++++
 merge-recursive.c                  | 46 +++++++++++++++++++++-----------------
 merge-recursive.h                  |  1 +
 t/t3030-merge-recursive.sh         | 13 +++++++++++
 4 files changed, 43 insertions(+), 21 deletions(-)

-- 
1.8.3.2.908.gbd0dbd0

[PATCH 1/3] merge-recursive: remove dead conditional in update_stages()

From: Thomas Rast <hidden>
Date: 2016-06-15 22:58:01

650467c (merge-recursive: Consolidate different update_stages
functions, 2011-08-11) changed the former argument 'clear' to always
be true.  Remove the useless conditional.

Signed-off-by: Thomas Rast <redacted>
---
 merge-recursive.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index ea9dbd3..0b9cafb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -544,11 +544,9 @@ static int update_stages(const char *path, const struct diff_filespec *o,
 	 * would_lose_untracked).  Instead, reverse the order of the calls
 	 * (executing update_file first and then update_stages).
 	 */
-	int clear = 1;
 	int options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_SKIP_DFCHECK;
-	if (clear)
-		if (remove_file_from_cache(path))
-			return -1;
+	if (remove_file_from_cache(path))
+		return -1;
 	if (o)
 		if (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options))
 			return -1;
-- 
1.8.3.2.908.gbd0dbd0

[PATCH 2/3] merge-recursive: untangle double meaning of o->call_depth

From: Thomas Rast <hidden>
Date: 2016-06-15 22:58:01

o->call_depth has a double function: a nonzero call_depth means we
want to construct virtual merge bases, but it also means we want to
avoid touching the worktree.  Introduce a new flag o->no_worktree for
the latter.

Signed-off-by: Thomas Rast <redacted>
---
 merge-recursive.c | 38 +++++++++++++++++++++-----------------
 merge-recursive.h |  1 +
 2 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 0b9cafb..b93b762 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -407,10 +407,10 @@ static void record_df_conflict_files(struct merge_options *o,
 	int i;
 
 	/*
-	 * If we're merging merge-bases, we don't want to bother with
-	 * any working directory changes.
+	 * If we're working in-core only (e.g., merging merge-bases),
+	 * we don't want to bother with any working directory changes.
 	 */
-	if (o->call_depth)
+	if (o->no_worktree)
 		return;
 
 	/* Ensure D/F conflicts are adjacent in the entries list. */
@@ -723,7 +723,7 @@ static void update_file_flags(struct merge_options *o,
 			      int update_cache,
 			      int update_wd)
 {
-	if (o->call_depth)
+	if (o->no_worktree)
 		update_wd = 0;
 
 	if (update_wd) {
@@ -930,7 +930,7 @@ static struct merge_file_info merge_file_1(struct merge_options *o,
 			result.clean = merge_submodule(result.sha,
 						       one->path, one->sha1,
 						       a->sha1, b->sha1,
-						       !o->call_depth);
+						       !o->no_worktree);
 		} else if (S_ISLNK(a->mode)) {
 			hashcpy(result.sha, a->sha1);
 
@@ -1002,7 +1002,7 @@ static void handle_change_delete(struct merge_options *o,
 				 const char *change, const char *change_past)
 {
 	char *renamed = NULL;
-	if (dir_in_way(path, !o->call_depth)) {
+	if (dir_in_way(path, !o->no_worktree)) {
 		renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
 	}
 
@@ -1127,10 +1127,10 @@ static void handle_file(struct merge_options *o,
 		char *add_name = unique_path(o, rename->path, other_branch);
 		update_file(o, 0, add->sha1, add->mode, add_name);
 
-		remove_file(o, 0, rename->path, 0);
+		remove_file(o, 0, rename->path, o->no_worktree);
 		dst_name = unique_path(o, rename->path, cur_branch);
 	} else {
-		if (dir_in_way(rename->path, !o->call_depth)) {
+		if (dir_in_way(rename->path, !o->no_worktree)) {
 			dst_name = unique_path(o, rename->path, cur_branch);
 			output(o, 1, _("%s is a directory in %s adding as %s instead"),
 			       rename->path, other_branch, dst_name);
@@ -1237,7 +1237,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
 		 * merge base just undo the renames; they can be detected
 		 * again later for the non-recursive merge.
 		 */
-		remove_file(o, 0, path, 0);
+		remove_file(o, 0, path, o->no_worktree);
 		update_file(o, 0, mfi_c1.sha, mfi_c1.mode, a->path);
 		update_file(o, 0, mfi_c2.sha, mfi_c2.mode, b->path);
 	} else {
@@ -1245,7 +1245,7 @@ static void conflict_rename_rename_2to1(struct merge_options *o,
 		char *new_path2 = unique_path(o, path, ci->branch2);
 		output(o, 1, _("Renaming %s to %s and %s to %s instead"),
 		       a->path, new_path1, b->path, new_path2);
-		remove_file(o, 0, path, 0);
+		remove_file(o, 0, path, o->no_worktree);
 		update_file(o, 0, mfi_c1.sha, mfi_c1.mode, new_path1);
 		update_file(o, 0, mfi_c2.sha, mfi_c2.mode, new_path2);
 		free(new_path2);
@@ -1404,7 +1404,7 @@ static int process_renames(struct merge_options *o,
 			 * add-source case).
 			 */
 			remove_file(o, 1, ren1_src,
-				    renamed_stage == 2 || !was_tracked(ren1_src));
+				    o->no_worktree || renamed_stage == 2 || !was_tracked(ren1_src));
 
 			hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha);
 			src_other.mode = ren1->src_entry->stages[other_stage].mode;
@@ -1600,7 +1600,7 @@ static int merge_content(struct merge_options *o,
 			 o->branch2 == rename_conflict_info->branch1) ?
 			pair1->two->path : pair1->one->path;
 
-		if (dir_in_way(path, !o->call_depth))
+		if (dir_in_way(path, !o->no_worktree))
 			df_conflict_remains = 1;
 	}
 	mfi = merge_file_special_markers(o, &one, &a, &b,
@@ -1620,7 +1620,7 @@ static int merge_content(struct merge_options *o,
 		path_renamed_outside_HEAD = !path2 || !strcmp(path, path2);
 		if (!path_renamed_outside_HEAD) {
 			add_cacheinfo(mfi.mode, mfi.sha, path,
-				      0, (!o->call_depth), 0);
+				      0, (!o->no_worktree), 0);
 			return mfi.clean;
 		}
 	} else
@@ -1721,7 +1721,7 @@ static int process_entry(struct merge_options *o,
 			if (a_sha)
 				output(o, 2, _("Removing %s"), path);
 			/* do not touch working file if it did not exist */
-			remove_file(o, 1, path, !a_sha);
+			remove_file(o, 1, path, o->no_worktree || !a_sha);
 		} else {
 			/* Modify/delete; deleted side may have put a directory in the way */
 			clean_merge = 0;
@@ -1752,7 +1752,7 @@ static int process_entry(struct merge_options *o,
 			sha = b_sha;
 			conf = _("directory/file");
 		}
-		if (dir_in_way(path, !o->call_depth)) {
+		if (dir_in_way(path, !o->no_worktree)) {
 			char *new_path = unique_path(o, path, add_branch);
 			clean_merge = 0;
 			output(o, 1, _("CONFLICT (%s): There is a directory with name %s in %s. "
@@ -1780,7 +1780,7 @@ static int process_entry(struct merge_options *o,
 		 * this entry was deleted altogether. a_mode == 0 means
 		 * we had that path and want to actively remove it.
 		 */
-		remove_file(o, 1, path, !a_mode);
+		remove_file(o, 1, path, o->no_worktree || !a_mode);
 	} else
 		die(_("Fatal merge failure, shouldn't happen."));
 
@@ -1806,7 +1806,7 @@ int merge_trees(struct merge_options *o,
 		return 1;
 	}
 
-	code = git_merge_trees(o->call_depth, common, head, merge);
+	code = git_merge_trees(o->no_worktree, common, head, merge);
 
 	if (code != 0) {
 		if (show(o, 4) || o->call_depth)
@@ -1915,6 +1915,7 @@ int merge_recursive(struct merge_options *o,
 
 	for (iter = ca; iter; iter = iter->next) {
 		const char *saved_b1, *saved_b2;
+		unsigned saved_no_worktree;
 		o->call_depth++;
 		/*
 		 * When the merge fails, the result contains files
@@ -1924,6 +1925,8 @@ int merge_recursive(struct merge_options *o,
 		 * "conflicts" were already resolved.
 		 */
 		discard_cache();
+		saved_no_worktree = o->no_worktree;
+		o->no_worktree = 1;
 		saved_b1 = o->branch1;
 		saved_b2 = o->branch2;
 		o->branch1 = "Temporary merge branch 1";
@@ -1932,6 +1935,7 @@ int merge_recursive(struct merge_options *o,
 				NULL, &merged_common_ancestors);
 		o->branch1 = saved_b1;
 		o->branch2 = saved_b2;
+		o->no_worktree = saved_no_worktree;
 		o->call_depth--;
 
 		if (!merged_common_ancestors)
diff --git a/merge-recursive.h b/merge-recursive.h
index 9e090a3..d8dd7a1 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -15,6 +15,7 @@ struct merge_options {
 	const char *subtree_shift;
 	unsigned buffer_output : 1;
 	unsigned renormalize : 1;
+	unsigned no_worktree : 1; /* do not touch worktree */
 	long xdl_opts;
 	int verbosity;
 	int diff_rename_limit;
-- 
1.8.3.2.908.gbd0dbd0

[PATCH 3/3] merge-recursive: -Xindex-only to leave worktree unchanged

From: Thomas Rast <hidden>
Date: 2016-06-15 22:58:01

Using the new no_worktree flag from the previous commit, we can teach
merge-recursive to leave the worktree untouched.  Expose this with a
new strategy option so that scripts can use it.
---
 Documentation/merge-strategies.txt |  4 ++++
 merge-recursive.c                  |  2 ++
 t/t3030-merge-recursive.sh         | 13 +++++++++++++
 3 files changed, 19 insertions(+)
diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 49a9a7d..b663a2e 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -92,6 +92,10 @@ subtree[=<path>];;
 	is prefixed (or stripped from the beginning) to make the shape of
 	two trees to match.
 
+index-only;;
+	Write the merge result only to the index; do not touch the
+	worktree.
+
 octopus::
 	This resolves cases with more than two heads, but refuses to do
 	a complex merge that needs manual resolution.  It is
diff --git a/merge-recursive.c b/merge-recursive.c
index b93b762..a58d691 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2094,6 +2094,8 @@ int parse_merge_opt(struct merge_options *o, const char *s)
 		if ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0)
 			return -1;
 	}
+	else if (!strcmp(s, "index-only"))
+		o->no_worktree = 1;
 	else
 		return -1;
 	return 0;
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 2f96100..2f3a16c 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -296,6 +296,19 @@ test_expect_success 'merge-recursive result' '
 
 '
 
+test_expect_success 'merge-recursive --index-only' '
+
+	rm -fr [abcd] &&
+	git checkout -f "$c2" &&
+	test_expect_code 1 git merge-recursive --index-only "$c0" -- "$c2" "$c1" &&
+	git ls-files -s >actual &&
+	# reuses "expected" from previous test!
+	test_cmp expected actual &&
+	git diff HEAD >actual-diff &&
+	: >expected-diff &&
+	test_cmp expected-diff actual-diff
+'
+
 test_expect_success 'fail if the index has unresolved entries' '
 
 	rm -fr [abcd] &&
-- 
1.8.3.2.908.gbd0dbd0

Re: [PATCH 0/3] merge -Xindex-only

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:58:02

[Resend because of address confusion in replied-to email.]

On 07/07/2013 08:00 PM, Thomas Rast wrote:
I recently looked into making merge-recursive more useful as a modular
piece in various tasks, e.g. Michael's git-imerge and the experiments
I made in showing evil merges.

This miniseries is the extremely low-hanging fruit.  If it makes a
good first step for git-imerge, perhaps it can go in like this.  It's
not a big speedup (about 2.2s vs 2.4s in a sample conflicting merge in
git.git), but it does feel much cleaner to avoid touching the worktree
unless actually necessary.

Otherwise it's probably not worth it just yet; for what I want to do
with it, we need some more reshuffling of things.
Interesting.

For git-imerge, it would be nice to speed up merges by skipping the
working tree updates.  10% might not be so noticeable, but every little
bit helps :-)

But the killer benefit would be if git-imerge could do some of its
automatic merge-testing and autofilling in the background while the user
is resolving conflicts in the main index and working tree.

Is it possible to use this option with an alternate index file (e.g.,
via the GIT_INDEX_FILE environment variable)?  Can it be made to leave
other shared state (e.g., MERGE_HEAD) alone?  If so, maybe it's already
there.

For this feature to be useful for test merges, it would be enough to
just get a retcode saying whether a given merge would succeed or conflict.

For it to be used for autofilling, it would also be necessary to be able
to create a commit from the merge result in the alternate index (this
would only be done when there are no conflicts).  I assume this part can
be done in the usual way using "git commit-tree".

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help