All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] log --remerge-diff
@ 2014-02-22  9:17 Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 1/8] merge-recursive: remove dead conditional in update_stages() Thomas Rast
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

This is the second iteration of

  http://thread.gmane.org/gmane.comp.version-control.git/241565

Changes since the last version:

* Dropped patches 4 and 5 (log --merge-bases)

* Implemented the "full-file conflict" scheme explained in the
  previous cover letter: if the conflicted version is lacking one
  stage of a file, it synthesizes a conflict file of the form

    <<<<<<<
    =======
    content of the stage that exists
    >>>>>>>

  (or the other way around if stage3 is missing).  This occurs at
  least with delete/modify conflicts.

* Implemented some basic handling of directory/file conflicts.  I'm
  not completely happy yet -- see the NEEDSWORK comments -- but at
  least it gives consistent input to the diffing stage.

  This required access to the dir hash, so there's a new patch 7 that
  makes this possible.

Patches 1-6 (used to be 1-3 and 6-8) are unchanged.


Thomas Rast (8):
  merge-recursive: remove dead conditional in update_stages()
  merge-recursive: internal flag to avoid touching the worktree
  merge-recursive: -Xindex-only to leave worktree unchanged
  combine-diff: do not pass revs->dense_combined_merges redundantly
  Fold all merge diff variants into an enum
  merge-recursive: allow storing conflict hunks in index
  name-hash: allow dir hashing even when !ignore_case
  log --remerge-diff: show what the conflict resolution changed

 Documentation/merge-strategies.txt |   9 ++
 Documentation/rev-list-options.txt |   7 +
 builtin/diff-files.c               |   5 +-
 builtin/diff-tree.c                |   2 +-
 builtin/diff.c                     |  12 +-
 builtin/fmt-merge-msg.c            |   2 +-
 builtin/log.c                      |   9 +-
 builtin/merge.c                    |   1 -
 cache.h                            |   2 +
 combine-diff.c                     |  13 +-
 diff-lib.c                         |  13 +-
 diff.h                             |   6 +-
 log-tree.c                         | 304 ++++++++++++++++++++++++++++++++++++-
 merge-recursive.c                  |  52 ++++---
 merge-recursive.h                  |   3 +
 name-hash.c                        |  19 ++-
 revision.c                         |  15 +-
 revision.h                         |  24 ++-
 submodule.c                        |   3 +-
 t/t3030-merge-recursive.sh         |  33 ++++
 t/t4213-log-remerge-diff.sh        | 222 +++++++++++++++++++++++++++
 21 files changed, 678 insertions(+), 78 deletions(-)
 create mode 100755 t/t4213-log-remerge-diff.sh

-- 
1.9.0.313.g3d0a325

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/8] merge-recursive: remove dead conditional in update_stages()
  2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
@ 2014-02-22  9:17 ` Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 2/8] merge-recursive: internal flag to avoid touching the worktree Thomas Rast
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, 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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 merge-recursive.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 8400a8e..c36dc79 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.9.0.313.g3d0a325

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/8] merge-recursive: internal flag to avoid touching the worktree
  2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 1/8] merge-recursive: remove dead conditional in update_stages() Thomas Rast
@ 2014-02-22  9:17 ` Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 3/8] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, 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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 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 c36dc79..35be144 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.9.0.313.g3d0a325

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/8] merge-recursive: -Xindex-only to leave worktree unchanged
  2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 1/8] merge-recursive: remove dead conditional in update_stages() Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 2/8] merge-recursive: internal flag to avoid touching the worktree Thomas Rast
@ 2014-02-22  9:17 ` Thomas Rast
  2014-02-23  9:07   ` Eric Sunshine
  2014-02-22  9:17 ` [PATCH v2 4/8] combine-diff: do not pass revs->dense_combined_merges redundantly Thomas Rast
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, 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.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 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 fb6e593..2934e99 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 35be144..f59c1d3 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.9.0.313.g3d0a325

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/8] combine-diff: do not pass revs->dense_combined_merges redundantly
  2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
                   ` (2 preceding siblings ...)
  2014-02-22  9:17 ` [PATCH v2 3/8] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast
@ 2014-02-22  9:17 ` Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 5/8] Fold all merge diff variants into an enum Thomas Rast
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The existing code passed revs->dense_combined_merges along revs itself
into the combine-diff functions, which is rather redundant.  Remove
the 'dense' argument until much further down the callchain to simplify
callers.

Note that while the caller in submodule.c needs to do extra work now,
the next commit will simplify this to a single setting again.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 builtin/diff.c |  3 +--
 combine-diff.c | 13 ++++++-------
 diff-lib.c     |  6 ++----
 diff.h         |  6 +++---
 log-tree.c     |  2 +-
 submodule.c    |  5 ++++-
 6 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 0f247d2..47f663b 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -196,8 +196,7 @@ static int builtin_diff_combined(struct rev_info *revs,
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	for (i = 1; i < ents; i++)
 		sha1_array_append(&parents, ent[i].item->sha1);
-	diff_tree_combined(ent[0].item->sha1, &parents,
-			   revs->dense_combined_merges, revs);
+	diff_tree_combined(ent[0].item->sha1, &parents, revs);
 	sha1_array_clear(&parents);
 	return 0;
 }
diff --git a/combine-diff.c b/combine-diff.c
index 3b92c448..6e80a73 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -952,7 +952,7 @@ static void show_combined_header(struct combine_diff_path *elem,
 }
 
 static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
-			    int dense, int working_tree_file,
+			    int working_tree_file,
 			    struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
@@ -967,6 +967,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	struct userdiff_driver *textconv = NULL;
 	int is_binary;
 	const char *line_prefix = diff_line_prefix(opt);
+	int dense = rev->dense_combined_merges;
 
 	context = opt->context;
 	userdiff = userdiff_find_by_path(elem->path);
@@ -1214,7 +1215,6 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re
  */
 void show_combined_diff(struct combine_diff_path *p,
 		       int num_parent,
-		       int dense,
 		       struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
@@ -1226,7 +1226,7 @@ void show_combined_diff(struct combine_diff_path *p,
 				  DIFF_FORMAT_NAME_STATUS))
 		show_raw_diff(p, num_parent, rev);
 	else if (opt->output_format & DIFF_FORMAT_PATCH)
-		show_patch_diff(p, num_parent, dense, 1, rev);
+		show_patch_diff(p, num_parent, 1, rev);
 }
 
 static void free_combined_pair(struct diff_filepair *pair)
@@ -1297,7 +1297,6 @@ static void handle_combined_callback(struct diff_options *opt,
 
 void diff_tree_combined(const unsigned char *sha1,
 			const struct sha1_array *parents,
-			int dense,
 			struct rev_info *rev)
 {
 	struct diff_options *opt = &rev->diffopt;
@@ -1365,7 +1364,7 @@ void diff_tree_combined(const unsigned char *sha1,
 				       opt->line_termination);
 			for (p = paths; p; p = p->next) {
 				if (p->len)
-					show_patch_diff(p, num_parent, dense,
+					show_patch_diff(p, num_parent,
 							0, rev);
 			}
 		}
@@ -1381,7 +1380,7 @@ void diff_tree_combined(const unsigned char *sha1,
 	free_pathspec(&diffopts.pathspec);
 }
 
-void diff_tree_combined_merge(const struct commit *commit, int dense,
+void diff_tree_combined_merge(const struct commit *commit,
 			      struct rev_info *rev)
 {
 	struct commit_list *parent = get_saved_parents(rev, commit);
@@ -1391,6 +1390,6 @@ void diff_tree_combined_merge(const struct commit *commit, int dense,
 		sha1_array_append(&parents, parent->item->object.sha1);
 		parent = parent->next;
 	}
-	diff_tree_combined(commit->object.sha1, &parents, dense, rev);
+	diff_tree_combined(commit->object.sha1, &parents, rev);
 	sha1_array_clear(&parents);
 }
diff --git a/diff-lib.c b/diff-lib.c
index 346cac6..8d0f572 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -174,9 +174,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			i--;
 
 			if (revs->combine_merges && num_compare_stages == 2) {
-				show_combined_diff(dpath, 2,
-						   revs->dense_combined_merges,
-						   revs);
+				show_combined_diff(dpath, 2, revs);
 				free(dpath);
 				continue;
 			}
@@ -338,7 +336,7 @@ static int show_modified(struct rev_info *revs,
 		p->parent[1].status = DIFF_STATUS_MODIFIED;
 		p->parent[1].mode = old->ce_mode;
 		hashcpy(p->parent[1].sha1, old->sha1);
-		show_combined_diff(p, 2, revs->dense_combined_merges, revs);
+		show_combined_diff(p, 2, revs);
 		free(p);
 		return 0;
 	}
diff --git a/diff.h b/diff.h
index ce123fa..ff77802 100644
--- a/diff.h
+++ b/diff.h
@@ -213,11 +213,11 @@ struct combine_diff_path {
 	 sizeof(struct combine_diff_parent) * (n) + (l) + 1)
 
 extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,
-			      int dense, struct rev_info *);
+			       struct rev_info *);
 
-extern void diff_tree_combined(const unsigned char *sha1, const struct sha1_array *parents, int dense, struct rev_info *rev);
+extern void diff_tree_combined(const unsigned char *sha1, const struct sha1_array *parents, struct rev_info *rev);
 
-extern void diff_tree_combined_merge(const struct commit *commit, int dense, struct rev_info *rev);
+extern void diff_tree_combined_merge(const struct commit *commit, struct rev_info *rev);
 
 void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);
 
