[PATCH v4 00/24] Clean up merge API

[Elijah Newren <newren@gmail.com>]

Before writing a replacement merge strategy for recursive, I decided to
first cleanup the merge API -- streamlining merge-recursive.h and making
it more readable.  This includes some minor fixes found along the way.

Changes since v3 (full range-diff below):
  * Addressed feedback from Junio and SZEDER:
    * [Patch 4] Avoided looping over commit_list in determining
                ancestor_name
    * [Patch 12] s/inmemory/in_core/ in new cache-tree function name
    * [Patch 18] restored the '>= 0' comparision
    * [Patch 21] Use FREE_AND_NULL
    * [Patch 23] Use range checks for show_rename_progress (don't use
                 a bitfield), and don't compare an unsigned using >= 0.

Notes:
  * Patches 7, 9, 13, 14, and 15 only changed in the range-diff due to
    context changes from other patches

Derrick Stolee (1):
  merge-recursive: introduce an enum for detect_directory_renames values

Elijah Newren (23):
  merge-recursive: be consistent with assert
  checkout: provide better conflict hunk description with detached HEAD
  merge-recursive: enforce opt->ancestor != NULL when calling
    merge_trees()
  merge-recursive: provide a better label for diff3 common ancestor
  merge-recursive: future-proof update_file_flags() against memory leaks
  merge-recursive: remove another implicit dependency on the_repository
  Ensure index matches head before invoking merge machinery, round N
  merge-recursive: exit early if index != head
  merge-recursive: remove useless parameter in merge_trees()
  merge-recursive: don't force external callers to do our logging
  cache-tree: share code between functions writing an index as a tree
  merge-recursive: fix some overly long lines
  merge-recursive: use common name for ancestors/common/base_list
  merge-recursive: rename 'mrtree' to 'result_tree', for clarity
  merge-recursive: rename merge_options argument to opt in header
  merge-recursive: move some definitions around to clean up the header
  merge-recursive: consolidate unnecessary fields in merge_options
  merge-recursive: comment and reorder the merge_options fields
  merge-recursive: avoid losing output and leaking memory holding that
    output
  merge-recursive: split internal fields into a separate struct
  merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_*
  merge-recursive: add sanity checks for relevant merge_options
  merge-recursive: alphabetize include list

 builtin/am.c                      |   2 +-
 builtin/checkout.c                |  14 +-
 builtin/merge-recursive.c         |   4 +
 builtin/stash.c                   |   2 +
 cache-tree.c                      |  85 +++--
 cache-tree.h                      |   3 +-
 merge-recursive.c                 | 566 ++++++++++++++++++------------
 merge-recursive.h                 | 164 +++++----
 sequencer.c                       |   5 +-
 t/t3030-merge-recursive.sh        |   9 +-
 t/t6036-recursive-corner-cases.sh |   8 +-
 t/t6047-diff3-conflict-markers.sh | 189 ++++++++++
 12 files changed, 717 insertions(+), 334 deletions(-)
 create mode 100755 t/t6047-diff3-conflict-markers.sh

