[PATCH 0/3] merge -Xindex-only

[PATCH 0/3] merge -Xindex-only

[Thomas Rast]

[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()

[Thomas Rast]

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 <trast@inf.ethz.ch>
---
 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

[Thomas Rast]

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 <trast@inf.ethz.ch>
---
 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

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

[Junio C Hamano]

Thomas Rast <trast@inf.ethz.ch> writes:

> 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.

I do remember discussing this with you the other day, and while I do
agree that we may not want to touch the working tree in the
outermost merge (i.e. o->call_depth is zero) in some applications, I
do not think of a situation where you _do_ want to touch working
tree while performing the inner merge.  I'd feel safer if the code
said that "no matter what no-worktree option says, we won't touch
the working tree if o->call_depth is not zero" clearly in some way,
e.g. 

	if (o->call_depth || o->in_index_merge)
        	return; /* leave without touching working tree */

In other words, I do not like the part of the code that pretends
these two are independent options, when what we really want is to
have two modes for the outermost (o->call_depth == 0) case.

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

[Thomas Rast]

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

[Michael Haggerty]

[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/

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

[Thomas Rast]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> [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.

GIT_INDEX_FILE yes, that one works out of the box.

I think for the shared state, the following is a (probably) ridiculously
unsupported yet magic way of achieving this:

  mkdir -p unshared/.git
  cd unshared/.git
  for f in ../../.git/*; do
    case "$f" in
      *HEAD | index)
        cp "$f" .
        ;;
      *)
        ln -s "$f" .
        ;;
    esac
  done

That gives you a repository that propagates ref changes and object
writing, but does not propagate changes to index, HEAD, FETCH_HEAD or
MERGE_HEAD.  Which might just be what you need?

Note that as far as I'm concerned, this is a live handgrenade.  It could
blow up in your face at any time, but it probably has its applications...

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

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

[Michael Haggerty]

On 07/08/2013 05:44 PM, Thomas Rast wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> [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.
> 
> GIT_INDEX_FILE yes, that one works out of the box.
> 
> I think for the shared state, the following is a (probably) ridiculously
> unsupported yet magic way of achieving this:
> 
>   mkdir -p unshared/.git
>   cd unshared/.git
>   for f in ../../.git/*; do
>     case "$f" in
>       *HEAD | index)
>         cp "$f" .
>         ;;
>       *)
>         ln -s "$f" .
>         ;;
>     esac
>   done
> 
> That gives you a repository that propagates ref changes and object
> writing, but does not propagate changes to index, HEAD, FETCH_HEAD or
> MERGE_HEAD.  Which might just be what you need?
> 
> Note that as far as I'm concerned, this is a live handgrenade.  It could
> blow up in your face at any time, but it probably has its applications...

I might consider such a thing for my own use, but I don't think I'll lob
live hand grenades at innocent git-imerge users :-)

Since you've already implemented a way to merge into the index (even an
alternative index) without touching the working copy, I'll just cross my
fingers and hope for the appearance of an option that makes merge leave
HEAD, MERGE_HEAD, etc. untouched.

It's not like I have any free time to work on git-imerge anyway :-(

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

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

[Thomas Rast]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> Since you've already implemented a way to merge into the index (even an
> alternative index) without touching the working copy, I'll just cross my
> fingers and hope for the appearance of an option that makes merge leave
> HEAD, MERGE_HEAD, etc. untouched.

The most annoying part is probably where to put the output, since
merging is more or less defined to do one of:

- update HEAD and return 0
- update MERGE_HEAD and return 1

I'm not sure how much flexibility is worth having.  Would it be
sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that
changes it to:

- update refs/heads/foo and return 0
- return 1, not updating any refs

That would mean that it would only work for noninteractive use.  In the
conflicting case, the driving script would need to remember what it
wanted to merge so as have the information when finally committing.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

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

[Michael Haggerty]

On 07/09/2013 02:08 PM, Thomas Rast wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> Since you've already implemented a way to merge into the index (even an
>> alternative index) without touching the working copy, I'll just cross my
>> fingers and hope for the appearance of an option that makes merge leave
>> HEAD, MERGE_HEAD, etc. untouched.
> 
> The most annoying part is probably where to put the output, since
> merging is more or less defined to do one of:
> 
> - update HEAD and return 0
> - update MERGE_HEAD and return 1

I don't understand what you mean here.  Why does *any* reference need to
be updated?  Why not

* load arbitrary commit-ish A into index

* merge arbitrary commit-ish B into index

* return error/OK depending on whether there was a conflict

?  The script that started the whole process would know what A and B are
and could create the commit itself using "git write-tree" and "git
commit-tree -p A -p B".  And if the index were an alternative index
chosen via GIT_INDEX_FILE then the rest of the git repo would be none
the wiser.

> I'm not sure how much flexibility is worth having.  Would it be
> sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that
> changes it to:
> 
> - update refs/heads/foo and return 0
> - return 1, not updating any refs
> 
> That would mean that it would only work for noninteractive use.  In the
> conflicting case, the driving script would need to remember what it
> wanted to merge so as have the information when finally committing.

That would be fine with me.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

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

[Thomas Rast]

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 07/09/2013 02:08 PM, Thomas Rast wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> 
>>> Since you've already implemented a way to merge into the index (even an
>>> alternative index) without touching the working copy, I'll just cross my
>>> fingers and hope for the appearance of an option that makes merge leave
>>> HEAD, MERGE_HEAD, etc. untouched.
>> 
>> The most annoying part is probably where to put the output, since
>> merging is more or less defined to do one of:
>> 
>> - update HEAD and return 0
>> - update MERGE_HEAD and return 1
>
> I don't understand what you mean here. [...]

I was simply trying to describe what the status quo is, as a basis for
the next paragraph.  Does that clarify it?

>> I'm not sure how much flexibility is worth having.  Would it be
>> sufficient if you had an option, e.g. -Xresult-ref=refs/heads/foo, that
>> changes it to:
>> 
>> - update refs/heads/foo and return 0
>> - return 1, not updating any refs
>> 
>> That would mean that it would only work for noninteractive use.  In the
>> conflicting case, the driving script would need to remember what it
>> wanted to merge so as have the information when finally committing.
>
> That would be fine with me.

On IRC you said you would like a version that always acts as
--no-commit, and simply returns the conflict/no conflict bit as usual.
The caller would then proceed using commit-tree itself.  I think that is
probably a saner solution than this "output ref" idea.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

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

[Thomas Rast]

Thomas Rast <trast@inf.ethz.ch> writes:

> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
> On IRC you said you would like a version that always acts as
> --no-commit, and simply returns the conflict/no conflict bit as usual.
> The caller would then proceed using commit-tree itself.  I think that is
> probably a saner solution than this "output ref" idea.

I just had a huge facepalm moment.  We already have this option.  It is
called git-merge-recursive.

That is,

  git merge-recursive $(git merge-base --all HEAD other) -- HEAD other

will internally do all the work that 'git merge other' would do, but not
update any refs.  With this series, you can therefore say

  git merge-recursive --index-only $(git merge-base --all HEAD other) -- HEAD other

and get an *index-only* merge of HEAD and other.

Can you see if this is enough to build git-imerge on top of it?
Otherwise I'm glad to help with building the git-merge infrastucture to
support it.

I'll send v2 of the series in a minute; the only change is that I
changed the internal flag semantics as per Junio's comment in

  http://thread.gmane.org/gmane.comp.version-control.git/229787/focus=229797

-- 
Thomas Rast
tr@thomasrast.ch

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

[Thomas Rast]

From: Thomas Rast <trast@inf.ethz.ch>

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 <trast@inf.ethz.ch>
---

Unchanged.

 merge-recursive.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index dbb7104..bdf69cb 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -545,11 +545,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.4.1.841.gb1dcd95

[PATCH 2/3] merge-recursive: internal flag to avoid touching the worktree

[Thomas Rast]

From: Thomas Rast <trast@inf.ethz.ch>

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 to
trigger only the latter.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---

Adapted as per Junio's comments in

  http://thread.gmane.org/gmane.comp.version-control.git/229787/focus=229797

I'm not sure which way is better; the downside of this version is that
we now have the burden of ensuring that o->no_worktree always remains
a strict subset of o->call_depth whenever we touch this code.


 merge-recursive.c | 37 +++++++++++++++++++++----------------
 merge-recursive.h |  1 +
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index bdf69cb..922a259 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -408,10 +408,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->call_depth || o->no_worktree)
 		return;
 
 	/* Ensure D/F conflicts are adjacent in the entries list. */
@@ -724,7 +724,7 @@ static void update_file_flags(struct merge_options *o,
 			      int update_cache,
 			      int update_wd)
 {
-	if (o->call_depth)
+	if (o->call_depth || o->no_worktree)
 		update_wd = 0;
 
 	if (update_wd) {
@@ -931,7 +931,8 @@ 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->call_depth ||
+							 o->no_worktree));
 		} else if (S_ISLNK(a->mode)) {
 			hashcpy(result.sha, a->sha1);
 
@@ -1003,7 +1004,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->call_depth || o->no_worktree))) {
 		renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
 	}
 