diff --git a/log-tree.c b/log-tree.c
index 08970bf..4c04812 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -718,7 +718,7 @@ int log_tree_diff_flush(struct rev_info *opt)
 
 static int do_diff_combined(struct rev_info *opt, struct commit *commit)
 {
-	diff_tree_combined_merge(commit, opt->dense_combined_merges, opt);
+	diff_tree_combined_merge(commit, opt);
 	return !opt->loginfo;
 }
 
diff --git a/submodule.c b/submodule.c
index 613857e..83b80fb 100644
--- a/submodule.c
+++ b/submodule.c
@@ -505,10 +505,13 @@ static void find_unpushed_submodule_commits(struct commit *commit,
 	struct rev_info rev;
 
 	init_revisions(&rev, NULL);
+	rev.ignore_merges = 0;
+	rev.combined_merges = 1;
+	rev.dense_combined_merges = 1;
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = collect_submodules_from_diff;
 	rev.diffopt.format_callback_data = needs_pushing;
-	diff_tree_combined_merge(commit, 1, &rev);
+	diff_tree_combined_merge(commit, &rev);
 }
 
 int find_unpushed_submodules(unsigned char new_sha1[20],
-- 
1.9.0.313.g3d0a325

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/8] Fold all merge diff variants into an enum
  2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
                   ` (3 preceding siblings ...)
  2014-02-22  9:17 ` [PATCH v2 4/8] combine-diff: do not pass revs->dense_combined_merges redundantly Thomas Rast