Range-diff:
 1:  21f1e04dc9 =  1:  7f99c2401e merge-recursive: be consistent with assert
 2:  ac24702773 =  2:  b305348f45 checkout: provide better conflict hunk description with detached HEAD
 3:  fd14ed9490 =  3:  5b9e32a356 merge-recursive: enforce opt->ancestor != NULL when calling merge_trees()
 4:  540a1d17d7 !  4:  566a55cf76 merge-recursive: provide a better label for diff3 common ancestor
    @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
      	struct commit *merged_common_ancestors;
      	struct tree *mrtree;
      	int clean;
    -+	int num_merge_bases;
    ++	const char *ancestor_name;
     +	struct strbuf merge_base_abbrev = STRBUF_INIT;
     +
     +	if (!opt->call_depth)
    @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
      	if (show(opt, 4)) {
      		output(opt, 4, _("Merging:"));
     @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
    - 			output_commit_title(opt, iter->item);
    + 
    + 		tree = lookup_tree(opt->repo, opt->repo->hash_algo->empty_tree);
    + 		merged_common_ancestors = make_virtual_commit(opt->repo, tree, "ancestor");
    ++		ancestor_name = "empty tree";
    ++	} else if (ca) {
    ++		ancestor_name = "merged common ancestors";
    ++	} else {
    ++		strbuf_add_unique_abbrev(&merge_base_abbrev,
    ++					 &merged_common_ancestors->object.oid,
    ++					 DEFAULT_ABBREV);
    ++		ancestor_name = merge_base_abbrev.buf;
      	}
      
    -+	num_merge_bases = commit_list_count(ca);
    - 	merged_common_ancestors = pop_commit(&ca);
    - 	if (merged_common_ancestors == NULL) {
    - 		/* if there is no common ancestor, use an empty tree */
    + 	for (iter = ca; iter; iter = iter->next) {
     @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
      	if (!opt->call_depth)
      		repo_read_index(opt->repo);
      
     -	opt->ancestor = "merged common ancestors";
    -+	switch (num_merge_bases) {
    -+	case 0:
    -+		opt->ancestor = "<empty tree>";
    -+		break;
    -+	case 1:
    -+		strbuf_add_unique_abbrev(&merge_base_abbrev,
    -+					 &merged_common_ancestors->object.oid,
    -+					 DEFAULT_ABBREV);
    -+		opt->ancestor = merge_base_abbrev.buf;
    -+		break;
    -+	default:
    -+		opt->ancestor = "merged common ancestors";
    -+	}
    ++	opt->ancestor = ancestor_name;
      	clean = merge_trees(opt, get_commit_tree(h1), get_commit_tree(h2),
      			    get_commit_tree(merged_common_ancestors),
      			    &mrtree);
    @@ t/t6047-diff3-conflict-markers.sh (new)
     +
     +		test_must_fail git -c merge.conflictstyle=diff3 merge --allow-unrelated-histories -s recursive R^0 &&
     +
    -+		grep "|||||| <empty tree>" content
    ++		grep "|||||| empty tree" content
     +	)
     +'
     +
 5:  19ff6a9503 =  5:  9c0bab2a0b merge-recursive: introduce an enum for detect_directory_renames values
 6:  5e44146da1 =  6:  2cd1a60a16 merge-recursive: future-proof update_file_flags() against memory leaks
 7:  df210eb029 !  7:  27a339781f merge-recursive: remove another implicit dependency on the_repository
    @@ Commit message
     
      ## merge-recursive.c ##
     @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
    - 	default:
    - 		opt->ancestor = "merged common ancestors";
    - 	}
    + 		repo_read_index(opt->repo);
    + 
    + 	opt->ancestor = ancestor_name;
     -	clean = merge_trees(opt, get_commit_tree(h1), get_commit_tree(h2),
     -			    get_commit_tree(merged_common_ancestors),
     +	clean = merge_trees(opt,
 8:  74dd7b8f59 =  8:  133c470371 Ensure index matches head before invoking merge machinery, round N
 9:  f04eba4184 !  9:  f43a229ae6 merge-recursive: exit early if index != head
    @@ merge-recursive.c: static struct commit_list *reverse_commit_list(struct commit_
      	struct commit_list *iter;
      	struct commit *merged_common_ancestors;
     @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
    - 	int num_merge_bases;
    + 	const char *ancestor_name;
      	struct strbuf merge_base_abbrev = STRBUF_INIT;
      
     -	if (!opt->call_depth)
    @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
      			return -1;
      		opt->branch1 = saved_b1;
     @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
    - 	default:
    - 		opt->ancestor = "merged common ancestors";
    - 	}
    + 		repo_read_index(opt->repo);
    + 
    + 	opt->ancestor = ancestor_name;
     -	clean = merge_trees(opt,
     -			    repo_get_commit_tree(opt->repo, h1),
     -			    repo_get_commit_tree(opt->repo, h2),
10:  8688d40739 = 10:  678dfbf507 merge-recursive: remove useless parameter in merge_trees()
11:  a92d460707 = 11:  7b28da5637 merge-recursive: don't force external callers to do our logging
12:  dec0ea7409 ! 12:  c1ae5cc1c4 cache-tree: share code between functions writing an index as a tree
    @@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *op
      			init_merge_options(&o, the_repository);
      			o.verbosity = 0;
     -			work = write_tree_from_memory(&o);
    -+			work = write_inmemory_index_as_tree(the_repository);
    ++			work = write_in_core_index_as_tree(the_repository);
      
      			ret = reset_tree(new_tree,
      					 opts, 1,
    @@ cache-tree.c: static struct cache_tree *cache_tree_find(struct cache_tree *it, c
      	return it;
      }
      
    -+int write_index_as_tree_internal(struct object_id *oid, struct index_state *index_state, int cache_tree_valid, int flags, const char *prefix)
    ++static int write_index_as_tree_internal(struct object_id *oid,
    ++					struct index_state *index_state,
    ++					int cache_tree_valid,
    ++					int flags,
    ++					const char *prefix)
     +{
     +	if (flags & WRITE_TREE_IGNORE_CACHE_TREE) {
     +		cache_tree_free(&index_state->cache_tree);
    @@ cache-tree.c: static struct cache_tree *cache_tree_find(struct cache_tree *it, c
     +	return 0;
     +}
     +
    -+struct tree* write_inmemory_index_as_tree(struct repository *repo) {
    ++struct tree* write_in_core_index_as_tree(struct repository *repo) {
     +	struct object_id o;
     +	int was_valid, ret;
     +
    @@ cache-tree.h: void cache_tree_verify(struct repository *, struct index_state *);
      #define WRITE_TREE_UNMERGED_INDEX (-2)
      #define WRITE_TREE_PREFIX_ERROR (-3)
      
    -+struct tree* write_inmemory_index_as_tree(struct repository *repo);
    ++struct tree* write_in_core_index_as_tree(struct repository *repo);
      int write_index_as_tree(struct object_id *oid, struct index_state *index_state, const char *index_path, int flags, const char *prefix);
      void prime_cache_tree(struct repository *, struct index_state *, struct tree *);
      
    @@ merge-recursive.c: static int merge_trees_internal(struct merge_options *opt,
      
     -	if (opt->call_depth && !(*result = write_tree_from_memory(opt)))
     +	if (opt->call_depth &&
    -+	    !(*result = write_inmemory_index_as_tree(opt->repo)))
    ++	    !(*result = write_in_core_index_as_tree(opt->repo)))
      		return -1;
      
      	return clean;
13:  b51f3d1924 ! 13:  2e9fb223d9 merge-recursive: fix some overly long lines
    @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt
     -		merged_common_ancestors = make_virtual_commit(opt->repo, tree, "ancestor");
     +		merged_common_ancestors = make_virtual_commit(opt->repo,
     +							      tree, "ancestor");
    - 	}
    - 
    - 	for (iter = ca; iter; iter = iter->next) {
    + 		ancestor_name = "empty tree";
    + 	} else if (ca) {
    + 		ancestor_name = "merged common ancestors";
     @@ merge-recursive.c: int merge_recursive(struct merge_options *opt,
      	return clean;
      }
14:  a069cc4cca ! 14:  230d903012 merge-recursive: use common name for ancestors/common/base_list
    @@ merge-recursive.c: static struct commit_list *reverse_commit_list(struct commit_
     +	struct commit *merged_merge_bases;
      	struct tree *mrtree;
      	int clean;
    - 	int num_merge_bases;
    + 	const char *ancestor_name;
     @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt,
      		output_commit_title(opt, h2);
      	}
    @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt
      			output_commit_title(opt, iter->item);
      	}
      
    --	num_merge_bases = commit_list_count(ca);
     -	merged_common_ancestors = pop_commit(&ca);
     -	if (merged_common_ancestors == NULL) {
    -+	num_merge_bases = commit_list_count(merge_bases);
     +	merged_merge_bases = pop_commit(&merge_bases);
     +	if (merged_merge_bases == NULL) {
      		/* if there is no common ancestor, use an empty tree */
    @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt
     -							      tree, "ancestor");
     +		merged_merge_bases = make_virtual_commit(opt->repo, tree,
     +							 "ancestor");
    + 		ancestor_name = "empty tree";
    +-	} else if (ca) {
    ++	} else if (merge_bases) {
    + 		ancestor_name = "merged common ancestors";
    + 	} else {
    + 		strbuf_add_unique_abbrev(&merge_base_abbrev,
    +-					 &merged_common_ancestors->object.oid,
    ++					 &merged_merge_bases->object.oid,
    + 					 DEFAULT_ABBREV);
    + 		ancestor_name = merge_base_abbrev.buf;
      	}
      
     -	for (iter = ca; iter; iter = iter->next) {
    @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt
      			return err(opt, _("merge returned no commit"));
      	}
      
    -@@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt,
    - 		break;
    - 	case 1:
    - 		strbuf_add_unique_abbrev(&merge_base_abbrev,
    --					 &merged_common_ancestors->object.oid,
    -+					 &merged_merge_bases->object.oid,
    - 					 DEFAULT_ABBREV);
    - 		opt->ancestor = merge_base_abbrev.buf;
    - 		break;
     @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt,
      				     repo_get_commit_tree(opt->repo, h1),
      				     repo_get_commit_tree(opt->repo, h2),
15:  93a3ce6b88 ! 15:  28c20a2d08 merge-recursive: rename 'mrtree' to 'result_tree', for clarity
    @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt
     -	struct tree *mrtree;
     +	struct tree *result_tree;
      	int clean;
    - 	int num_merge_bases;
    + 	const char *ancestor_name;
      	struct strbuf merge_base_abbrev = STRBUF_INIT;
     @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt,
      				     repo_get_commit_tree(opt->repo, h2),
16:  1b1df10c11 = 16:  50e2e51cf6 merge-recursive: rename merge_options argument to opt in header
17:  1526977a85 = 17:  b8cd111bd2 merge-recursive: move some definitions around to clean up the header
18:  c90f2f15cd ! 18:  9bb8a7f162 merge-recursive: consolidate unnecessary fields in merge_options
    @@ merge-recursive.c: static int add_cacheinfo(struct merge_options *opt,
      {
     -	return opt->merge_detect_rename >= 0 ? opt->merge_detect_rename :
     -		opt->diff_detect_rename >= 0 ? opt->diff_detect_rename : 1;
    -+	return (opt->detect_renames != -1) ? opt->detect_renames : 1;
    ++	return (opt->detect_renames >= 0) ? opt->detect_renames : 1;
      }
      
      static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)
    @@ merge-recursive.c: static struct diff_queue_struct *get_diffpairs(struct merge_o
     -	opts.rename_limit = opt->merge_rename_limit >= 0 ? opt->merge_rename_limit :
     -			    opt->diff_rename_limit >= 0 ? opt->diff_rename_limit :
     -			    1000;
    -+	opts.rename_limit = (opt->rename_limit != -1) ? opt->rename_limit : 1000;
    ++	opts.rename_limit = (opt->rename_limit >= 0) ? opt->rename_limit : 1000;
      	opts.rename_score = opt->rename_score;
      	opts.show_rename_progress = opt->show_rename_progress;
      	opts.output_format = DIFF_FORMAT_NO_OUTPUT;
19:  6d930dba72 = 19:  2140490796 merge-recursive: comment and reorder the merge_options fields
20:  ec3e15f6a8 = 20:  02478e2088 merge-recursive: avoid losing output and leaking memory holding that output
21:  7edfac7048 ! 21:  2781ca78d7 merge-recursive: split internal fields into a separate struct
    @@ merge-recursive.c: static int merge_trees_internal(struct merge_options *opt,
      
     -	if (opt->call_depth &&
     +	if (opt->priv->call_depth &&
    - 	    !(*result = write_inmemory_index_as_tree(opt->repo)))
    + 	    !(*result = write_in_core_index_as_tree(opt->repo)))
      		return -1;
      
     @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt,
    @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt
     +	if (!opt->priv->call_depth)
      		repo_read_index(opt->repo);
      
    - 	switch (num_merge_bases) {
    + 	opt->ancestor = ancestor_name;
     @@ merge-recursive.c: static int merge_recursive_internal(struct merge_options *opt,
      		return clean;
      	}
    @@ merge-recursive.c: static int merge_start(struct merge_options *opt, struct tree
      		diff_warn_rename_limit("merge.renamelimit",
     -				       opt->needed_rename_limit, 0);
     +				       opt->priv->needed_rename_limit, 0);
    -+	free(opt->priv);
    -+	opt->priv = NULL;
    ++	FREE_AND_NULL(opt->priv);
      }
      
      int merge_trees(struct merge_options *opt,
22:  9a381873c2 = 22:  5a52aa786e merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_*
23:  c6bc8a196f ! 23:  b7e6c2436a merge-recursive: add sanity checks for relevant merge_options
    @@ merge-recursive.c: static int merge_start(struct merge_options *opt, struct tree
     +	       opt->detect_directory_renames <= MERGE_DIRECTORY_RENAMES_TRUE);
     +	assert(opt->rename_limit >= -1);
     +	assert(opt->rename_score >= 0 && opt->rename_score <= MAX_SCORE);
    ++	assert(opt->show_rename_progress >= 0 && opt->show_rename_progress <= 1);
     +
     +	assert(opt->xdl_opts >= 0);
     +	assert(opt->recursive_variant >= MERGE_VARIANT_NORMAL &&
     +	       opt->recursive_variant <= MERGE_VARIANT_THEIRS);
     +
     +	assert(opt->verbosity >= 0 && opt->verbosity <= 5);
    -+	assert(opt->buffer_output >= 0 && opt->buffer_output <= 2);
    ++	assert(opt->buffer_output <= 2);
     +	assert(opt->obuf.len == 0);
     +
     +	assert(opt->priv == NULL);
    @@ merge-recursive.c: static int merge_start(struct merge_options *opt, struct tree
      	if (repo_index_has_changes(opt->repo, head, &sb)) {
      		err(opt, _("Your local changes to the following files would be overwritten by merge:\n  %s"),
      		    sb.buf);
    -
    - ## merge-recursive.h ##
    -@@ merge-recursive.h: struct merge_options {
    - 	} detect_directory_renames;
    - 	int rename_limit;
    - 	int rename_score;
    --	int show_rename_progress;
    -+	int show_rename_progress : 1;
    - 
    - 	/* xdiff-related options (patience, ignore whitespace, ours/theirs) */
    - 	long xdl_opts;
24:  2123e9e4e4 = 24:  6ac9e42a3e merge-recursive: alphabetize include list
-- 
2.23.0.rc2.28.g5f89f15d7b.dirty