@@ -1128,10 +1129,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->call_depth || 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->call_depth || 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);
@@ -1238,7 +1239,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->call_depth || 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 {
@@ -1246,7 +1247,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->call_depth || 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);
@@ -1405,6 +1406,7 @@ static int process_renames(struct merge_options *o,
 			 * add-source case).
 			 */
 			remove_file(o, 1, ren1_src,
+				    o->call_depth || o->no_worktree ||
 				    renamed_stage == 2 || !was_tracked(ren1_src));
 
 			hashcpy(src_other.sha1, ren1->src_entry->stages[other_stage].sha);
@@ -1601,7 +1603,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->call_depth || o->no_worktree)))
 			df_conflict_remains = 1;
 	}
 	mfi = merge_file_special_markers(o, &one, &a, &b,
@@ -1621,7 +1623,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->call_depth || o->no_worktree), 0);
 			return mfi.clean;
 		}
 	} else
@@ -1722,7 +1724,8 @@ 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->call_depth || o->no_worktree || !a_sha);
 		} else {
 			/* Modify/delete; deleted side may have put a directory in the way */
 			clean_merge = 0;
@@ -1753,7 +1756,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->call_depth || 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. "
@@ -1781,7 +1784,8 @@ 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->call_depth || o->no_worktree || !a_mode);
 	} else
 		die(_("Fatal merge failure, shouldn't happen."));
 
@@ -1807,7 +1811,8 @@ int merge_trees(struct merge_options *o,
 		return 1;
 	}
 
-	code = git_merge_trees(o->call_depth, common, head, merge);
+	code = git_merge_trees(o->call_depth || o->no_worktree,
+			       common, head, merge);
 
 	if (code != 0) {
 		if (show(o, 4) || o->call_depth)
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.4.1.841.gb1dcd95

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

[Thomas Rast]

From: Thomas Rast <trast@inf.ethz.ch>

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.
---

Unchanged.

 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 922a259..addfb72 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2096,6 +2096,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.4.1.841.gb1dcd95