Merge branch 'en/merge-recursive-cleanup'

The merge-recursive machiery is one of the most complex parts of
the system that accumulated cruft over time.  This large series
cleans up the implementation quite a bit.

* en/merge-recursive-cleanup: (26 commits)
  merge-recursive: fix the fix to the diff3 common ancestor label
  merge-recursive: fix the diff3 common ancestor label for virtual commits
  merge-recursive: alphabetize include list
  merge-recursive: add sanity checks for relevant merge_options
  merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_*
  merge-recursive: split internal fields into a separate struct
  merge-recursive: avoid losing output and leaking memory holding that output
  merge-recursive: comment and reorder the merge_options fields
  merge-recursive: consolidate unnecessary fields in merge_options
  merge-recursive: move some definitions around to clean up the header
  merge-recursive: rename merge_options argument to opt in header
  merge-recursive: rename 'mrtree' to 'result_tree', for clarity
  merge-recursive: use common name for ancestors/common/base_list
  merge-recursive: fix some overly long lines
  cache-tree: share code between functions writing an index as a tree
  merge-recursive: don't force external callers to do our logging
  merge-recursive: remove useless parameter in merge_trees()
  merge-recursive: exit early if index != head
  Ensure index matches head before invoking merge machinery, round N
  merge-recursive: remove another implicit dependency on the_repository
  ...
This commit is contained in:
Junio C Hamano 2019-10-15 13:47:59 +09:00
commit 280bd44551
12 changed files with 744 additions and 342 deletions

View File

@ -1526,7 +1526,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa
o.branch1 = "HEAD";
their_tree_name = xstrfmt("%.*s", linelen(state->msg), state->msg);
o.branch2 = their_tree_name;
o.detect_directory_renames = 0;
o.detect_directory_renames = MERGE_DIRECTORY_RENAMES_NONE;
if (state->quiet)
o.verbosity = 0;

View File

@ -709,11 +709,11 @@ static int merge_working_tree(const struct checkout_opts *opts,
* give up or do a real merge, depending on
* whether the merge flag was used.
*/
struct tree *result;
struct tree *work;
struct tree *old_tree;
struct merge_options o;
struct strbuf sb = STRBUF_INIT;
struct strbuf old_commit_shortname = STRBUF_INIT;
if (!opts->merge)
return 1;
@ -754,7 +754,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
*/
init_merge_options(&o, the_repository);
o.verbosity = 0;
work = write_tree_from_memory(&o);
work = write_in_core_index_as_tree(the_repository);
ret = reset_tree(new_tree,
opts, 1,
@ -762,19 +762,25 @@ static int merge_working_tree(const struct checkout_opts *opts,
if (ret)
return ret;
o.ancestor = old_branch_info->name;
if (old_branch_info->name == NULL) {
strbuf_add_unique_abbrev(&old_commit_shortname,
&old_branch_info->commit->object.oid,
DEFAULT_ABBREV);
o.ancestor = old_commit_shortname.buf;
}
o.branch1 = new_branch_info->name;
o.branch2 = "local";
ret = merge_trees(&o,
new_tree,
work,
old_tree,
&result);
old_tree);
if (ret < 0)
exit(128);
ret = reset_tree(new_tree,
opts, 0,
writeout_error);
strbuf_release(&o.obuf);
strbuf_release(&old_commit_shortname);
if (ret)
return ret;
}

View File

@ -1,3 +1,4 @@
#include "cache.h"
#include "builtin.h"
#include "commit.h"
#include "tag.h"
@ -63,6 +64,9 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
if (argc - i != 3) /* "--" "<head>" "<remote>" */
die(_("not handling anything other than two heads merge."));
if (repo_read_index_unmerged(the_repository))
die_resolve_conflict("merge");
o.branch1 = argv[++i];
o.branch2 = argv[++i];

View File

@ -427,6 +427,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
return error(_("could not save index tree"));
reset_head();
discard_cache();
read_cache();
}
}

View File