@ 2014-02-22  9:17 ` Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 6/8] merge-recursive: allow storing conflict hunks in index Thomas Rast
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The four ways of displaying merge diffs,

* none: no diff
* -m: against each parent
* -c: combined
* --cc: combined-condensed

were encoded in three flag bits in struct rev_info.  Fold them all
into a single enum field that captures the variants.

This makes it easier to add new merge diff variants without yet more
special casing.  It should also be slightly easier to read because one
does not have to ensure that the flag bits are set in an expected
combination.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 builtin/diff-files.c    |  5 +++--
 builtin/diff-tree.c     |  2 +-
 builtin/diff.c          |  9 +++++----
 builtin/fmt-merge-msg.c |  2 +-
 builtin/log.c           |  9 ++++-----
 builtin/merge.c         |  1 -
 combine-diff.c          |  2 +-
 diff-lib.c              |  7 ++++---
 log-tree.c              |  4 ++--
 revision.c              | 13 +++----------
 revision.h              | 22 +++++++++++++++++++---
 submodule.c             |  4 +---
 12 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 9200069..172b50d 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -57,9 +57,10 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	 * was not asked to.  "diff-files -c -p" should not densify
 	 * (the user should ask with "diff-files --cc" explicitly).
 	 */
-	if (rev.max_count == -1 && !rev.combine_merges &&
+	if (rev.max_count == -1 &&
+	    !merge_diff_mode_is_any_combined(&rev) &&
 	    (rev.diffopt.output_format & DIFF_FORMAT_PATCH))
-		rev.combine_merges = rev.dense_combined_merges = 1;
+		rev.merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;
 
 	if (read_cache_preload(&rev.diffopt.pathspec) < 0) {
 		perror("read_cache_preload");
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index be6417d..2950f80 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -96,7 +96,7 @@ static int diff_tree_stdin(char *line)
 static void diff_tree_tweak_rev(struct rev_info *rev, struct setup_revision_opt *opt)
 {
 	if (!rev->diffopt.output_format) {
-		if (rev->dense_combined_merges)
+		if (rev->merge_diff_mode == MERGE_DIFF_COMBINED_CONDENSED)
 			rev->diffopt.output_format = DIFF_FORMAT_PATCH;
 		else
 			rev->diffopt.output_format = DIFF_FORMAT_RAW;
diff --git a/builtin/diff.c b/builtin/diff.c
index 47f663b..fd4c75f 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -192,8 +192,8 @@ static int builtin_diff_combined(struct rev_info *revs,
 	if (argc > 1)
 		usage(builtin_diff_usage);
 
-	if (!revs->dense_combined_merges && !revs->combine_merges)
-		revs->dense_combined_merges = revs->combine_merges = 1;
+	if (!merge_diff_mode_is_any_combined(revs))
+		revs->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;
 	for (i = 1; i < ents; i++)
 		sha1_array_append(&parents, ent[i].item->sha1);
 	diff_tree_combined(ent[0].item->sha1, &parents, revs);
@@ -242,9 +242,10 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
 	 * dense one, --cc can be explicitly asked for, or just rely
 	 * on the default).
 	 */
-	if (revs->max_count == -1 && !revs->combine_merges &&
+	if (revs->max_count == -1 &&
+	    !merge_diff_mode_is_any_combined(revs) &&
 	    (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
-		revs->combine_merges = revs->dense_combined_merges = 1;
+		revs->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;
 
 	setup_work_tree();
 	if (read_cache_preload(&revs->diffopt.pathspec) < 0) {
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 3906eda..2deeacd 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -637,7 +637,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		head = lookup_commit_or_die(head_sha1, "HEAD");
 		init_revisions(&rev, NULL);
 		rev.commit_format = CMIT_FMT_ONELINE;
-		rev.ignore_merges = 1;
+		rev.merge_diff_mode = MERGE_DIFF_IGNORE;
 		rev.limited = 1;
 
 		strbuf_complete_line(out);
diff --git a/builtin/log.c b/builtin/log.c
index b97373d..cebebea 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -499,13 +499,12 @@ static int show_tree_object(const unsigned char *sha1,
 
 static void show_rev_tweak_rev(struct rev_info *rev, struct setup_revision_opt *opt)
 {
-	if (rev->ignore_merges) {
+	if (!rev->merge_diff_mode) {
 		/* There was no "-m" on the command line */
-		rev->ignore_merges = 0;
-		if (!rev->first_parent_only && !rev->combine_merges) {
+		rev->merge_diff_mode = MERGE_DIFF_EACH;
+		if (!rev->first_parent_only) {
 			/* No "--first-parent", "-c", nor "--cc" */
-			rev->combine_merges = 1;
-			rev->dense_combined_merges = 1;
+			rev->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;
 		}
 	}
 	if (!rev->diffopt.output_format)
diff --git a/builtin/merge.c b/builtin/merge.c
index e576a7f..6977af7 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -343,7 +343,6 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead
 		die_errno(_("Could not write to '%s'"), filename);
 
 	init_revisions(&rev, NULL);
-	rev.ignore_merges = 1;
 	rev.commit_format = CMIT_FMT_MEDIUM;
 
 	commit->object.flags |= UNINTERESTING;
diff --git a/combine-diff.c b/combine-diff.c
index 6e80a73..3fae2dd 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -967,7 +967,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	struct userdiff_driver *textconv = NULL;
 	int is_binary;
 	const char *line_prefix = diff_line_prefix(opt);
-	int dense = rev->dense_combined_merges;
+	int dense = (rev->merge_diff_mode == MERGE_DIFF_COMBINED_CONDENSED);
 
 	context = opt->context;
 	userdiff = userdiff_find_by_path(elem->path);
diff --git a/diff-lib.c b/diff-lib.c
index 8d0f572..e2700eb 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -173,7 +173,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			 */
 			i--;
 
-			if (revs->combine_merges && num_compare_stages == 2) {
+			if (merge_diff_mode_is_any_combined(revs) &&
+			    num_compare_stages == 2) {
 				show_combined_diff(dpath, 2, revs);
 				free(dpath);
 				continue;
@@ -316,7 +317,7 @@ static int show_modified(struct rev_info *revs,
 		return -1;
 	}
 
-	if (revs->combine_merges && !cached &&
+	if (merge_diff_mode_is_any_combined(revs) && !cached &&
 	    (hashcmp(sha1, old->sha1) || hashcmp(old->sha1, new->sha1))) {
 		struct combine_diff_path *p;
 		int pathlen = ce_namelen(new);
@@ -375,7 +376,7 @@ static void do_oneway_diff(struct unpack_trees_options *o,
 	 * But with the revision flag parsing, that's found in
 	 * "!revs->ignore_merges".
 	 */
-	match_missing = !revs->ignore_merges;
+	match_missing = (revs->merge_diff_mode == MERGE_DIFF_EACH);
 
 	if (cached && idx && ce_stage(idx)) {
 		struct diff_filepair *pair;
diff --git a/log-tree.c b/log-tree.c
index 4c04812..30b3063 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -751,9 +751,9 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 
 	/* More than one parent? */
 	if (parents && parents->next) {
-		if (opt->ignore_merges)
+		if (opt->merge_diff_mode == MERGE_DIFF_IGNORE)
 			return 0;
-		else if (opt->combine_merges)
+		else if (merge_diff_mode_is_any_combined(opt))
 			return do_diff_combined(opt, commit);
 		else if (opt->first_parent_only) {
 			/*
diff --git a/revision.c b/revision.c
index a0df72f..127b75a 100644
--- a/revision.c
+++ b/revision.c
@@ -1329,7 +1329,6 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	memset(revs, 0, sizeof(*revs));
 
 	revs->abbrev = DEFAULT_ABBREV;
-	revs->ignore_merges = 1;
 	revs->simplify_history = 1;
 	DIFF_OPT_SET(&revs->pruning, RECURSIVE);
 	DIFF_OPT_SET(&revs->pruning, QUICK);
@@ -1807,15 +1806,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
 		DIFF_OPT_SET(&revs->diffopt, TREE_IN_RECURSIVE);
 	} else if (!strcmp(arg, "-m")) {
-		revs->ignore_merges = 0;
+		revs->merge_diff_mode = MERGE_DIFF_EACH;
 	} else if (!strcmp(arg, "-c")) {
-		revs->diff = 1;
-		revs->dense_combined_merges = 0;
-		revs->combine_merges = 1;
+		revs->merge_diff_mode = MERGE_DIFF_COMBINED;
 	} else if (!strcmp(arg, "--cc")) {
-		revs->diff = 1;
-		revs->dense_combined_merges = 1;
-		revs->combine_merges = 1;
+		revs->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;
 	} else if (!strcmp(arg, "-v")) {
 		revs->verbose_header = 1;
 	} else if (!strcmp(arg, "--pretty")) {
@@ -2228,8 +2223,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 			copy_pathspec(&revs->diffopt.pathspec,
 				      &revs->prune_data);
 	}
-	if (revs->combine_merges)
-		revs->ignore_merges = 0;
 	revs->diffopt.abbrev = revs->abbrev;
 
 	if (revs->line_level_traverse) {
diff --git a/revision.h b/revision.h
index 88967d6..0847902 100644
--- a/revision.h
+++ b/revision.h
@@ -50,6 +50,17 @@ struct rev_cmdline_info {
 #define REVISION_WALK_NO_WALK_SORTED 1
 #define REVISION_WALK_NO_WALK_UNSORTED 2
 
+enum merge_diff_mode {
+	/* default: do not show diffs for merge */
+	MERGE_DIFF_IGNORE = 0,
+	/* diff against each side (-m) */
+	MERGE_DIFF_EACH,
+	/* combined format (-c) */
+	MERGE_DIFF_COMBINED,
+	/* combined-condensed format (-cc) */
+	MERGE_DIFF_COMBINED_CONDENSED
+};
+
 struct rev_info {
 	/* Starting list */
 	struct commit_list *commits;
@@ -116,11 +127,10 @@ struct rev_info {
 			show_root_diff:1,
 			no_commit_id:1,
 			verbose_header:1,
-			ignore_merges:1,
-			combine_merges:1,
-			dense_combined_merges:1,
 			always_show_header:1;
 
+	enum merge_diff_mode merge_diff_mode;
+
 	/* Format info */
 	unsigned int	shown_one:1,
 			shown_dashes:1,
@@ -197,6 +207,12 @@ struct rev_info {
 	struct saved_parents *saved_parents_slab;
 };
 
+static inline int merge_diff_mode_is_any_combined(struct rev_info *revs)
+{
+	return (revs->merge_diff_mode == MERGE_DIFF_COMBINED ||
+		revs->merge_diff_mode == MERGE_DIFF_COMBINED_CONDENSED);
+}
+
 extern int ref_excluded(struct string_list *, const char *path);
 void clear_ref_exclusion(struct string_list **);
 void add_ref_exclusion(struct string_list **, const char *exclude);
diff --git a/submodule.c b/submodule.c
index 83b80fb..38973f2 100644
--- a/submodule.c
+++ b/submodule.c
@@ -505,9 +505,7 @@ static void find_unpushed_submodule_commits(struct commit *commit,
 	struct rev_info rev;
 
 	init_revisions(&rev, NULL);
-	rev.ignore_merges = 0;
-	rev.combined_merges = 1;
-	rev.dense_combined_merges = 1;
+	rev.merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;
 	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = collect_submodules_from_diff;
 	rev.diffopt.format_callback_data = needs_pushing;
-- 
1.9.0.313.g3d0a325

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/8] merge-recursive: allow storing conflict hunks in index
  2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
                   ` (4 preceding siblings ...)
  2014-02-22  9:17 ` [PATCH v2 5/8] Fold all merge diff variants into an enum Thomas Rast
@ 2014-02-22  9:17 ` Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case Thomas Rast
  2014-02-22  9:17 ` [PATCH v2 8/8] log --remerge-diff: show what the conflict resolution changed Thomas Rast
  7 siblings, 0 replies; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Add a --conflicts-in-index option to merge-recursive, which instructs
it to always store the 3-way merged result in the index.  (Normally it
only does so in recursive invocations, but not for the final result.)

This serves as a building block for the "remerge diff" feature coming
up in a subsequent patch.  The external option lets us easily use it
from tests, where we'd otherwise need a new test-* helper to access
the feature.

Furthermore, it might occasionally be useful for scripts that want to
look at the result of invoking git-merge without tampering with the
worktree.  They could already get the _conflicts_ with --index-only,
but not (conveniently) the conflict-hunk formatted files that would
normally be written to the worktree.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 Documentation/merge-strategies.txt |  5 +++++
 merge-recursive.c                  |  4 ++++
 merge-recursive.h                  |  1 +
 t/t3030-merge-recursive.sh         | 20 ++++++++++++++++++++
 4 files changed, 30 insertions(+)

diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 2934e99..3468d99 100644
--- a/Documentation/merge-strategies.txt
+++ b/Documentation/merge-strategies.txt
@@ -96,6 +96,11 @@ index-only;;
 	Write the merge result only to the index; do not touch the
 	worktree.
 
+conflicts-in-index;;
+	For conflicted files, write 3-way merged contents with
+	conflict hunks to the index, instead of leaving their entries
+	unresolved.
+
 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 f59c1d3..b682812 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -724,6 +724,8 @@ static void update_file_flags(struct merge_options *o,
 			      int update_cache,
 			      int update_wd)
 {
+	if (o->conflicts_in_index)
+		update_cache = 1;
 	if (o->call_depth || o->no_worktree)
 		update_wd = 0;
 
@@ -2098,6 +2100,8 @@ int parse_merge_opt(struct merge_options *o, const char *s)
 	}
 	else if (!strcmp(s, "index-only"))
 		o->no_worktree = 1;
+	else if (!strcmp(s, "conflicts-in-index"))
+		o->conflicts_in_index = 1;
 	else
 		return -1;
 	return 0;
diff --git a/merge-recursive.h b/merge-recursive.h
index d8dd7a1..9b8e20b 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -16,6 +16,7 @@ struct merge_options {
 	unsigned buffer_output : 1;
 	unsigned renormalize : 1;
 	unsigned no_worktree : 1; /* do not touch worktree */
+	unsigned conflicts_in_index : 1; /* index will contain conflict hunks */
 	long xdl_opts;
 	int verbosity;
 	int diff_rename_limit;
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 2f3a16c..4192fd3 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -309,6 +309,26 @@ test_expect_success 'merge-recursive --index-only' '
 	test_cmp expected-diff actual-diff
 '
 
+test_expect_success 'merge-recursive --index-only --conflicts-in-index' '
+	# first pass: do a merge as usual to obtain "expected"
+	rm -fr [abcd] &&
+	git checkout -f "$c2" &&
+	test_expect_code 1 git merge-recursive "$c0" -- "$c2" "$c1" &&
+	git add [abcd] &&
+	git ls-files -s >expected &&
+	# second pass: actual test
+	rm -fr [abcd] &&
+	git checkout -f "$c2" &&
+	test_expect_code 1 \
+		git merge-recursive --index-only --conflicts-in-index \
+		"$c0" -- "$c2" "$c1" &&
+	git ls-files -s >actual &&
+	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.9.0.313.g3d0a325

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case
  2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
                   ` (5 preceding siblings ...)
  2014-02-22  9:17 ` [PATCH v2 6/8] merge-recursive: allow storing conflict hunks in index Thomas Rast
@ 2014-02-22  9:17 ` Thomas Rast
  2014-02-23  9:19   ` Eric Sunshine
  2014-02-27 23:20   ` Junio C Hamano
  2014-02-22  9:17 ` [PATCH v2 8/8] log --remerge-diff: show what the conflict resolution changed Thomas Rast
  7 siblings, 2 replies; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

The directory hash (for fast checks if the index already has a
directory) was only used in ignore_case mode and so depended on that
flag.

Make it generally available on request.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 cache.h     |  2 ++
 name-hash.c | 19 ++++++++++++-------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/cache.h b/cache.h
index dc040fb..e162021 100644
--- a/cache.h
+++ b/cache.h
@@ -276,6 +276,7 @@ struct index_state {
 	struct cache_tree *cache_tree;
 	struct cache_time timestamp;
 	unsigned name_hash_initialized : 1,
+		 has_dir_hash : 1,
 		 initialized : 1;
 	struct hash_table name_hash;
 	struct hash_table dir_hash;
@@ -284,6 +285,7 @@ struct index_state {
 extern struct index_state the_index;
 
 /* Name hashing */
+extern void init_name_hash(struct index_state *istate, int force_dir_hash);
 extern void add_name_hash(struct index_state *istate, struct cache_entry *ce);
 extern void remove_name_hash(struct index_state *istate, struct cache_entry *ce);
 extern void free_name_hash(struct index_state *istate);
diff --git a/name-hash.c b/name-hash.c
index e5b6e1a..c8953be 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -141,16 +141,19 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)
 		*pos = ce;
 	}
 
-	if (ignore_case && !(ce->ce_flags & CE_UNHASHED))
+	if (istate->has_dir_hash && !(ce->ce_flags & CE_UNHASHED))
 		add_dir_entry(istate, ce);
 }
 
-static void lazy_init_name_hash(struct index_state *istate)
+void init_name_hash(struct index_state *istate, int force_dir_hash)
 {
 	int nr;
 
 	if (istate->name_hash_initialized)
 		return;
+
+	istate->has_dir_hash = force_dir_hash || ignore_case;
+
 	if (istate->cache_nr)
 		preallocate_hash(&istate->name_hash, istate->cache_nr);
 	for (nr = 0; nr < istate->cache_nr; nr++)
@@ -161,7 +164,7 @@ static void lazy_init_name_hash(struct index_state *istate)
 void add_name_hash(struct index_state *istate, struct cache_entry *ce)
 {
 	/* if already hashed, add reference to directory entries */
-	if (ignore_case && (ce->ce_flags & CE_STATE_MASK) == CE_STATE_MASK)
+	if (istate->has_dir_hash && (ce->ce_flags & CE_STATE_MASK) == CE_STATE_MASK)
 		add_dir_entry(istate, ce);
 
 	ce->ce_flags &= ~CE_UNHASHED;
@@ -181,7 +184,7 @@ void add_name_hash(struct index_state *istate, struct cache_entry *ce)
 void remove_name_hash(struct index_state *istate, struct cache_entry *ce)
 {
 	/* if already hashed, release reference to directory entries */
-	if (ignore_case && (ce->ce_flags & CE_STATE_MASK) == CE_HASHED)
+	if (istate->has_dir_hash && (ce->ce_flags & CE_STATE_MASK) == CE_HASHED)
 		remove_dir_entry(istate, ce);
 
 	ce->ce_flags |= CE_UNHASHED;
@@ -228,7 +231,7 @@ struct cache_entry *index_dir_exists(struct index_state *istate, const char *nam
 	struct cache_entry *ce;
 	struct dir_entry *dir;
 
-	lazy_init_name_hash(istate);
+	init_name_hash(istate, 0);
 	dir = find_dir_entry(istate, name, namelen);
 	if (dir && dir->nr)
 		return dir->ce;
@@ -250,7 +253,7 @@ struct cache_entry *index_file_exists(struct index_state *istate, const char *na
 	unsigned int hash = hash_name(name, namelen);
 	struct cache_entry *ce;
 
-	lazy_init_name_hash(istate);
+	init_name_hash(istate, 0);
 	ce = lookup_hash(hash, &istate->name_hash);
 
 	while (ce) {
@@ -286,9 +289,11 @@ void free_name_hash(struct index_state *istate)
 	if (!istate->name_hash_initialized)
 		return;
 	istate->name_hash_initialized = 0;
-	if (ignore_case)
+	if (istate->has_dir_hash) {
 		/* free directory entries */
 		for_each_hash(&istate->dir_hash, free_dir_entry, NULL);
+		istate->has_dir_hash = 0;
+	}
 
 	free_hash(&istate->name_hash);
 	free_hash(&istate->dir_hash);
-- 
1.9.0.313.g3d0a325

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 8/8] log --remerge-diff: show what the conflict resolution changed
  2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
                   ` (6 preceding siblings ...)
  2014-02-22  9:17 ` [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case Thomas Rast
@ 2014-02-22  9:17 ` Thomas Rast
  2014-02-27  0:40   ` Junio C Hamano
  7 siblings, 1 reply; 16+ messages in thread
From: Thomas Rast @ 2014-02-22  9:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Git has --cc as a very fast inspection tool that shows a brief summary
of what a conflicted merge "looks like", and -c/-m as "give me the
full information" data dumps.

But --cc actually loses information: if the merge lost(!) some changes
from one side, that hunk would fully agree with the other side, and
therefore be elided.  So --cc cannot be used to investigate mismerges.
Indeed it is rather hard to find a merge that has lost changes, unless
one knows where to look.

The new option --remerge-diff is an attempt at filling this gap,
admittedly at the cost of a lot of CPU cycles.  For each merge commit,
it diffs the merge result against a recursive merge of the merge's
parents.

For files that can be auto-merged cleanly, it will typically show
nothing.  However, it will make it obvious when the merge introduces
extra changes.

For files that result in merge conflicts, we diff against the
representation with conflict hunks (what the user would usually see in
the worktree).  So the diff will show what was changed in the conflict
hunks to resolve the conflict.

It still takes a bit of staring to tell an evil from a regular merge.
But at least the information is there, unlike with --cc; and the
output is usually much shorter than with -c.

Signed-off-by: Thomas Rast <tr@thomasrast.ch>
---
 Documentation/rev-list-options.txt |   7 +
 log-tree.c                         | 298 +++++++++++++++++++++++++++++++++++++
 merge-recursive.c                  |   3 +-
 merge-recursive.h                  |   1 +
 revision.c                         |   2 +
 revision.h                         |   4 +-
 t/t4213-log-remerge-diff.sh        | 222 +++++++++++++++++++++++++++
 7 files changed, 535 insertions(+), 2 deletions(-)
 create mode 100755 t/t4213-log-remerge-diff.sh

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 03533af..73264dc 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -790,6 +790,13 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 	in that case, the output represents the changes the merge
 	brought _into_ the then-current branch.
 
+--remerge-diff::
+	Diff merge commits against a recursive merge of their parents,
+	with conflict hunks.  Intuitively speaking, this shows what
+	the author of the merge changed to resolve the merge.  It
+	assumes that all (or most) merges are recursive merges; other
+	strategies are not supported.
+
 -r::
 	Show recursive diffs.
 
diff --git a/log-tree.c b/log-tree.c
index 30b3063..9b831e9 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -11,6 +11,8 @@
 #include "gpg-interface.h"
 #include "sequencer.h"
 #include "line-log.h"
+#include "cache-tree.h"
+#include "merge-recursive.h"
 
 struct decoration name_decoration = { "object names" };
 
@@ -723,6 +725,300 @@ static int do_diff_combined(struct rev_info *opt, struct commit *commit)
 }
 
 /*
+ * Helpers for make_asymmetric_conflict_entries() below.
+ */
+static char *load_cache_entry_blob(struct cache_entry *entry,
+				   unsigned long *size)
+{
+	enum object_type type;
+	void *data;
+
+	if (!entry)
+		return NULL;
+
+	data = read_sha1_file(entry->sha1, &type, size);
+	if (type != OBJ_BLOB)
+		die("BUG: load_cache_entry_blob for non-blob");
+
+	return data;
+}
+
+static void strbuf_append_cache_entry_blob(struct strbuf *sb,
+					   struct cache_entry *entry)
+{
+	unsigned long size;
+	char *data = load_cache_entry_blob(entry, &size);;
+
+	if (!data)
+		return;
+
+	strbuf_add(sb, data, size);
+	free(data);
+}
+
+static void assemble_conflict_entry(struct strbuf *sb,
+				    const char *branch1,
+				    const char *branch2,
+				    struct cache_entry *entry1,
+				    struct cache_entry *entry2)
+{
+	strbuf_addf(sb, "<<<<<<< %s\n", branch1);
+	strbuf_append_cache_entry_blob(sb, entry1);
+	strbuf_addstr(sb, "=======\n");
+	strbuf_append_cache_entry_blob(sb, entry2);
+	strbuf_addf(sb, ">>>>>>> %s\n", branch2);
+}
+
+/*
+ * For --remerge-diff, we need conflicted (<<<<<<< ... >>>>>>>)
+ * representations of as many conflicts as possible.  Default conflict
+ * generation only applies to files that have all three stages.
+ *
+ * This function generates conflict hunk representations for files
+ * that have only one of stage 2 or 3.  The corresponding side in the
+ * conflict hunk format will be empty.  A stage 1, if any, will be
+ * dropped in the process.
+ */
+static void make_asymmetric_conflict_entries(const char *branch1,
+					     const char *branch2)
+{
+	int o = 0, i = 0;
+
+	/*
+	 * NEEDSWORK: we trample all over the cache below, so we need
+	 * to set up the name hash early, before modifying it.  And
+	 * after that we cannot free any cache entries, because they
+	 * remain hashed even if deleted.  Sigh.
+	 */
+	init_name_hash(&the_index, 1);
+
+	/*
+	 * The loop always starts with 'i' pointing at the first entry
+	 * for a pathname.
+	 */
+	while (i < active_nr) {
+		struct cache_entry *ce;
+		struct cache_entry *stage1 = NULL;
+		struct cache_entry *stage2 = NULL;
+		struct cache_entry *stage3 = NULL;
+		struct cache_entry *new_ce = NULL;
+		struct strbuf content = STRBUF_INIT;
+		unsigned char sha1[20];
+
+		assert(o <= i);
+
+		ce = active_cache[i];
+
+		/*
+		 * Pass through stage 0 and submodules unchanged, we
+		 * don't know how to handle them.
+		 */
+		if (ce_stage(ce) == 0
+		    || S_ISGITLINK(ce->ce_mode)) {
+			active_cache[o++] = ce;
+			i++;
+			continue;
+		}
+
+		/*
+		 * Collect the stages we have.  Point 'i' past the
+		 * entry.
+		 */
+		if (/* i < active_nr && */
+		    !strcmp(ce->name, active_cache[i]->name) &&
+		    ce_stage(active_cache[i]) == 1)
+			stage1 = active_cache[i++];
+
+		if (i < active_nr &&
+		    !strcmp(ce->name, active_cache[i]->name) &&
+		    ce_stage(active_cache[i]) == 2)
+			stage2 = active_cache[i++];
+
+		if (i < active_nr &&
+		    !strcmp(ce->name, active_cache[i]->name) &&
+		    ce_stage(active_cache[i]) == 3)
+			stage3 = active_cache[i++];
+
+		if (!stage2 && !stage3)
+			die("BUG: merging resulted in conflict with neither "
+			    "stage 2 nor 3");
+
+		if (cache_dir_exists(ce->name, ce->ce_namelen)) {
+			/*
+			 * If a conflicting directory for this entry exists,
+			 * we can drop it:
+			 *
+			 * In the face of a file/directory conflict,
+			 * merge-recursive
+			 * - puts the file at stage >0 as usual
+			 * - also attempts to check out the file as
+			 *   'file~side' where side is the sha1 of the commit
+			 *   the file came from, to avoid colliding with the
+			 *   directory
+			 *
+			 * But we have requested that files go to the index
+			 * instead of the worktree, so by the time we get
+			 * here, we have both stage>0 'file' from ordinary
+			 * merging and a stage=0 'file~side' from the "write
+			 * all files to index".
+			 *
+			 * We need to remove one of them.  Currently we ditch
+			 * the 'file' entry because it's easier to detect.
+			 * This amounts to always renaming the file to make
+			 * room for the directory.
+			 *
+			 * NEEDSWORK: Two options:
+			 *
+			 * - If the merge result kept the file, it would be
+			 *   better to rename the _directory_ to make room for
+			 *   the file, so that filenames match between the
+			 *   result and the re-merge.
+			 *
+			 * - Or we could avoid going through a tree, since the
+			 *   index can represent (though it's not "legal") a
+			 *   file/directory collision just fine.
+			 */
+		} else {
+			/*
+			 * Otherwise, there is room for a file entry
+			 * at stage 0.  It has fake-conflict content,
+			 * but its mode is the same.
+			 */
+
+			assemble_conflict_entry(&content,
+						branch1, branch2,
+						stage2, stage3);
+			if (write_sha1_file(content.buf, content.len,
+					    typename(OBJ_BLOB), sha1))
+				die("write_sha1_file failed");
+			strbuf_release(&content);
+
+			new_ce = xmalloc(cache_entry_size(ce->ce_namelen));
+			new_ce->ce_mode = ce->ce_mode;
+			new_ce->ce_flags = ce->ce_flags & ~CE_STAGEMASK;
+			new_ce->ce_namelen = ce->ce_namelen;
+			hashcpy(new_ce->sha1, sha1);
+			new_ce->next = NULL;
+			memcpy(new_ce->name, ce->name, ce->ce_namelen+1);
+			active_cache[o++] = new_ce;
+			add_name_hash(&the_index, new_ce);
+		}
+
+		if (stage1)
+			remove_name_hash(&the_index, stage1);
+		if (stage2)
+			remove_name_hash(&the_index, stage2);
+		if (stage3)
+			remove_name_hash(&the_index, stage3);
+		/*
+		 * Can't free stage[1-3] because they are still in the
+		 * name hash, just marked deleted :-(  So we leak them.
+		 */
+	}
+
+	active_nr = o;
+}
+
+/*
+ * --remerge-diff doesn't currently handle entries that cannot be
+ * turned into a stage0 conflicted-file format blob.  So this routine
+ * clears the corresponding entries from the index.  This is
+ * suboptimal; we should eventually handle them _somehow_.
+*/
+static void drop_non_stage0()
+{
+	int o = 0, i = 0;
+
+	while (i < active_nr) {
+		struct cache_entry *ce = active_cache[i];
+		const char *name;
+
+		if (!ce_stage(ce)) {
+			active_cache[o++] = active_cache[i++];
+			continue;
+		}
+
+		name = ce->name;
+
+		printf("Cannot handle stage %d entry '%s', skipping\n",
+		       ce_stage(ce), name);
+		i++;
+
+		while (i < active_nr && !strcmp(name, active_cache[i]->name))
+			free(active_cache[i++]);
+
+		free(ce);
+	}
+
+	active_nr = o;
+}
+
+static int do_diff_remerge(struct rev_info *opt, struct commit *commit)
+{
+	struct commit_list *merge_bases;
+	struct commit *result, *parent1, *parent2;
+	struct merge_options o;
+	char *branch1, *branch2;
+
+	/*
+	 * We show the log message early to avoid headaches later.  In
+	 * general we need to run this before printing anything in
+	 * this routine.
+	 */
+	if (opt->loginfo && !opt->no_commit_id) {
+		show_log(opt);
+
+		if (opt->verbose_header && opt->diffopt.output_format)
+			printf("%s%c", diff_line_prefix(&opt->diffopt),
+			       opt->diffopt.line_termination);
+	}
+
+	if (commit->parents->next->next) {
+		printf("--remerge-diff not supported for octopus merges.\n");
+		return !opt->loginfo;
+	}
+
+	parent1 = commit->parents->item;
+	parent2 = commit->parents->next->item;
+	parse_commit(parent1);
+	parse_commit(parent2);
+	branch1 = xstrdup(sha1_to_hex(parent1->object.sha1));
+	branch2 = xstrdup(sha1_to_hex(parent2->object.sha1));
+
+	merge_bases = get_octopus_merge_bases(commit->parents);
+	init_merge_options(&o);
+	o.verbosity = -1;
+	o.no_worktree = 1;
+	o.conflicts_in_index = 1;
+	o.use_ondisk_index = 0;
+	o.branch1 = branch1;
+	o.branch2 = branch2;
+	merge_recursive(&o, parent1, parent2, merge_bases, &result);
+
+	make_asymmetric_conflict_entries(branch1, branch2);
+	drop_non_stage0();
+
+	free(branch1);
+	free(branch2);
+
+	active_cache_tree = cache_tree();
+	if (cache_tree_update(active_cache_tree,
+			      (const struct cache_entry * const *)active_cache,
+			      active_nr, WRITE_TREE_SILENT) < 0) {
+		printf("BUG: merge conflicts not fully folded, cannot diff.\n");
+		return !opt->loginfo;
+	}
+
+	diff_tree_sha1(active_cache_tree->sha1, commit->tree->object.sha1,
+		       "", &opt->diffopt);
+	log_tree_diff_flush(opt);
+
+	cache_tree_free(&active_cache_tree);
+
+	return !opt->loginfo;
+}
+
+/*
  * Show the diff of a commit.
  *
  * Return true if we printed any log info messages
@@ -755,6 +1051,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 			return 0;
 		else if (merge_diff_mode_is_any_combined(opt))
 			return do_diff_combined(opt, commit);
+		else if (opt->merge_diff_mode == MERGE_DIFF_REMERGE)
+			return do_diff_remerge(opt, commit);
 		else if (opt->first_parent_only) {
 			/*
 			 * Generate merge log entry only for the first
diff --git a/merge-recursive.c b/merge-recursive.c
index b682812..1507a7a 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1947,7 +1947,7 @@ int merge_recursive(struct merge_options *o,
 	}
 
 	discard_cache();
-	if (!o->call_depth)
+	if (!o->call_depth && o->use_ondisk_index)
 		read_cache();
 
 	o->ancestor = "merged common ancestors";
@@ -2043,6 +2043,7 @@ void init_merge_options(struct merge_options *o)
 	o->diff_rename_limit = -1;
 	o->merge_rename_limit = -1;
 	o->renormalize = 0;
+	o->use_ondisk_index = 1;
 	git_config(merge_recursive_config, o);
 	if (getenv("GIT_MERGE_VERBOSITY"))
 		o->verbosity =
diff --git a/merge-recursive.h b/merge-recursive.h
index 9b8e20b..d7466c7 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -17,6 +17,7 @@ struct merge_options {
 	unsigned renormalize : 1;
 	unsigned no_worktree : 1; /* do not touch worktree */
 	unsigned conflicts_in_index : 1; /* index will contain conflict hunks */
+	unsigned use_ondisk_index : 1; /* tree-level merge loads .git/index */
 	long xdl_opts;
 	int verbosity;
 	int diff_rename_limit;
diff --git a/revision.c b/revision.c
index 127b75a..956a527 100644
--- a/revision.c
+++ b/revision.c
@@ -1811,6 +1811,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->merge_diff_mode = MERGE_DIFF_COMBINED;
 	} else if (!strcmp(arg, "--cc")) {
 		revs->merge_diff_mode = MERGE_DIFF_COMBINED_CONDENSED;
+	} else if (!strcmp(arg, "--remerge-diff")) {
+		revs->merge_diff_mode = MERGE_DIFF_REMERGE;
 	} else if (!strcmp(arg, "-v")) {
 		revs->verbose_header = 1;
 	} else if (!strcmp(arg, "--pretty")) {
diff --git a/revision.h b/revision.h
index 0847902..10a2992 100644
--- a/revision.h
+++ b/revision.h
@@ -58,7 +58,9 @@ enum merge_diff_mode {
 	/* combined format (-c) */
 	MERGE_DIFF_COMBINED,
 	/* combined-condensed format (-cc) */
-	MERGE_DIFF_COMBINED_CONDENSED
+	MERGE_DIFF_COMBINED_CONDENSED,
+	/* --remerge-diff */
+	MERGE_DIFF_REMERGE
 };
 
 struct rev_info {
diff --git a/t/t4213-log-remerge-diff.sh b/t/t4213-log-remerge-diff.sh
new file mode 100755
index 0000000..36ef17a
--- /dev/null
+++ b/t/t4213-log-remerge-diff.sh
@@ -0,0 +1,222 @@
+#!/bin/sh
+
+test_description='test log --remerge-diff'
+. ./test-lib.sh
+
+# A -----------------+----
+# | \  \      \      |    \
+# |  C  \      \     |    |
+# B  |\  \      |    |    |
+# |  | |  D     U   dir  file
+# |\ | |__|__   |    | \ /|
+# | X  |_ |  \  |    |  X |
+# |/ \/  \|   \ |    | / \|
+# M1 M2   M3   M4    M5   M6
+# ^  ^    ^     ^    ^    ^
+# |  |    |     |    |    filedir
+# |  |    |     |    dirfile
+# |  |    dm    unrelated
+# |  evil
+# benign
+#
+#
+# M1 has a "benign" conflict
+# M2 has an "evil" conflict: it ignores the changes in D
+# M3 has a delete/modify conflict, resolved in favor of a modification
+# M4 is a merge of an unrelated change, without conflicts
+# M5 has a file/directory conflict, resolved in favor of the directory
+# M6 has a file/directory conflict, resolved in favor of the file
+
+test_expect_success 'setup' '
+	test_commit A file original &&
+	test_commit B file change &&
+	git checkout -b side A &&
+	test_commit C file side &&
+	git checkout -b delete A &&
+	git rm file &&
+	test_commit D &&
+	git checkout -b benign master &&
+	test_must_fail git merge C &&
+	test_commit M1 file merged &&
+	git checkout -b evil B &&
+	test_must_fail git merge C &&
+	test_commit M2 file change &&
+	git checkout -b dm C &&
+	test_must_fail git merge D &&
+	test_commit M3 file resolved &&
+	git checkout -b unrelated A &&
+	test_commit unrelated_file &&
+	git merge C &&
+	test_tick &&
+	git tag M4 &&
+	git checkout -b dir A &&
+	mkdir sub &&
+	test_commit dir sub/file &&
+	git checkout -b file A &&
+	test_commit file sub &&
+	git checkout -b dirfile tags/dir &&
+	test_must_fail git merge tags/file &&
+	git rm --cached sub &&
+	test_commit M5 sub/file resolved &&
+	git checkout -b filedir tags/file &&
+	test_must_fail git merge tags/dir &&
+	git rm --cached sub/file &&
+	rm -rf sub &&
+	test_commit M6 sub resolved &&
+	git branch -D master side delete dir file
+'
+
+test_expect_success 'unrelated merge: without conflicts' '
+	git log -p --cc unrelated >expected &&
+	git log -p --remerge-diff unrelated >actual &&
+	test_cmp expected actual
+'
+
+clean_output () {
+	git name-rev --name-only --stdin |
+	# strip away bits that aren't treated by the above
+	sed -e 's/^\(index\|Merge:\|Date:\).*/\1/'
+}
+
+cat >expected <<EOF
+commit benign
+Merge:
+Author: A U Thor <author@example.com>
+Date:
+
+    M1
+
+diff --git a/file b/file
+index
+--- a/file
++++ b/file
+@@ -1,5 +1 @@
+-<<<<<<< tags/B
+-change
+-=======
+-side
+->>>>>>> tags/C
++merged
+EOF
+
+test_expect_success 'benign merge: conflicts resolved' '
+	git log -1 -p --remerge-diff benign >output &&
+	clean_output <output >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+commit evil
+Merge:
+Author: A U Thor <author@example.com>
+Date:
+
+    M2
+
+diff --git a/file b/file
+index
+--- a/file
++++ b/file
+@@ -1,5 +1 @@
+-<<<<<<< tags/B
+ change
+-=======
+-side
+->>>>>>> tags/C
+EOF
+
+test_expect_success 'evil merge: changes ignored' '
+	git log -1 --remerge-diff -p evil >output &&
+	clean_output <output >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+commit dm
+Merge:
+Author: A U Thor <author@example.com>
+Date:
+
+    M3
+
+diff --git a/file b/file
+index
+--- a/file
++++ b/file
+@@ -1,4 +1 @@
+-<<<<<<< tags/C
+-side
+-=======
+->>>>>>> tags/D
++resolved
+EOF
+
+test_expect_success 'delete/modify conflict' '
+	git log -1 --remerge-diff -p dm >output &&
+	clean_output <output >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+commit dirfile
+Merge:
+Author: A U Thor <author@example.com>
+Date:
+
+    M5
+
+diff --git a/sub/file b/sub/file
+index
+--- a/sub/file
++++ b/sub/file
+@@ -1 +1 @@
+-dir
++resolved
+diff --git a/sub~tags/file b/sub~tags/file
+deleted file mode 100644
+index
+--- a/sub~tags/file
++++ /dev/null
+@@ -1 +0,0 @@
+-file
+EOF
+
+test_expect_success 'file/directory conflict resulting in directory' '
+	git log -1 --remerge-diff -p dirfile >output &&
+	clean_output <output >actual &&
+	test_cmp expected actual
+'
+
+# This is wishful thinking, see the NEEDSWORK in
+# make_asymmetric_conflict_entries().
+cat >expected <<EOF
+commit filedir
+Merge:
+Author: A U Thor <author@example.com>
+Date:
+
+    M6
+
+diff --git a/sub b/sub
+index
+--- a/sub
++++ b/sub
+@@ -1 +1 @@
+-file
++resolved
+diff --git a/sub/file b/sub/file
+deleted file mode 100644
+index
+--- a/sub/file
++++ /dev/null
+@@ -1 +0,0 @@
+-dir
+EOF
+
+test_expect_failure 'file/directory conflict resulting in file' '
+	git log -1 --remerge-diff -p filedir >output &&
+	clean_output <output >actual &&
+	test_cmp expected actual
+'
+
+test_done
-- 
1.9.0.313.g3d0a325

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/8] merge-recursive: -Xindex-only to leave worktree unchanged
  2014-02-22  9:17 ` [PATCH v2 3/8] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast
@ 2014-02-23  9:07   ` Eric Sunshine
  2014-02-23 11:57     ` Thomas Rast
  0 siblings, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2014-02-23  9:07 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Git List, Junio C Hamano, Thomas Rast

On Sat, Feb 22, 2014 at 4:17 AM, Thomas Rast <tr@thomasrast.ch> wrote:
> 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.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
> index fb6e593..2934e99 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;;

s/;;/::/

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case
  2014-02-22  9:17 ` [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case Thomas Rast
@ 2014-02-23  9:19   ` Eric Sunshine
  2014-09-06 17:46     ` Thomas Rast
  2014-02-27 23:20   ` Junio C Hamano
  1 sibling, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2014-02-23  9:19 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Git List, Junio C Hamano

On Sat, Feb 22, 2014 at 4:17 AM, Thomas Rast <tr@thomasrast.ch> wrote:
> The directory hash (for fast checks if the index already has a
> directory) was only used in ignore_case mode and so depended on that
> flag.
>
> Make it generally available on request.
>
> Signed-off-by: Thomas Rast <tr@thomasrast.ch>
> ---
> diff --git a/name-hash.c b/name-hash.c
> index e5b6e1a..c8953be 100644
> --- a/name-hash.c
> +++ b/name-hash.c
> @@ -141,16 +141,19 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)
>                 *pos = ce;
>         }
>
> -       if (ignore_case && !(ce->ce_flags & CE_UNHASHED))
> +       if (istate->has_dir_hash && !(ce->ce_flags & CE_UNHASHED))
>                 add_dir_entry(istate, ce);
>  }
>
> -static void lazy_init_name_hash(struct index_state *istate)
> +void init_name_hash(struct index_state *istate, int force_dir_hash)
>  {
>         int nr;
>
>         if (istate->name_hash_initialized)
>                 return;
> +
> +       istate->has_dir_hash = force_dir_hash || ignore_case;

This is getting a bit convoluted. Refactoring lazy_init_name_hash()
into two functions would eliminate the complexity. For instance:

  void init_name_hash(struct index_state *istate)
  {
  ...pure initialization code...
  }

  static void init_name_hash_if_needed(struct index_state *istate)
  {
    if (ignore_case && !istate->name_hash_initialized)
      init_name_hash(istate);
  }

The two existing callers of lazy_init_name_hash() in name-hash.c,
which rely upon the lazy/ignore-case logic, would invoke the static
init_name_hash_if_needed().

The new caller in patch 8/8, which knows explicitly if and when it
wants the hash initialized can invoke the public init_name_hash().

>         if (istate->cache_nr)
>                 preallocate_hash(&istate->name_hash, istate->cache_nr);
>         for (nr = 0; nr < istate->cache_nr; nr++)
> @@ -161,7 +164,7 @@ static void lazy_init_name_hash(struct index_state *istate)
>  void add_name_hash(struct index_state *istate, struct cache_entry *ce)
>  {
>         /* if already hashed, add reference to directory entries */
> -       if (ignore_case && (ce->ce_flags & CE_STATE_MASK) == CE_STATE_MASK)
> +       if (istate->has_dir_hash && (ce->ce_flags & CE_STATE_MASK) == CE_STATE_MASK)
>                 add_dir_entry(istate, ce);
>
>         ce->ce_flags &= ~CE_UNHASHED;
> @@ -181,7 +184,7 @@ void add_name_hash(struct index_state *istate, struct cache_entry *ce)
>  void remove_name_hash(struct index_state *istate, struct cache_entry *ce)
>  {
>         /* if already hashed, release reference to directory entries */
> -       if (ignore_case && (ce->ce_flags & CE_STATE_MASK) == CE_HASHED)
> +       if (istate->has_dir_hash && (ce->ce_flags & CE_STATE_MASK) == CE_HASHED)
>                 remove_dir_entry(istate, ce);
>
>         ce->ce_flags |= CE_UNHASHED;
> @@ -228,7 +231,7 @@ struct cache_entry *index_dir_exists(struct index_state *istate, const char *nam
>         struct cache_entry *ce;
>         struct dir_entry *dir;
>
> -       lazy_init_name_hash(istate);
> +       init_name_hash(istate, 0);
>         dir = find_dir_entry(istate, name, namelen);
>         if (dir && dir->nr)
>                 return dir->ce;
> @@ -250,7 +253,7 @@ struct cache_entry *index_file_exists(struct index_state *istate, const char *na
>         unsigned int hash = hash_name(name, namelen);
>         struct cache_entry *ce;
>
> -       lazy_init_name_hash(istate);
> +       init_name_hash(istate, 0);
>         ce = lookup_hash(hash, &istate->name_hash);
>
>         while (ce) {
> @@ -286,9 +289,11 @@ void free_name_hash(struct index_state *istate)
>         if (!istate->name_hash_initialized)
>                 return;
>         istate->name_hash_initialized = 0;
> -       if (ignore_case)
> +       if (istate->has_dir_hash) {
>                 /* free directory entries */
>                 for_each_hash(&istate->dir_hash, free_dir_entry, NULL);
> +               istate->has_dir_hash = 0;
> +       }
>
>         free_hash(&istate->name_hash);
>         free_hash(&istate->dir_hash);
> --
> 1.9.0.313.g3d0a325

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/8] merge-recursive: -Xindex-only to leave worktree unchanged
  2014-02-23  9:07   ` Eric Sunshine
@ 2014-02-23 11:57     ` Thomas Rast
  2014-02-23 18:42       ` Eric Sunshine
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Rast @ 2014-02-23 11:57 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Junio C Hamano, Thomas Rast

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sat, Feb 22, 2014 at 4:17 AM, Thomas Rast <tr@thomasrast.ch> wrote:
>> 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.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
>> index fb6e593..2934e99 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;;
>
> s/;;/::/

I think ;; is actually correct: this continues the sub-list of options
to the recursive strategy.  The :: level lists the available strategies.

>> +       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
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Thomas Rast
tr@thomasrast.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/8] merge-recursive: -Xindex-only to leave worktree unchanged
  2014-02-23 11:57     ` Thomas Rast
@ 2014-02-23 18:42       ` Eric Sunshine
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Sunshine @ 2014-02-23 18:42 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Git List, Junio C Hamano, Thomas Rast

On Sun, Feb 23, 2014 at 6:57 AM, Thomas Rast <tr@thomasrast.ch> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> On Sat, Feb 22, 2014 at 4:17 AM, Thomas Rast <tr@thomasrast.ch> wrote:
>>> 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.
>>>
>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>>> ---
>>> diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
>>> index fb6e593..2934e99 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;;
>>
>> s/;;/::/
>
> I think ;; is actually correct: this continues the sub-list of options
> to the recursive strategy.  The :: level lists the available strategies.

Make sense. Thanks for the explanation.

>>> +       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
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Thomas Rast
> tr@thomasrast.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 8/8] log --remerge-diff: show what the conflict resolution changed
  2014-02-22  9:17 ` [PATCH v2 8/8] log --remerge-diff: show what the conflict resolution changed Thomas Rast
@ 2014-02-27  0:40   ` Junio C Hamano
  0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2014-02-27  0:40 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git

Thomas Rast <tr@thomasrast.ch> writes:

> diff --git a/log-tree.c b/log-tree.c
> index 30b3063..9b831e9 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -11,6 +11,8 @@
>  #include "gpg-interface.h"
>  #include "sequencer.h"
>  #include "line-log.h"
> +#include "cache-tree.h"
> +#include "merge-recursive.h"
>  
>  struct decoration name_decoration = { "object names" };
>  
> @@ -723,6 +725,300 @@ static int do_diff_combined(struct rev_info *opt, struct commit *commit)
>  }
>  
>  /*
> + * Helpers for make_asymmetric_conflict_entries() below.
> + */
> +static char *load_cache_entry_blob(struct cache_entry *entry,
> +				   unsigned long *size)
> +{

I briefly wondered if this helper need to know about contents
conversions (e.g. smudge/clean or crlf), but not doing any of that
*is* the right thing to do.  IOW, I agree with what this part of the
patch does.

But it feels, at least to me, that this helper function ...

> +static void strbuf_append_cache_entry_blob(struct strbuf *sb,
> +					   struct cache_entry *entry)
> +{

... and this one are overly specific.  Why should they know about
(and limit themselves to operate on) cache-entry type?  Isn't it too
much to ask for the caller to say "entry->sha1" instead of "entry"?

I do not have an issue with a "load_blob()" helper that makes sure
what it reads is a blob, though, of course.

> +static void assemble_conflict_entry(struct strbuf *sb,
> +				    const char *branch1,
> +				    const char *branch2,
> +				    struct cache_entry *entry1,
> +				    struct cache_entry *entry2)
> +{
> +	strbuf_addf(sb, "<<<<<<< %s\n", branch1);
> +	strbuf_append_cache_entry_blob(sb, entry1);
> +	strbuf_addstr(sb, "=======\n");
> +	strbuf_append_cache_entry_blob(sb, entry2);
> +	strbuf_addf(sb, ">>>>>>> %s\n", branch2);
> +}
> +
> +/*
> + * For --remerge-diff, we need conflicted (<<<<<<< ... >>>>>>>)
> + * representations of as many conflicts as possible.  Default conflict
> + * generation only applies to files that have all three stages.
> + *
> + * This function generates conflict hunk representations for files
> + * that have only one of stage 2 or 3.  The corresponding side in the
> + * conflict hunk format will be empty.  A stage 1, if any, will be
> + * dropped in the process.
> + */
> +static void make_asymmetric_conflict_entries(const char *branch1,
> +					     const char *branch2)
> +{

I should comment on this one in a separate message after I read it
over once again.

> +}
> +
> +/*
> + * --remerge-diff doesn't currently handle entries that cannot be
> + * turned into a stage0 conflicted-file format blob.  So this routine
> + * clears the corresponding entries from the index.  This is
> + * suboptimal; we should eventually handle them _somehow_.
> +*/
> +static void drop_non_stage0()

"static void drop_non_stage0(void)"

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case
  2014-02-22  9:17 ` [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case Thomas Rast
  2014-02-23  9:19   ` Eric Sunshine
@ 2014-02-27 23:20   ` Junio C Hamano
  1 sibling, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2014-02-27 23:20 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git

Thomas Rast <tr@thomasrast.ch> writes:

> The directory hash (for fast checks if the index already has a
> directory) was only used in ignore_case mode and so depended on that
> flag.
>
> Make it generally available on request.
>
> Signed-off-by: Thomas Rast <tr@thomasrast.ch>
> ---

I somehow had an impression that we were getting rid of these two
hashes and merging them into one, but I do not see any such change
between 'master..pu'.  Perhaps I am confused with some other topic.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case
  2014-02-23  9:19   ` Eric Sunshine
@ 2014-09-06 17:46     ` Thomas Rast
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Rast @ 2014-09-06 17:46 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Git List, Junio C Hamano

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Sat, Feb 22, 2014 at 4:17 AM, Thomas Rast <tr@thomasrast.ch> wrote:
>> -static void lazy_init_name_hash(struct index_state *istate)
>> +void init_name_hash(struct index_state *istate, int force_dir_hash)
>>  {
>>         int nr;
>>
>>         if (istate->name_hash_initialized)
>>                 return;
>> +
>> +       istate->has_dir_hash = force_dir_hash || ignore_case;
>
> This is getting a bit convoluted. Refactoring lazy_init_name_hash()
> into two functions would eliminate the complexity. For instance:
>
>   void init_name_hash(struct index_state *istate)
>   {
>   ...pure initialization code...
>   }
>
>   static void init_name_hash_if_needed(struct index_state *istate)
>   {
>     if (ignore_case && !istate->name_hash_initialized)
>       init_name_hash(istate);
>   }
>
> The two existing callers of lazy_init_name_hash() in name-hash.c,
> which rely upon the lazy/ignore-case logic, would invoke the static
> init_name_hash_if_needed().
>
> The new caller in patch 8/8, which knows explicitly if and when it
> wants the hash initialized can invoke the public init_name_hash().

That unfortunately doesn't really help because the conditional part only
affects the dir hash.  Callers request the name hash for other reasons,
but we only do the dir hashing when ignore_case is in effect.

So it's not simply about overriding lazy initialization, but about
choosing to trigger a specific subpart of the initialization.

-- 
Thomas Rast
tr@thomasrast.ch

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-09-06 17:46 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-22  9:17 [PATCH v2 0/8] log --remerge-diff Thomas Rast
2014-02-22  9:17 ` [PATCH v2 1/8] merge-recursive: remove dead conditional in update_stages() Thomas Rast
2014-02-22  9:17 ` [PATCH v2 2/8] merge-recursive: internal flag to avoid touching the worktree Thomas Rast
2014-02-22  9:17 ` [PATCH v2 3/8] merge-recursive: -Xindex-only to leave worktree unchanged Thomas Rast
2014-02-23  9:07   ` Eric Sunshine
2014-02-23 11:57     ` Thomas Rast
2014-02-23 18:42       ` Eric Sunshine
2014-02-22  9:17 ` [PATCH v2 4/8] combine-diff: do not pass revs->dense_combined_merges redundantly Thomas Rast
2014-02-22  9:17 ` [PATCH v2 5/8] Fold all merge diff variants into an enum Thomas Rast
2014-02-22  9:17 ` [PATCH v2 6/8] merge-recursive: allow storing conflict hunks in index Thomas Rast
2014-02-22  9:17 ` [PATCH v2 7/8] name-hash: allow dir hashing even when !ignore_case Thomas Rast
2014-02-23  9:19   ` Eric Sunshine
2014-09-06 17:46     ` Thomas Rast
2014-02-27 23:20   ` Junio C Hamano
2014-02-22  9:17 ` [PATCH v2 8/8] log --remerge-diff: show what the conflict resolution changed Thomas Rast
2014-02-27  0:40   ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.