@ -609,11 +609,66 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
return it;
}
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_valid = 0;
}
if (!index_state->cache_tree)
index_state->cache_tree = cache_tree();
if (!cache_tree_valid && cache_tree_update(index_state, flags) < 0)
return WRITE_TREE_UNMERGED_INDEX;
if (prefix) {
struct cache_tree *subtree;
subtree = cache_tree_find(index_state->cache_tree, prefix);
if (!subtree)
return WRITE_TREE_PREFIX_ERROR;
oidcpy(oid, &subtree->oid);
}
else
oidcpy(oid, &index_state->cache_tree->oid);
return 0;
}
struct tree* write_in_core_index_as_tree(struct repository *repo) {
struct object_id o;
int was_valid, ret;
struct index_state *index_state = repo->index;
was_valid = index_state->cache_tree &&
cache_tree_fully_valid(index_state->cache_tree);
ret = write_index_as_tree_internal(&o, index_state, was_valid, 0, NULL);
if (ret == WRITE_TREE_UNMERGED_INDEX) {
int i;
fprintf(stderr, "BUG: There are unmerged index entries:\n");
for (i = 0; i < index_state->cache_nr; i++) {
const struct cache_entry *ce = index_state->cache[i];
if (ce_stage(ce))
fprintf(stderr, "BUG: %d %.*s\n", ce_stage(ce),
(int)ce_namelen(ce), ce->name);
}
BUG("unmerged index entries when writing inmemory index");
}
return lookup_tree(repo, &index_state->cache_tree->oid);
}
int write_index_as_tree(struct object_id *oid, struct index_state *index_state, const char *index_path, int flags, const char *prefix)
{
int entries, was_valid;
struct lock_file lock_file = LOCK_INIT;
int ret = 0;
int ret;
hold_lock_file_for_update(&lock_file, index_path, LOCK_DIE_ON_ERROR);
@ -622,18 +677,14 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
ret = WRITE_TREE_UNREADABLE_INDEX;
goto out;
}
if (flags & WRITE_TREE_IGNORE_CACHE_TREE)
cache_tree_free(&index_state->cache_tree);
if (!index_state->cache_tree)
index_state->cache_tree = cache_tree();
was_valid = !(flags & WRITE_TREE_IGNORE_CACHE_TREE) &&
index_state->cache_tree &&
cache_tree_fully_valid(index_state->cache_tree);
was_valid = cache_tree_fully_valid(index_state->cache_tree);
if (!was_valid) {
if (cache_tree_update(index_state, flags) < 0) {
ret = WRITE_TREE_UNMERGED_INDEX;
goto out;
}
ret = write_index_as_tree_internal(oid, index_state, was_valid, flags,
prefix);
if (!ret && !was_valid) {
write_locked_index(index_state, &lock_file, COMMIT_LOCK);
/* Not being able to write is fine -- we are only interested
* in updating the cache-tree part, and if the next caller
@ -643,18 +694,6 @@ int write_index_as_tree(struct object_id *oid, struct index_state *index_state,
*/
}
if (prefix) {
struct cache_tree *subtree;
subtree = cache_tree_find(index_state->cache_tree, prefix);
if (!subtree) {
ret = WRITE_TREE_PREFIX_ERROR;
goto out;
}
oidcpy(oid, &subtree->oid);
}
else
oidcpy(oid, &index_state->cache_tree->oid);
out:
rollback_lock_file(&lock_file);
return ret;

View File

@ -34,7 +34,7 @@ int cache_tree_fully_valid(struct cache_tree *);
int cache_tree_update(struct index_state *, int);
void cache_tree_verify(struct repository *, struct index_state *);
/* bitmasks to write_cache_as_tree flags */
/* bitmasks to write_index_as_tree flags */
#define WRITE_TREE_MISSING_OK 1
#define WRITE_TREE_IGNORE_CACHE_TREE 2
#define WRITE_TREE_DRY_RUN 4
@ -46,6 +46,7 @@ void cache_tree_verify(struct repository *, struct index_state *);
#define WRITE_TREE_UNMERGED_INDEX (-2)
#define WRITE_TREE_PREFIX_ERROR (-3)
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 *);

File diff suppressed because it is too large Load Diff

View File

@ -1,104 +1,124 @@
#ifndef MERGE_RECURSIVE_H
#define MERGE_RECURSIVE_H
#include "string-list.h"
#include "unpack-trees.h"
#include "strbuf.h"
struct commit;
struct commit_list;
struct object_id;
struct repository;
struct tree;
struct merge_options_internal;
struct merge_options {
struct repository *repo;
/* ref names used in console messages and conflict markers */
const char *ancestor;
const char *branch1;
const char *branch2;
/* rename related options */
int detect_renames;
enum {
MERGE_RECURSIVE_NORMAL = 0,
MERGE_RECURSIVE_OURS,
MERGE_RECURSIVE_THEIRS
} recursive_variant;
const char *subtree_shift;
unsigned buffer_output; /* 1: output at end, 2: keep buffered */
unsigned renormalize : 1;
long xdl_opts;
int verbosity;
int detect_directory_renames;
int diff_detect_rename;
int merge_detect_rename;
int diff_rename_limit;
int merge_rename_limit;
MERGE_DIRECTORY_RENAMES_NONE = 0,
MERGE_DIRECTORY_RENAMES_CONFLICT = 1,
MERGE_DIRECTORY_RENAMES_TRUE = 2
} detect_directory_renames;
int rename_limit;
int rename_score;
int needed_rename_limit;
int show_rename_progress;
int call_depth;
struct strbuf obuf;
struct hashmap current_file_dir_set;
struct string_list df_conflict_file_set;
struct unpack_trees_options unpack_opts;
struct index_state orig_index;
struct repository *repo;
/* xdiff-related options (patience, ignore whitespace, ours/theirs) */
long xdl_opts;
enum {
MERGE_VARIANT_NORMAL = 0,
MERGE_VARIANT_OURS,
MERGE_VARIANT_THEIRS
} recursive_variant;
/* console output related options */
int verbosity;
unsigned buffer_output; /* 1: output at end, 2: keep buffered */
struct strbuf obuf; /* output buffer; if buffer_output == 2, caller
* must handle and call strbuf_release */
/* miscellaneous control options */
const char *subtree_shift;
unsigned renormalize : 1;
/* internal fields used by the implementation */
struct merge_options_internal *priv;
};
void init_merge_options(struct merge_options *opt, struct repository *repo);
/* parse the option in s and update the relevant field of opt */
int parse_merge_opt(struct merge_options *opt, const char *s);
/*
* For dir_rename_entry, directory names are stored as a full path from the
* toplevel of the repository and do not include a trailing '/'. Also:
*
* dir: original name of directory being renamed
* non_unique_new_dir: if true, could not determine new_dir
* new_dir: final name of directory being renamed
* possible_new_dirs: temporary used to help determine new_dir; see comments
* in get_directory_renames() for details
* RETURN VALUES: All the merge_* functions below return a value as follows:
* > 0 Merge was clean
* = 0 Merge had conflicts
* < 0 Merge hit an unexpected and unrecoverable problem (e.g. disk
* full) and aborted merge part-way through.
*/
struct dir_rename_entry {
struct hashmap_entry ent; /* must be the first member! */
char *dir;
unsigned non_unique_new_dir:1;
struct strbuf new_dir;
struct string_list possible_new_dirs;
};
struct collision_entry {
struct hashmap_entry ent; /* must be the first member! */
char *target_file;
struct string_list source_files;
unsigned reported_already:1;
};
static inline int merge_detect_rename(struct merge_options *o)
{
return o->merge_detect_rename >= 0 ? o->merge_detect_rename :
o->diff_detect_rename >= 0 ? o->diff_detect_rename : 1;
}
/* merge_trees() but with recursive ancestor consolidation */
int merge_recursive(struct merge_options *o,
struct commit *h1,
struct commit *h2,
struct commit_list *ancestors,
struct commit **result);
/* rename-detecting three-way merge, no recursion */
int merge_trees(struct merge_options *o,
/*
* rename-detecting three-way merge, no recursion.
*
* Outputs:
* - See RETURN VALUES above
* - No commit is created
* - opt->repo->index has the new index
* - $GIT_INDEX_FILE is not updated
* - The working tree is updated with results of the merge
*/
int merge_trees(struct merge_options *opt,
struct tree *head,
struct tree *merge,
struct tree *common,
struct tree **result);
struct tree *merge_base);
/*
* "git-merge-recursive" can be fed trees; wrap them into
* virtual commits and call merge_recursive() proper.
* merge_recursive is like merge_trees() but with recursive ancestor
* consolidation and, if the commit is clean, creation of a commit.
*
* NOTE: empirically, about a decade ago it was determined that with more
* than two merge bases, optimal behavior was found when the
* merge_bases were passed in the order of oldest commit to newest
* commit. Also, merge_bases will be consumed (emptied) so make a
* copy if you need it.
*
* Outputs:
* - See RETURN VALUES above
* - If merge is clean, a commit is created and its address written to *result
* - opt->repo->index has the new index
* - $GIT_INDEX_FILE is not updated
* - The working tree is updated with results of the merge
*/
int merge_recursive_generic(struct merge_options *o,
int merge_recursive(struct merge_options *opt,
struct commit *h1,
struct commit *h2,
struct commit_list *merge_bases,
struct commit **result);
/*
* merge_recursive_generic can operate on trees instead of commits, by
* wrapping the trees into virtual commits, and calling merge_recursive().
* It also writes out the in-memory index to disk if the merge is successful.
*
* Outputs:
* - See RETURN VALUES above
* - If merge is clean, a commit is created and its address written to *result
* - opt->repo->index has the new index
* - $GIT_INDEX_FILE is updated
* - The working tree is updated with results of the merge
*/
int merge_recursive_generic(struct merge_options *opt,
const struct object_id *head,
const struct object_id *merge,
int num_ca,
const struct object_id **ca,
int num_merge_bases,
const struct object_id **merge_bases,
struct commit **result);
void init_merge_options(struct merge_options *o,
struct repository *repo);
struct tree *write_tree_from_memory(struct merge_options *o);
int parse_merge_opt(struct merge_options *out, const char *s);
#endif

View File

@ -586,7 +586,7 @@ static int do_recursive_merge(struct repository *r,
struct replay_opts *opts)
{
struct merge_options o;
struct tree *result, *next_tree, *base_tree, *head_tree;
struct tree *next_tree, *base_tree, *head_tree;
int clean;
char **xopt;
struct lock_file index_lock = LOCK_INIT;
@ -613,11 +613,10 @@ static int do_recursive_merge(struct repository *r,
clean = merge_trees(&o,
head_tree,
next_tree, base_tree, &result);
next_tree, base_tree);
if (is_rebase_i(opts) && clean <= 0)
fputs(o.obuf.buf, stdout);
strbuf_release(&o.obuf);
diff_warn_rename_limit("merge.renamelimit", o.needed_rename_limit, 0);
if (clean < 0) {
rollback_lock_file(&index_lock);
return clean;

View File

@ -695,15 +695,22 @@ test_expect_success 'merging with triple rename across D/F conflict' '
test_expect_success 'merge-recursive remembers the names of all base trees' '
git reset --hard HEAD &&
# make the index match $c1 so that merge-recursive below does not
# fail early
git diff --binary HEAD $c1 -- | git apply --cached &&
# more trees than static slots used by oid_to_hex()
for commit in $c0 $c2 $c4 $c5 $c6 $c7
do
git rev-parse "$commit^{tree}"
done >trees &&
# ignore the return code -- it only fails because the input is weird
# ignore the return code; it only fails because the input is weird...
test_must_fail git -c merge.verbosity=5 merge-recursive $(cat trees) -- $c1 $c3 >out &&
# ...but make sure it fails in the expected way
test_i18ngrep CONFLICT.*rename/rename out &&
# merge-recursive prints in reverse order, but we do not care
sort <trees >expect &&
sed -n "s/^virtual //p" out | sort >actual &&

View File

@ -1562,6 +1562,7 @@ test_expect_success 'check nested conflicts' '
cd nested_conflicts &&
git clean -f &&
MASTER=$(git rev-parse --short master) &&
git checkout L2^0 &&
# Merge must fail; there is a conflict
@ -1582,7 +1583,7 @@ test_expect_success 'check nested conflicts' '
git cat-file -p R1:a >theirs &&
test_must_fail git merge-file --diff3 \
-L "Temporary merge branch 1" \
-L "merged common ancestors" \
-L "$MASTER" \
-L "Temporary merge branch 2" \
ours \
base \
@ -1594,7 +1595,7 @@ test_expect_success 'check nested conflicts' '
git cat-file -p R1:b >theirs &&
test_must_fail git merge-file --diff3 \
-L "Temporary merge branch 1" \
-L "merged common ancestors" \
-L "$MASTER" \
-L "Temporary merge branch 2" \
ours \
base \
@ -1732,6 +1733,7 @@ test_expect_success 'check virtual merge base with nested conflicts' '
(
cd virtual_merge_base_has_nested_conflicts &&
MASTER=$(git rev-parse --short master) &&
git checkout L3^0 &&
# Merge must fail; there is a conflict
@ -1760,7 +1762,7 @@ test_expect_success 'check virtual merge base with nested conflicts' '
cp left merged-once &&
test_must_fail git merge-file --diff3 \
-L "Temporary merge branch 1" \
-L "merged common ancestors" \
-L "$MASTER" \
-L "Temporary merge branch 2" \
merged-once \
base \

202
t/t6047-diff3-conflict-markers.sh Executable file
View File

@ -0,0 +1,202 @@
#!/bin/sh
test_description='recursive merge diff3 style conflict markers'
. ./test-lib.sh
# Setup:
# L1
# \
# ?
# /
# R1
#
# Where:
# L1 and R1 both have a file named 'content' but have no common history
#
test_expect_success 'setup no merge base' '
test_create_repo no_merge_base &&
(
cd no_merge_base &&
git checkout -b L &&
test_commit A content A &&
git checkout --orphan R &&
test_commit B content B
)
'
test_expect_success 'check no merge base' '
(
cd no_merge_base &&
git checkout L^0 &&
test_must_fail git -c merge.conflictstyle=diff3 merge --allow-unrelated-histories -s recursive R^0 &&
grep "|||||| empty tree" content
)
'
# Setup:
# L1
# / \
# master ?
# \ /
# R1
#
# Where:
# L1 and R1 have modified the same file ('content') in conflicting ways
#
test_expect_success 'setup unique merge base' '
test_create_repo unique_merge_base &&
(
cd unique_merge_base &&
test_commit base content "1
2
3
4
5
" &&
git branch L &&
git branch R &&
git checkout L &&
test_commit L content "1
2
3
4
5
7" &&
git checkout R &&
git rm content &&
test_commit R renamed "1
2
3
4
5
six"
)
'
test_expect_success 'check unique merge base' '
(
cd unique_merge_base &&
git checkout L^0 &&
MASTER=$(git rev-parse --short master) &&
test_must_fail git -c merge.conflictstyle=diff3 merge -s recursive R^0 &&
grep "|||||| $MASTER:content" renamed
)
'
# Setup:
# L1---L2--L3
# / \ / \
# master X1 ?
# \ / \ /
# R1---R2--R3
#
# Where:
# commits L1 and R1 have modified the same file in non-conflicting ways
# X1 is an auto-generated merge-base used when merging L1 and R1
# commits L2 and R2 are merges of R1 and L1 into L1 and R1, respectively
# commits L3 and R3 both modify 'content' in conflicting ways
#
test_expect_success 'setup multiple merge bases' '
test_create_repo multiple_merge_bases &&
(
cd multiple_merge_bases &&
test_commit initial content "1
2
3
4
5" &&
git branch L &&
git branch R &&
# Create L1
git checkout L &&
test_commit L1 content "0
1
2
3
4
5" &&
# Create R1
git checkout R &&
test_commit R1 content "1
2
3
4
5
6" &&
# Create L2
git checkout L &&
git merge R1 &&
# Create R2
git checkout R &&
git merge L1 &&
# Create L3
git checkout L &&
test_commit L3 content "0
1
2
3
4
5
A" &&
# Create R3
git checkout R &&
git rm content &&
test_commit R3 renamed "0
2
3
4
5
six"
)
'
test_expect_success 'check multiple merge bases' '
(
cd multiple_merge_bases &&
git checkout L^0 &&
test_must_fail git -c merge.conflictstyle=diff3 merge -s recursive R^0 &&
grep "|||||| merged common ancestors:content" renamed
)
'
test_expect_success 'rebase describes fake ancestor base' '
test_create_repo rebase &&
(
cd rebase &&
test_commit base file &&
test_commit master file &&
git checkout -b side HEAD^ &&
test_commit side file &&
test_must_fail git -c merge.conflictstyle=diff3 rebase master &&
grep "||||||| constructed merge base" file
)
'
test_done